Re: [PATCH] fscrypt / ext4: make test_dummy_encryption require a keyring key

[Theodore Ts'o <tytso@mit.edu>]

On Wed, Jan 04, 2017 at 04:16:06PM -0800, Eric Biggers wrote:
> I'm fine with your proposed version, though I'm not convinced it's really any
> better than mine, since it basically just moves the "hack" from
> fscrypt_inherit_context() to fscrypt_get_encryption_info().  The reason I
> preferred it in fscrypt_inherit_context() was that allowing
> fscrypt_get_encryption_info() to work on unencrypted files is kind of weird and
> could allow for confusing scenarios where a previously existing unencrypted file
> is accidentally treated as an encrypted one --- though that would require a
> missing ext4_encrypted_inode() check of course.

Except that you *always* need to call ext4_encrypt_inode() before you
call fscrypt_get_encryption_info(), because otherwise it becomes a
performance disaster in the no encryption case, because we would be
constantly doing failing xattr lookups.

It also made for some especially tangled logic, which I noticed when
you had to make a change in fs/ext4/namei.c:

        if ((ext4_encrypted_inode(dir) ||  <-----------------------
             DUMMY_ENCRYPTION_ENABLED(EXT4_SB(dir->i_sb))) &&
            (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode))) {
-               err = fscrypt_get_encryption_info(dir);
-               if (err)
-                       return ERR_PTR(err);
-               if (!fscrypt_has_encryption_key(dir))
-                       return ERR_PTR(-EPERM);
+               if (ext4_encrypted_inode(dir)) {   <-------------------
+                       err = fscrypt_get_encryption_info(dir);
+                       if (err)
+                               return ERR_PTR(err);
+                       if (!fscrypt_has_encryption_key(dir))
+                               return ERR_PTR(-EPERM);
+               }
                if (!handle)


Your patch required the addition of a *second* call to
ext4_encrypted_inode() or else the call to
fscrypt_get_encryption_info() would fail in the test_dummy_encryption
case.

Not having hacks in the fscrypt_inherit_context() case also has the
happy advantage that we test the normal context inheritance code path
when creating files in the (unencrypted) root directory.

						- Ted