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

Proposed by Richard Laager on 2016-04-25
Status: Merged
Merged at revision: 881
Proposed branch: lp:~rlaager/ecryptfs/fix-lp-1574174-2
Merge into: lp:ecryptfs
Diff against target: 12 lines (+1/-1)
1 file modified
src/utils/ecryptfs-setup-private (+1/-1)
To merge this branch: bzr merge lp:~rlaager/ecryptfs/fix-lp-1574174-2
Reviewer Review Type Date Requested Status
Tyler Hicks 2016-04-25 Approve on 2016-04-25
Review via email:

This proposal supersedes a proposal from 2016-04-25.

Description of the change

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.

To post a comment you must log in.
Tyler Hicks (tyhicks) wrote : Posted in a previous version of this proposal

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) wrote : Posted in a previous version of this proposal

I think the check should be:

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

Richard Laager (rlaager) wrote : Posted in a previous version of this proposal

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.

Richard Laager (rlaager) wrote :

I've resubmitted this without the CRYPTDIR change.

Tyler Hicks (tyhicks) wrote :

No problem.

This one looks good. Thanks again!

review: Approve
Richard Laager (rlaager) wrote :

I don't know what the process or timeline is from here, but once this turns into an Ubuntu package, I'll retest. Ideally, after that, I'd like to see this fix SRUed into Xenial.

Tyler Hicks (tyhicks) wrote :

Outside of this bug, can you comment on how well eCryptfs and ZFS are working together?

Richard Laager (rlaager) wrote :

Bottom line: I haven't run into any problems in my testing, but I'm not personally running it day-to-day.

It's something I've long supported in my HOWTO for root-on-ZFS, but I don't know how many people use that combination. I primarily support it because it's supported out-of-the-box (i.e. in the installer) by Ubuntu on non-ZFS filesystems.

I filed this bug report because if I'm going to support it, it'd be nice if it didn't need the extra steps: "create the ZFS filesystem at /home/temp-USER, ecryptfs-setup-private, rename the temp-USER filesystem to USER".

Coincidentally, I've just posted something to the ZFS-on-Linux mailing list which may elicit responses from such users (among others).

The reason I personally stopped using ecryptfs (which I used for years, first for ~/Private, then ~) when I moved to ZFS is that I lose the ability to dive into old snapshots easily. In ZFS, you can do this with "cd ~/.zfs/snapshot". The problem is that the children of .zfs/snapshot are auto-mounted. And my skills were not enough to figure out any way to make ecryptfs work through that auto-mount transparently. I'm not a kernel guy, but I did dive into the kernel code a bit.

So with the current state of affairs, I'd have to dig up the ecryptfs private key and mount the snapshot somewhere else through ecryptfs to be able to read my data.

People seem to be more interested in running LUKS under ZFS. This provides all the goodness of ZFS, with the encryption they want.

Richard Laager (rlaager) wrote :

Do you have specific concerns about ZFS + eCryptfs? If it's important to you, I'm open to the idea of running it for a while for testing.

Tyler Hicks (tyhicks) wrote :

I don't have specific concerns but I haven't regularly tested the combination. I was just curious if you were aware of any issues. Thanks!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 22:49:43 +0000
@@ -231,7 +231,7 @@
233# Check for active mounts233# Check for active mounts
234grep -qs "$MOUNTPOINT " /proc/mounts && error "[$MOUNTPOINT]" "$(gettext 'is already mounted')"234grep -qs " $MOUNTPOINT " /proc/mounts && error "[$MOUNTPOINT]" "$(gettext 'is already mounted')"
235grep -qs "$CRYPTDIR " /proc/mounts && error "[$CRYPTDIR]" "$(gettext 'is already mounted')"235grep -qs "$CRYPTDIR " /proc/mounts && error "[$CRYPTDIR]" "$(gettext 'is already mounted')"
237# Check that the mount point and encrypted directory are empty (skip symlinks).237# Check that the mount point and encrypted directory are empty (skip symlinks).


People subscribed via source and target branches