| Message ID | 20191009233840.224128-1-ebiggers@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | fscrypt: invoke crypto API for ESSIV handling | expand |
On Thu, 10 Oct 2019 at 01:39, Eric Biggers <ebiggers@kernel.org> wrote: > > From: Eric Biggers <ebiggers@google.com> > > Instead of open-coding the calculations for ESSIV handling, use an ESSIV > skcipher which does all of this under the hood. ESSIV was added to the > crypto API in v5.4. > > This is based on a patch from Ard Biesheuvel, but reworked to apply > after all the fscrypt changes that went into v5.4. > > Tested with 'kvm-xfstests -c ext4,f2fs -g encrypt', including the > ciphertext verification tests for v1 and v2 encryption policies. > > Originally-from: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Signed-off-by: Eric Biggers <ebiggers@google.com> Thanks for picking this up. Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> [in case it matters :-)] > --- > Documentation/filesystems/fscrypt.rst | 5 +- > fs/crypto/crypto.c | 4 - > fs/crypto/fscrypt_private.h | 7 -- > fs/crypto/keysetup.c | 110 +++----------------------- > fs/crypto/keysetup_v1.c | 4 - > 5 files changed, 14 insertions(+), 116 deletions(-) > > diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst > index 8a0700af95967..6ec459be3de16 100644 > --- a/Documentation/filesystems/fscrypt.rst > +++ b/Documentation/filesystems/fscrypt.rst > @@ -308,8 +308,9 @@ If unsure, you should use the (AES-256-XTS, AES-256-CTS-CBC) pair. > > AES-128-CBC was added only for low-powered embedded devices with > crypto accelerators such as CAAM or CESA that do not support XTS. To > -use AES-128-CBC, CONFIG_CRYPTO_SHA256 (or another SHA-256 > -implementation) must be enabled so that ESSIV can be used. > +use AES-128-CBC, CONFIG_CRYPTO_ESSIV and CONFIG_CRYPTO_SHA256 (or > +another SHA-256 implementation) must be enabled so that ESSIV can be > +used. > > Adiantum is a (primarily) stream cipher-based mode that is fast even > on CPUs without dedicated crypto instructions. It's also a true > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c > index 32a7ad0098cc2..6bc3e4f1e657e 100644 > --- a/fs/crypto/crypto.c > +++ b/fs/crypto/crypto.c > @@ -27,7 +27,6 @@ > #include <linux/ratelimit.h> > #include <linux/dcache.h> > #include <linux/namei.h> > -#include <crypto/aes.h> > #include <crypto/skcipher.h> > #include "fscrypt_private.h" > > @@ -143,9 +142,6 @@ void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num, > > if (fscrypt_is_direct_key_policy(&ci->ci_policy)) > memcpy(iv->nonce, ci->ci_nonce, FS_KEY_DERIVATION_NONCE_SIZE); > - > - if (ci->ci_essiv_tfm != NULL) > - crypto_cipher_encrypt_one(ci->ci_essiv_tfm, iv->raw, iv->raw); > } > > /* Encrypt or decrypt a single filesystem block of file contents */ > diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h > index e84efc01512e4..76c64297ce187 100644 > --- a/fs/crypto/fscrypt_private.h > +++ b/fs/crypto/fscrypt_private.h > @@ -163,12 +163,6 @@ struct fscrypt_info { > /* The actual crypto transform used for encryption and decryption */ > struct crypto_skcipher *ci_ctfm; > > - /* > - * Cipher for ESSIV IV generation. Only set for CBC contents > - * encryption, otherwise is NULL. > - */ > - struct crypto_cipher *ci_essiv_tfm; > - > /* > * Encryption mode used for this inode. It corresponds to either the > * contents or filenames encryption mode, depending on the inode type. > @@ -444,7 +438,6 @@ struct fscrypt_mode { > int keysize; > int ivsize; > bool logged_impl_name; > - bool needs_essiv; > }; > > static inline bool > diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c > index d71c2d6dd162a..8eb5a0e762ec6 100644 > --- a/fs/crypto/keysetup.c > +++ b/fs/crypto/keysetup.c > @@ -8,15 +8,11 @@ > * Heavily modified since then. > */ > > -#include <crypto/aes.h> > -#include <crypto/sha.h> > #include <crypto/skcipher.h> > #include <linux/key.h> > > #include "fscrypt_private.h" > > -static struct crypto_shash *essiv_hash_tfm; > - > static struct fscrypt_mode available_modes[] = { > [FSCRYPT_MODE_AES_256_XTS] = { > .friendly_name = "AES-256-XTS", > @@ -31,11 +27,10 @@ static struct fscrypt_mode available_modes[] = { > .ivsize = 16, > }, > [FSCRYPT_MODE_AES_128_CBC] = { > - .friendly_name = "AES-128-CBC", > - .cipher_str = "cbc(aes)", > + .friendly_name = "AES-128-CBC-ESSIV", > + .cipher_str = "essiv(cbc(aes),sha256)", > .keysize = 16, > .ivsize = 16, > - .needs_essiv = true, > }, > [FSCRYPT_MODE_AES_128_CTS] = { > .friendly_name = "AES-128-CTS-CBC", > @@ -111,97 +106,16 @@ struct crypto_skcipher *fscrypt_allocate_skcipher(struct fscrypt_mode *mode, > return ERR_PTR(err); > } > > -static int derive_essiv_salt(const u8 *key, int keysize, u8 *salt) > -{ > - struct crypto_shash *tfm = READ_ONCE(essiv_hash_tfm); > - > - /* init hash transform on demand */ > - if (unlikely(!tfm)) { > - struct crypto_shash *prev_tfm; > - > - tfm = crypto_alloc_shash("sha256", 0, 0); > - if (IS_ERR(tfm)) { > - if (PTR_ERR(tfm) == -ENOENT) { > - fscrypt_warn(NULL, > - "Missing crypto API support for SHA-256"); > - return -ENOPKG; > - } > - fscrypt_err(NULL, > - "Error allocating SHA-256 transform: %ld", > - PTR_ERR(tfm)); > - return PTR_ERR(tfm); > - } > - prev_tfm = cmpxchg(&essiv_hash_tfm, NULL, tfm); > - if (prev_tfm) { > - crypto_free_shash(tfm); > - tfm = prev_tfm; > - } > - } > - > - { > - SHASH_DESC_ON_STACK(desc, tfm); > - desc->tfm = tfm; > - > - return crypto_shash_digest(desc, key, keysize, salt); > - } > -} > - > -static int init_essiv_generator(struct fscrypt_info *ci, const u8 *raw_key, > - int keysize) > -{ > - int err; > - struct crypto_cipher *essiv_tfm; > - u8 salt[SHA256_DIGEST_SIZE]; > - > - if (WARN_ON(ci->ci_mode->ivsize != AES_BLOCK_SIZE)) > - return -EINVAL; > - > - essiv_tfm = crypto_alloc_cipher("aes", 0, 0); > - if (IS_ERR(essiv_tfm)) > - return PTR_ERR(essiv_tfm); > - > - ci->ci_essiv_tfm = essiv_tfm; > - > - err = derive_essiv_salt(raw_key, keysize, salt); > - if (err) > - goto out; > - > - /* > - * Using SHA256 to derive the salt/key will result in AES-256 being > - * used for IV generation. File contents encryption will still use the > - * configured keysize (AES-128) nevertheless. > - */ > - err = crypto_cipher_setkey(essiv_tfm, salt, sizeof(salt)); > - if (err) > - goto out; > - > -out: > - memzero_explicit(salt, sizeof(salt)); > - return err; > -} > - > -/* Given the per-file key, set up the file's crypto transform object(s) */ > +/* Given the per-file key, set up the file's crypto transform object */ > int fscrypt_set_derived_key(struct fscrypt_info *ci, const u8 *derived_key) > { > - struct fscrypt_mode *mode = ci->ci_mode; > - struct crypto_skcipher *ctfm; > - int err; > - > - ctfm = fscrypt_allocate_skcipher(mode, derived_key, ci->ci_inode); > - if (IS_ERR(ctfm)) > - return PTR_ERR(ctfm); > + struct crypto_skcipher *tfm; > > - ci->ci_ctfm = ctfm; > + tfm = fscrypt_allocate_skcipher(ci->ci_mode, derived_key, ci->ci_inode); > + if (IS_ERR(tfm)) > + return PTR_ERR(tfm); > > - if (mode->needs_essiv) { > - err = init_essiv_generator(ci, derived_key, mode->keysize); > - if (err) { > - fscrypt_warn(ci->ci_inode, > - "Error initializing ESSIV generator: %d", > - err); > - return err; > - } > - } > + ci->ci_ctfm = tfm; > return 0; > } > > @@ -388,13 +302,11 @@ static void put_crypt_info(struct fscrypt_info *ci) > if (!ci) > return; > > - if (ci->ci_direct_key) { > + if (ci->ci_direct_key) > fscrypt_put_direct_key(ci->ci_direct_key); > - } else if ((ci->ci_ctfm != NULL || ci->ci_essiv_tfm != NULL) && > - !fscrypt_is_direct_key_policy(&ci->ci_policy)) { > + else if (ci->ci_ctfm != NULL && > + !fscrypt_is_direct_key_policy(&ci->ci_policy)) > crypto_free_skcipher(ci->ci_ctfm); > - crypto_free_cipher(ci->ci_essiv_tfm); > - } > > key = ci->ci_master_key; > if (key) { > diff --git a/fs/crypto/keysetup_v1.c b/fs/crypto/keysetup_v1.c > index ad1a36c370c3f..5298ef22aa859 100644 > --- a/fs/crypto/keysetup_v1.c > +++ b/fs/crypto/keysetup_v1.c > @@ -270,10 +270,6 @@ static int setup_v1_file_key_direct(struct fscrypt_info *ci, > return -EINVAL; > } > > - /* ESSIV implies 16-byte IVs which implies !DIRECT_KEY */ > - if (WARN_ON(mode->needs_essiv)) > - return -EINVAL; > - > dk = fscrypt_get_direct_key(ci, raw_master_key); > if (IS_ERR(dk)) > return PTR_ERR(dk); > -- > 2.23.0.581.g78d2f28ef7-goog >
On Wed, Oct 09, 2019 at 04:38:40PM -0700, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > Instead of open-coding the calculations for ESSIV handling, use an ESSIV > skcipher which does all of this under the hood. ESSIV was added to the > crypto API in v5.4. > > This is based on a patch from Ard Biesheuvel, but reworked to apply > after all the fscrypt changes that went into v5.4. > > Tested with 'kvm-xfstests -c ext4,f2fs -g encrypt', including the > ciphertext verification tests for v1 and v2 encryption policies. > > Originally-from: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Signed-off-by: Eric Biggers <ebiggers@google.com> > --- Applied to fscrypt.git for 5.5. - Eric
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst index 8a0700af95967..6ec459be3de16 100644 --- a/Documentation/filesystems/fscrypt.rst +++ b/Documentation/filesystems/fscrypt.rst @@ -308,8 +308,9 @@ If unsure, you should use the (AES-256-XTS, AES-256-CTS-CBC) pair. AES-128-CBC was added only for low-powered embedded devices with crypto accelerators such as CAAM or CESA that do not support XTS. To -use AES-128-CBC, CONFIG_CRYPTO_SHA256 (or another SHA-256 -implementation) must be enabled so that ESSIV can be used. +use AES-128-CBC, CONFIG_CRYPTO_ESSIV and CONFIG_CRYPTO_SHA256 (or +another SHA-256 implementation) must be enabled so that ESSIV can be +used. Adiantum is a (primarily) stream cipher-based mode that is fast even on CPUs without dedicated crypto instructions. It's also a true diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c index 32a7ad0098cc2..6bc3e4f1e657e 100644 --- a/fs/crypto/crypto.c +++ b/fs/crypto/crypto.c @@ -27,7 +27,6 @@ #include <linux/ratelimit.h> #include <linux/dcache.h> #include <linux/namei.h> -#include <crypto/aes.h> #include <crypto/skcipher.h> #include "fscrypt_private.h" @@ -143,9 +142,6 @@ void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num, if (fscrypt_is_direct_key_policy(&ci->ci_policy)) memcpy(iv->nonce, ci->ci_nonce, FS_KEY_DERIVATION_NONCE_SIZE); - - if (ci->ci_essiv_tfm != NULL) - crypto_cipher_encrypt_one(ci->ci_essiv_tfm, iv->raw, iv->raw); } /* Encrypt or decrypt a single filesystem block of file contents */ diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h index e84efc01512e4..76c64297ce187 100644 --- a/fs/crypto/fscrypt_private.h +++ b/fs/crypto/fscrypt_private.h @@ -163,12 +163,6 @@ struct fscrypt_info { /* The actual crypto transform used for encryption and decryption */ struct crypto_skcipher *ci_ctfm; - /* - * Cipher for ESSIV IV generation. Only set for CBC contents - * encryption, otherwise is NULL. - */ - struct crypto_cipher *ci_essiv_tfm; - /* * Encryption mode used for this inode. It corresponds to either the * contents or filenames encryption mode, depending on the inode type. @@ -444,7 +438,6 @@ struct fscrypt_mode { int keysize; int ivsize; bool logged_impl_name; - bool needs_essiv; }; static inline bool diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c index d71c2d6dd162a..8eb5a0e762ec6 100644 --- a/fs/crypto/keysetup.c +++ b/fs/crypto/keysetup.c @@ -8,15 +8,11 @@ * Heavily modified since then. */ -#include <crypto/aes.h> -#include <crypto/sha.h> #include <crypto/skcipher.h> #include <linux/key.h> #include "fscrypt_private.h" -static struct crypto_shash *essiv_hash_tfm; - static struct fscrypt_mode available_modes[] = { [FSCRYPT_MODE_AES_256_XTS] = { .friendly_name = "AES-256-XTS", @@ -31,11 +27,10 @@ static struct fscrypt_mode available_modes[] = { .ivsize = 16, }, [FSCRYPT_MODE_AES_128_CBC] = { - .friendly_name = "AES-128-CBC", - .cipher_str = "cbc(aes)", + .friendly_name = "AES-128-CBC-ESSIV", + .cipher_str = "essiv(cbc(aes),sha256)", .keysize = 16, .ivsize = 16, - .needs_essiv = true, }, [FSCRYPT_MODE_AES_128_CTS] = { .friendly_name = "AES-128-CTS-CBC", @@ -111,97 +106,16 @@ struct crypto_skcipher *fscrypt_allocate_skcipher(struct fscrypt_mode *mode, return ERR_PTR(err); } -static int derive_essiv_salt(const u8 *key, int keysize, u8 *salt) -{ - struct crypto_shash *tfm = READ_ONCE(essiv_hash_tfm); - - /* init hash transform on demand */ - if (unlikely(!tfm)) { - struct crypto_shash *prev_tfm; - - tfm = crypto_alloc_shash("sha256", 0, 0); - if (IS_ERR(tfm)) { - if (PTR_ERR(tfm) == -ENOENT) { - fscrypt_warn(NULL, - "Missing crypto API support for SHA-256"); - return -ENOPKG; - } - fscrypt_err(NULL, - "Error allocating SHA-256 transform: %ld", - PTR_ERR(tfm)); - return PTR_ERR(tfm); - } - prev_tfm = cmpxchg(&essiv_hash_tfm, NULL, tfm); - if (prev_tfm) { - crypto_free_shash(tfm); - tfm = prev_tfm; - } - } - - { - SHASH_DESC_ON_STACK(desc, tfm); - desc->tfm = tfm; - - return crypto_shash_digest(desc, key, keysize, salt); - } -} - -static int init_essiv_generator(struct fscrypt_info *ci, const u8 *raw_key, - int keysize) -{ - int err; - struct crypto_cipher *essiv_tfm; - u8 salt[SHA256_DIGEST_SIZE]; - - if (WARN_ON(ci->ci_mode->ivsize != AES_BLOCK_SIZE)) - return -EINVAL; - - essiv_tfm = crypto_alloc_cipher("aes", 0, 0); - if (IS_ERR(essiv_tfm)) - return PTR_ERR(essiv_tfm); - - ci->ci_essiv_tfm = essiv_tfm; - - err = derive_essiv_salt(raw_key, keysize, salt); - if (err) - goto out; - - /* - * Using SHA256 to derive the salt/key will result in AES-256 being - * used for IV generation. File contents encryption will still use the - * configured keysize (AES-128) nevertheless. - */ - err = crypto_cipher_setkey(essiv_tfm, salt, sizeof(salt)); - if (err) - goto out; - -out: - memzero_explicit(salt, sizeof(salt)); - return err; -} - -/* Given the per-file key, set up the file's crypto transform object(s) */ +/* Given the per-file key, set up the file's crypto transform object */ int fscrypt_set_derived_key(struct fscrypt_info *ci, const u8 *derived_key) { - struct fscrypt_mode *mode = ci->ci_mode; - struct crypto_skcipher *ctfm; - int err; - - ctfm = fscrypt_allocate_skcipher(mode, derived_key, ci->ci_inode); - if (IS_ERR(ctfm)) - return PTR_ERR(ctfm); + struct crypto_skcipher *tfm; - ci->ci_ctfm = ctfm; + tfm = fscrypt_allocate_skcipher(ci->ci_mode, derived_key, ci->ci_inode); + if (IS_ERR(tfm)) + return PTR_ERR(tfm); - if (mode->needs_essiv) { - err = init_essiv_generator(ci, derived_key, mode->keysize); - if (err) { - fscrypt_warn(ci->ci_inode, - "Error initializing ESSIV generator: %d", - err); - return err; - } - } + ci->ci_ctfm = tfm; return 0; } @@ -388,13 +302,11 @@ static void put_crypt_info(struct fscrypt_info *ci) if (!ci) return; - if (ci->ci_direct_key) { + if (ci->ci_direct_key) fscrypt_put_direct_key(ci->ci_direct_key); - } else if ((ci->ci_ctfm != NULL || ci->ci_essiv_tfm != NULL) && - !fscrypt_is_direct_key_policy(&ci->ci_policy)) { + else if (ci->ci_ctfm != NULL && + !fscrypt_is_direct_key_policy(&ci->ci_policy)) crypto_free_skcipher(ci->ci_ctfm); - crypto_free_cipher(ci->ci_essiv_tfm); - } key = ci->ci_master_key; if (key) { diff --git a/fs/crypto/keysetup_v1.c b/fs/crypto/keysetup_v1.c index ad1a36c370c3f..5298ef22aa859 100644 --- a/fs/crypto/keysetup_v1.c +++ b/fs/crypto/keysetup_v1.c @@ -270,10 +270,6 @@ static int setup_v1_file_key_direct(struct fscrypt_info *ci, return -EINVAL; } - /* ESSIV implies 16-byte IVs which implies !DIRECT_KEY */ - if (WARN_ON(mode->needs_essiv)) - return -EINVAL; - dk = fscrypt_get_direct_key(ci, raw_master_key); if (IS_ERR(dk)) return PTR_ERR(dk);