| Message ID | 20161228005147.751-1-tytso@mit.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" 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/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; }
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(-)