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

Proposed by Richard Laager
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 Approve
Review via email: mp+292866@code.launchpad.net

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.
Revision history for this message
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
Revision history for this message
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')"

Revision history for this message
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.

Revision history for this message
Richard Laager (rlaager) wrote :

I've resubmitted this without the CRYPTDIR change.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

No problem.

This one looks good. Thanks again!

review: Approve
Revision history for this message
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.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

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

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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
1=== modified file 'src/utils/ecryptfs-setup-private'
2--- src/utils/ecryptfs-setup-private 2016-02-26 23:57:37 +0000
3+++ src/utils/ecryptfs-setup-private 2016-04-25 22:49:43 +0000
4@@ -231,7 +231,7 @@
5 fi
6
7 # Check for active mounts
8-grep -qs "$MOUNTPOINT " /proc/mounts && error "[$MOUNTPOINT]" "$(gettext 'is already mounted')"
9+grep -qs " $MOUNTPOINT " /proc/mounts && error "[$MOUNTPOINT]" "$(gettext 'is already mounted')"
10 grep -qs "$CRYPTDIR " /proc/mounts && error "[$CRYPTDIR]" "$(gettext 'is already mounted')"
11
12 # Check that the mount point and encrypted directory are empty (skip symlinks).

Subscribers

People subscribed via source and target branches