| Message ID | 20170616183411.31587-1-ebiggers3@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Jun 16, 2017, at 12:34 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > > From: Eric Biggers <ebiggers@google.com> > > Currently it's possible to encrypt all files and directories on an ext4 > filesystem by deleting everything, including lost+found, then setting an > encryption policy on the root directory. However, this is incompatible > with e2fsck because e2fsck expects to find, create, and/or write to > lost+found and does not have access to any encryption keys. Especially > problematic is that if e2fsck can't find lost+found, it will create it > without regard for whether the root directory is encrypted. This is > wrong for obvious reasons, and it causes a later run of e2fsck to > consider the lost+found directory entry to be corrupted. > > Encrypting the root directory may also be of limited use because it is > the "all-or-nothing" use case, for which dm-crypt can be used instead. > (By design, encryption policies are inherited and cannot be overridden; > so the root directory having an encryption policy implies that all files > and directories on the filesystem have that same encryption policy.) > > In any case, encrypting the root directory is broken currently and must > not be allowed; so start returning an error if userspace requests it. > For now only do this in ext4, because f2fs and ubifs do not appear to > have the lost+found requirement. We could move it into > fscrypt_ioctl_set_policy() later if desired, though. > > Signed-off-by: Eric Biggers <ebiggers@google.com> Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > > v2: use EPERM instead of EBUSY, and tweak commit message > > fs/ext4/super.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index d37c81f327e7..d5b5c80c23f5 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -1145,6 +1145,15 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len, > handle_t *handle = fs_data; > int res, res2, retries = 0; > > + /* > + * Encrypting the root directory is not allowed because e2fsck expects > + * lost+found to exist and be unencrypted, and encrypting the root > + * directory would imply encrypting the lost+found directory as well as > + * the filename "lost+found" itself. > + */ > + if (inode->i_ino == EXT4_ROOT_INO) > + return -EPERM; > + > res = ext4_convert_inline_data(inode); > if (res) > return res; > -- > 2.13.1.518.g3df882009-goog > Cheers, Andreas
On Mon, Jun 19, 2017 at 03:18:24PM -0600, Andreas Dilger wrote: > On Jun 16, 2017, at 12:34 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > > > > From: Eric Biggers <ebiggers@google.com> > > > > Currently it's possible to encrypt all files and directories on an ext4 > > filesystem by deleting everything, including lost+found, then setting an > > encryption policy on the root directory. However, this is incompatible > > with e2fsck because e2fsck expects to find, create, and/or write to > > lost+found and does not have access to any encryption keys. Especially > > problematic is that if e2fsck can't find lost+found, it will create it > > without regard for whether the root directory is encrypted. This is > > wrong for obvious reasons, and it causes a later run of e2fsck to > > consider the lost+found directory entry to be corrupted. > > > > Encrypting the root directory may also be of limited use because it is > > the "all-or-nothing" use case, for which dm-crypt can be used instead. > > (By design, encryption policies are inherited and cannot be overridden; > > so the root directory having an encryption policy implies that all files > > and directories on the filesystem have that same encryption policy.) > > > > In any case, encrypting the root directory is broken currently and must > > not be allowed; so start returning an error if userspace requests it. > > For now only do this in ext4, because f2fs and ubifs do not appear to > > have the lost+found requirement. We could move it into > > fscrypt_ioctl_set_policy() later if desired, though. > > > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > Reviewed-by: Andreas Dilger <adilger@dilger.ca> Thanks, applied. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-fscrypt" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index d37c81f327e7..d5b5c80c23f5 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1145,6 +1145,15 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len, handle_t *handle = fs_data; int res, res2, retries = 0; + /* + * Encrypting the root directory is not allowed because e2fsck expects + * lost+found to exist and be unencrypted, and encrypting the root + * directory would imply encrypting the lost+found directory as well as + * the filename "lost+found" itself. + */ + if (inode->i_ino == EXT4_ROOT_INO) + return -EPERM; + res = ext4_convert_inline_data(inode); if (res) return res;