search

dyne / tomb

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

improve the check if a tomb file is in use #389

Closed jaromil closed 3 years ago

jaromil commented 3 years ago

Change the mapper path using a hash of the tomb file path, making it unique and reproducible to check if tomb is in use. Check happens inside the new render_mapper() function which is executed right after the key file opening.

Provides a fix for #387 and #326 as well signaled in https://github.com/roddhjav/pass-tomb/issues/27

shaform commented 3 years ago

Thanks for the update. It does avoid the problem in roddhjav/pass-tomb/issues/27.

I've noticed a small problem though. Because the hash is computed based on the input path. If the users use the following commands, they could still mount the same file multiple times. But I guess this situation might be rare.

$ cd /tmp
$ tomb dig -s 100 secret.tomb
$ tomb forge secret.tomb.key
$ tomb lock secret.tomb -k secret.tomb.key
$ mkdir -p target_dir
$ tomb open secret.tomb target_dir -k secret.tomb.key
$ tomb open ../tmp/secret.tomb target_dir -k secret.tomb.key
$ tomb open ../tmp/../tmp/secret.tomb target_dir -k secret.tomb.key
shaform commented 3 years ago

I also notice that if we use the version in this PR. Every time the tomb is closed. A new entry of a encrypted disk would be added to the sidebar of nautilus: side bar

However, this issue does not exist in Tomb 2.7.

Not sure if this would have any negative impact except for cluttering the sidebar.

jaromil commented 3 years ago

I've noticed a small problem though. Because the hash is computed based on the input path. If the users use the following commands, they could still mount the same file multiple times. But I guess this situation might be rare.

nevertheless, good to have it fixed, thanks! using realpath now to computer always an absolute path.

jaromil commented 3 years ago

I also notice that if we use the version in this PR. Every time the tomb is closed. A new entry of a encrypted disk would be added to the sidebar of nautilus: However, this issue does not exist in Tomb 2.7.

Not sure if this would have any negative impact except for cluttering the sidebar.

I don't know Nautilus well and puzzled by the fact this wasn't happening in Tomb 2.7 already since the mountpoint haven't changed, just the length of the /dev/mapper file... would be good to receive advice from someone more expert about Nautilus or at least find its logic documented somewhere, freedesktop "standards" or the likes.

I'll leave this to another PR if any in the future that can improve desktop integration.

roddhjav commented 3 years ago

I have the same issue. It seems the loop devices are not correctly detached (and are still visible with losetup), it explains why nautilus shows them. Doing losetup --detach-all fix it but is not really a sustainable solution.

shaform commented 3 years ago

I have done some tests and it seems that the problem arises when .$(basename $nstloop) is removed from the filename of the mapper.

jaromil commented 3 years ago

My wild guess on a (undocumented feature?) in Nautilus: mounted /dev/mapper volumes with .loop extension don't get listed in sidebar? is there anything like this?

jaromil commented 3 years ago

I've found out that is possible to hide mount drives from this setting: gsettings set org.gnome.shell.extensions.dash-to-dock show-mounts false which I believe may be the desired option for deniability, maybe worth mentioning in docs.

shaform commented 3 years ago

@jaromil

My wild guess on a (undocumented feature?) in Nautilus: mounted /dev/mapper volumes with .loop extension don't get listed in sidebar? is there anything like this?

No, simply adding .loop does not work. It has to be .loop[N] and [N] has to be the actual number that is used for mapping the device.

As mentioned by @roddhjav, this probably is not specific to Nautilus, but rather, when the file name does not contain the actual loop device being used, the loop device would not be detached even after tomb close is executed.

jaromil commented 3 years ago

I have now seen the issue with the missing detachment of the loop file, there is a problem in carrying the state of the loop number and there are solutions I'll be experimenting with.

jaromil commented 3 years ago

Please if you can share some feedback on #391 seems to fix it see https://travis-ci.org/github/dyne/Tomb/builds/744123599 also it improves the code by throwing away some and making some cleanup (had it in mind, long due)