[PATCH] fscrypt: fix the test_dummy_encryption mount option

[PATCH] fscrypt: fix the test_dummy_encryption mount option

[Theodore Ts'o]

Commit f1c131b45410a: "crypto: xts - Convert to skcipher" now fails
the setkey operation if the AES key is the same as the tweak key.
Previously this check was only done if FIPS mode is enabled.  Now this
check is also done if weak key checking was requested.  This is
reasonable, but since we were using the dummy key which was a constant
series of 0x42 bytes, it now caused dummy encrpyption test mode to
fail.

Fix this by using 0x42... and 0x24... for the two keys, so they are
different.

Fixes: f1c131b45410a202eb45cc55980a7a9e4e4b4f40
Cc: stable@vger.kernel.org
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/crypto/keyinfo.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 6eeea1dcba41..95cd4c3b06c3 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -248,7 +248,8 @@ int fscrypt_get_crypt_info(struct inode *inode)
 		goto out;
 
 	if (fscrypt_dummy_context_enabled(inode)) {
-		memset(raw_key, 0x42, FS_AES_256_XTS_KEY_SIZE);
+		memset(raw_key, 0x42, keysize/2);
+		memset(raw_key+keysize/2, 0x24, keysize - (keysize/2));
 		goto got_key;
 	}
 
-- 
2.11.0.rc0.7.gbe5a750

Re: [PATCH] fscrypt: fix the test_dummy_encryption mount option

[Eric Biggers]

Hi Ted,

On Tue, Dec 27, 2016 at 07:51:47PM -0500, Theodore Ts'o wrote:
> Commit f1c131b45410a: "crypto: xts - Convert to skcipher" now fails
> the setkey operation if the AES key is the same as the tweak key.
> Previously this check was only done if FIPS mode is enabled.  Now this
> check is also done if weak key checking was requested.  This is
> reasonable, but since we were using the dummy key which was a constant
> series of 0x42 bytes, it now caused dummy encrpyption test mode to
> fail.
> 
> Fix this by using 0x42... and 0x24... for the two keys, so they are
> different.
> 

This problem would also be fixed by my patch to make the test_dummy_encryption
encryption keys go through the regular keyring lookup and key derivation paths,
which IMO is a better solution long-term:

	fscrypt / ext4: make test_dummy_encryption require a keyring key

and corresponding xfstests-bld patch:

	xfstests-bld: populate keyring with default key for test_dummy_encryption

Would it make any sense to apply those patches instead?

I'd also be okay with your patch for 4.10 and mine for 4.11 or something like
that.

Eric

Re: [PATCH] fscrypt: fix the test_dummy_encryption mount option

[Theodore Ts'o]

On Wed, Dec 28, 2016 at 03:27:59PM -0600, Eric Biggers wrote:
> This problem would also be fixed by my patch to make the test_dummy_encryption
> encryption keys go through the regular keyring lookup and key derivation paths,
> which IMO is a better solution long-term:
> 
> 	fscrypt / ext4: make test_dummy_encryption require a keyring key
> 
> and corresponding xfstests-bld patch:
> 
> 	xfstests-bld: populate keyring with default key for test_dummy_encryption
> 

My problem with this patch is that it breaks backwards compatibility
with older kernels --- such as the 3.10 and 3.18 kernels currently
shipping today in Android handsets.  So I don't want to make changes
to xfstests-bld that require specific kernel patches which aren't
necesarily available on existing kernels which are in use in
production today.

And it won't necessarily be simple to get your fscrypt/ext4 change
into all of the various Android device kernels, the android-common
kernels, the unreleased device kernels in use at various handset
manufactuers, etc.

I don't know if there are going to be a lot of device handset
manufacturers outside of Google that will be trying to make
xfstests-bld work on Android, or by compiling a device kernel on x86
and then running kvm-xfstests or gce-xfstests as the only easy way to
test an Android kernel --- but if someone from some handset
manufacturer comes up to me and asks me for help debugging ext4
encryption, I'd much rather be able to point them at the standard
kvm-xfstests image and tell them it will work with their device kernel
which was based off of the 3.10 or 3.18 android-common git tree.

Cheers,

						- Ted

Re: [PATCH] fscrypt: fix the test_dummy_encryption mount option

[Eric Biggers]

On Wed, Dec 28, 2016 at 07:45:26PM -0500, Theodore Ts'o wrote:
> On Wed, Dec 28, 2016 at 03:27:59PM -0600, Eric Biggers wrote:
> > This problem would also be fixed by my patch to make the test_dummy_encryption
> > encryption keys go through the regular keyring lookup and key derivation paths,
> > which IMO is a better solution long-term:
> > 
> > 	fscrypt / ext4: make test_dummy_encryption require a keyring key
> > 
> > and corresponding xfstests-bld patch:
> > 
> > 	xfstests-bld: populate keyring with default key for test_dummy_encryption
> > 
> 
> My problem with this patch is that it breaks backwards compatibility
> with older kernels --- such as the 3.10 and 3.18 kernels currently
> shipping today in Android handsets.  So I don't want to make changes
> to xfstests-bld that require specific kernel patches which aren't
> necesarily available on existing kernels which are in use in
> production today.
> 
> And it won't necessarily be simple to get your fscrypt/ext4 change
> into all of the various Android device kernels, the android-common
> kernels, the unreleased device kernels in use at various handset
> manufactuers, etc.
> 

Actually the patched xfstests-bld can still test both old and new kernels.
Therefore there would be no need to backport the kernel patch.  The xfstests-bld
patch just adds a key to the keyring, which new kernels will use but old kernels
won't (since when test_dummy_encryption is enabled, old kernels don't look at
the keyring at all).

Granted, there is breakage in the other direction --- the kernel change breaks
the current xfstests-bld --- but that's not really an issue since we can just
update xfstests-bld.

Eric