Merge lp:~rlaager/ecryptfs/fix-lp-1574174 into lp:ecryptfs

[Richard Laager]

Fix improper "already mounted" errors with ZFS

The obvious approach for using ZFS and ecryptfs together involves
creating a dataset like this:
zfs create -o mountpoint=/home/.ecryptfs/USER rpool/home/USER

As a result, /proc/mounts looks like this:
rpool/home/USER /home/.ecryptfs/USER zfs rw,xattr 0 0

ecryptfs-setup-private checked for existing mount points using a grep
which was effectively left-anchored. Unfortunately, this can match the
device column. A space at the beginning of the pattern corrects this.

I applied the same fix to the CRYPTDIR for consistency and correctness,
though I do not believe it was a practical problem in the same way.

[Tyler Hicks (tyhicks)]

Hi Richard - Thanks for the merge proposal. I could be wrong because I didn't test this but I think adding a leading space to CRYPTDIR would break the "already mounted" check since CRYPTDIR is the device (or source) and will be listed at the beginning of a line in /proc/mounts. The check will never fail even if the CRYPTDIR is already mounted.

review: Needs Fixing

[Tyler Hicks (tyhicks)]

I think the check should be:

grep -qs "^$CRYPTDIR " /proc/mounts && error "[$CRYPTDIR]" "$(gettext 'is already mounted')"

[Richard Laager (rlaager)]

Right, I'm dumb. I added that last minute for "consistency". CRYPTDIR isn't the problem for ZFS anyway, MOUNTDIR is. Just drop the CRYPTDIR change from me.

Unmerged revisions

881. By Richard Laager on 2016-04-25

Fix improper "already mounted" errors with ZFS

The obvious approach for using ZFS and ecryptfs together involves
creating a dataset like this:
zfs create -o mountpoint=/home/.ecryptfs/USER rpool/home/USER

As a result, /proc/mounts looks like this:
rpool/home/USER /home/.ecryptfs/USER zfs rw,xattr 0 0

ecryptfs-setup-private checked for existing mount points using a grep
which was effectively left-anchored. Unfortunately, this can match the
device column. A space at the beginning of the pattern corrects this.

I applied the same fix to the CRYPTDIR for consistency and correctness,
though I do not believe it was a practical problem in the same way.

=== modified file 'src/utils/ecryptfs-setup-private'
--- src/utils/ecryptfs-setup-private	2016-02-26 23:57:37 +0000
+++ src/utils/ecryptfs-setup-private	2016-04-25 18:00:16 +0000
@@ -231,8 +231,8 @@
 fi
 
 # Check for active mounts
-grep -qs "$MOUNTPOINT " /proc/mounts && error "[$MOUNTPOINT]" "$(gettext 'is already mounted')"
-grep -qs "$CRYPTDIR " /proc/mounts && error "[$CRYPTDIR]" "$(gettext 'is already mounted')"
+grep -qs " $MOUNTPOINT " /proc/mounts && error "[$MOUNTPOINT]" "$(gettext 'is already mounted')"
+grep -qs " $CRYPTDIR " /proc/mounts && error "[$CRYPTDIR]" "$(gettext 'is already mounted')"
 
 # Check that the mount point and encrypted directory are empty (skip symlinks).
 # Perhaps one day we could provide a migration mode (using rsync or something),