dyne / tomb

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

mount_tomb: make use of ACL in specific locations #475

Open Narrat opened 1 year ago

Narrat commented 1 year ago

Namely /run/media/$USER. The mount point itself is owned by root, therefore one needs to know the name of the mountpoint to change to the location. Other tools for mounting media like udisksctl set ACL to allow the owner to use it normally (autocompletion and such).

Fixes #461


To get the solution for the named issue going. There were some undecided points and maybe with some changed code this can go on further.

Therefore this PR is only a draft. Request for comments

jaromil commented 1 year ago

This is especially interesting if it allows us to remove the chown to $USER in case ACL==1 and just use setfacl.

Narrat commented 1 year ago

Interesting point. Theoretically it would allow that. But then it probably needs a better check for ACL support. Let's see what can be done.

Narrat commented 1 year ago

Urg.. not sure how to proceed, as this could easily spiral somewhat out of control and cause big changes :D At least it seems currently like that. In general all major file systems have ACL support (ext, btrfs, zfs, tmpfs, f2fs). At least that worry is gone. And it seems to be included in the default mount options. But doesn't hinder people to mount with noacl. And it currently seems there is no easy way to detect if it is supported in the mount location. Unless trying to parse the output of mount? In general it could be nice to use it also for mount points like /media (as an alternative to chown). But that needs special or a changed handling as /media and /run/media/$USER/ are somewhat different. The ACL in the latter case currently just makes sure that the $USER folder can be accessed by the $USER as it isn't (and shouldn't?) covered by the chown. The actual mountpoint can indeed be additionally handled by ACL instead of the chown. And from that point things are simpler. Default ext4, default options. So replacing the chown and whatever should happend with the -p option could be viable. And ACL could be stored in an additional file for easy restoration and allowing the $USER customizing it. Comments? Or something I overlooked?

jaromil commented 11 months ago

We need more exhaustive testing for this, many corner cases can arise... Should we leave the code as-is? Perhaps ACL as an opt-in rather than auto-detect.

Narrat commented 11 months ago

This is definitively nothing for 2.10 :D

I lean to split the approach:

For the first point we can use auto-detect or entirely switch, as there is control over the initial tomb. But could also be be opt-in. Set a flag and a ACL config file is placed beside the other tomb files in a tomb. If it's there apply that instead of the chown. Regarding the second point.. yeah... Opt-in may be the way to go. But still not really sure how to approach it the best (although I must admit this got again lost in daily chores...)

Narrat commented 2 weeks ago

The scope of this PR adjusted to its original intent to match the behaviour when mounting a tomb at /run/media/$USER/. Additional thoughts on replacing chown or support ACL-files in a tomb aren't forgotten, but should discussed separately. I got some ideas on how to proceed with those cases, but currently there are enough open construction sites :D