[PATCH v2] ext4: forbid encrypting root directory

[PATCH v2] ext4: forbid encrypting root directory

[Eric Biggers]

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>
---

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

Re: [PATCH v2] ext4: forbid encrypting root directory

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2553 bytes --]

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






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH v2] ext4: forbid encrypting root directory

[Theodore Ts'o]

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