dyne / tomb

the Crypto Undertaker
https://dyne.org/software/tomb
GNU General Public License v3.0
1.32k stars 150 forks source link

mountpoint `/run/media/$USER` always with root ownership #461

Open amalgame21 opened 1 year ago

amalgame21 commented 1 year ago

After reboot, running tomb open example.tomb -k example.tomb.key, it mounts on /run/media/$USER which with root ownership. As a normal user, somehow I cannot use the bash auto-completion to cd into the /run/media/$USER/example, I have to type the absolute path to cd into that. So every time I just use sudo chown $USER:$USER /run/media/$USER to change the ownership of that mountpoint (I do not know if it impact the security) so I can use the bash auto-completion. However after a reboot, this problem happen again. Is it written by design for security? Thank.

amalgame21 commented 1 year ago

Sorry let me re-describe the issue.

I found that the problem should be after reboot, /run/media/$USER is removed, leaving only /run/media in the system, and tomb open will recreate /run/media/$USER will 711 permission.

Switch it to 755 manually should be fine, but I don't know if it is security concern by design.

I cannot find anything about it in the source code mount_tomb() section. So I have no idea why it created with 711 permission.

amalgame21 commented 1 year ago

Created a /media folder to use tomb open and all problem solved.

Narrat commented 1 year ago

See the comments to this commit: https://github.com/dyne/Tomb/commit/843b7fdfc4c125065d31cc11cff8a994ed680bd4 It was different and changed somewhere on the road. I had planned to look into this, but it was kinda forgotten. As I was the only one who was "bothered" by it :D

Narrat commented 1 year ago

And regarding /media: Using this location has some other security related implications. One of the reasons it got in general abandoned in favour of /run/media/<user>

jaromil commented 1 year ago

hi @amalgame21 ! thanks for the report and @Narrat for keeping track of this issue through earlier comments. I would like to look into this and improve if possible, perhaps just a non-recursive chown on mount dir in /var/run ?

Narrat commented 1 year ago

Maybe. At least this was done in the past? What bothers me still, is why it seemed to work with chown'ing the file inside the tomb. Will try to do some test scenarios this week. I also remember that it made a difference if /run/media/<user> was already existing (due to mounting an USB drive or else) or not.

Narrat commented 1 year ago

Mounting in run/media seems to involve ACL (Access Control Lists). Mounting with tomb produces the following dir structure: run/media/$user/<mount> And the dir $user has the following permissions:

/run/media/
$  ls -l
insgesamt 0
drwx--x--x 3 root root 60  7. Jan 20:12 testuser

After removing the folders and mounting something with for example udiskctl the dir structure is the same: run/media/$user/<mount>. But the permissions differ:

/run/media/
$  ls -l
insgesamt 0
drwxr-x---+  3 root root  60  7. Jan 20:26 testuser

There is the additional + which tells us about existing ACL:

/run/media/
$ getfacl testuser 
# file: user
# owner: root
# group: root
user::rwx
user:testuser:r-x
group::---
mask::r-x
other::---
amalgame21 commented 1 year ago

@Narrat Yes, that's also what I discovered between the mount point permission of udiskie mounting USBs and tomb mounting tomb files. I did not know what the + stand for in the udiskie case, but it seems magically make the mount point readable to users other than root, although the permission was set to 750. ACL is a new word to me, thanks for mentioning this.

jaromil commented 1 year ago

I wonder, shall we then use setfacl on the /run/media/$USER/$tombname mount folder with something like: setfacl --set=u::rwx,u:$USER:rwx,g::---,g:$USER:---,m::---,o::--- perhaps printing it in log messages to ease adjustments by hand?

Narrat commented 1 year ago

Imo yes. But should it only be used on /run/media? Or is there also a gain if it used on other mount locations? And there should probably a check if the filesystem supports ACL. Regarding the command itself. Shouldn't setfacl --modify=u:$USER:rwx be enough? Dunno about setting the group, because that varies from distribution to distribution. Debian and such create a group for the username. Others have a generic group users. And if I open something with secrets only I want to be able to see the contents.