3.16.70-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Eric Biggers <ebigg...@google.com>

commit ba7d7433a0e998c902132bd47330e355a1eaa894 upstream.

Some algorithms have a ->setkey() method that is not atomic, in the
sense that setting a key can fail after changes were already made to the
tfm context.  In this case, if a key was already set the tfm can end up
in a state that corresponds to neither the old key nor the new key.

It's not feasible to make all ->setkey() methods atomic, especially ones
that have to key multiple sub-tfms.  Therefore, make the crypto API set
CRYPTO_TFM_NEED_KEY if ->setkey() fails and the algorithm requires a
key, to prevent the tfm from being used until a new key is set.

Note: we can't set CRYPTO_TFM_NEED_KEY for OPTIONAL_KEY algorithms, so
->setkey() for those must nevertheless be atomic.  That's fine for now
since only the crc32 and crc32c algorithms set OPTIONAL_KEY, and it's
not intended that OPTIONAL_KEY be used much.

[Cc stable mainly because when introducing the NEED_KEY flag I changed
 AF_ALG to rely on it; and unlike in-kernel crypto API users, AF_ALG
 previously didn't have this problem.  So these "incompletely keyed"
 states became theoretically accessible via AF_ALG -- though, the
 opportunities for causing real mischief seem pretty limited.]

Fixes: 9fa68f620041 ("crypto: hash - prevent using keyed hashes without setting 
key")
Signed-off-by: Eric Biggers <ebigg...@google.com>
Signed-off-by: Herbert Xu <herb...@gondor.apana.org.au>
Signed-off-by: Ben Hutchings <b...@decadent.org.uk>
---
 crypto/ahash.c | 28 +++++++++++++++++++---------
 crypto/shash.c | 18 +++++++++++++-----
 2 files changed, 32 insertions(+), 14 deletions(-)

--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -200,6 +200,21 @@ static int ahash_setkey_unaligned(struct
        return ret;
 }
 
+static int ahash_nosetkey(struct crypto_ahash *tfm, const u8 *key,
+                         unsigned int keylen)
+{
+       return -ENOSYS;
+}
+
+static void ahash_set_needkey(struct crypto_ahash *tfm)
+{
+       const struct hash_alg_common *alg = crypto_hash_alg_common(tfm);
+
+       if (tfm->setkey != ahash_nosetkey &&
+           !(alg->base.cra_flags & CRYPTO_ALG_OPTIONAL_KEY))
+               crypto_ahash_set_flags(tfm, CRYPTO_TFM_NEED_KEY);
+}
+
 int crypto_ahash_setkey(struct crypto_ahash *tfm, const u8 *key,
                        unsigned int keylen)
 {
@@ -211,20 +226,16 @@ int crypto_ahash_setkey(struct crypto_ah
        else
                err = tfm->setkey(tfm, key, keylen);
 
-       if (err)
+       if (unlikely(err)) {
+               ahash_set_needkey(tfm);
                return err;
+       }
 
        crypto_ahash_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
        return 0;
 }
 EXPORT_SYMBOL_GPL(crypto_ahash_setkey);
 
-static int ahash_nosetkey(struct crypto_ahash *tfm, const u8 *key,
-                         unsigned int keylen)
-{
-       return -ENOSYS;
-}
-
 static inline unsigned int ahash_align_buffer_size(unsigned len,
                                                   unsigned long mask)
 {
@@ -493,8 +504,7 @@ static int crypto_ahash_init_tfm(struct
 
        if (alg->setkey) {
                hash->setkey = alg->setkey;
-               if (!(alg->halg.base.cra_flags & CRYPTO_ALG_OPTIONAL_KEY))
-                       crypto_ahash_set_flags(hash, CRYPTO_TFM_NEED_KEY);
+               ahash_set_needkey(hash);
        }
        if (alg->export)
                hash->export = alg->export;
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -52,6 +52,13 @@ static int shash_setkey_unaligned(struct
        return err;
 }
 
+static void shash_set_needkey(struct crypto_shash *tfm, struct shash_alg *alg)
+{
+       if (crypto_shash_alg_has_setkey(alg) &&
+           !(alg->base.cra_flags & CRYPTO_ALG_OPTIONAL_KEY))
+               crypto_shash_set_flags(tfm, CRYPTO_TFM_NEED_KEY);
+}
+
 int crypto_shash_setkey(struct crypto_shash *tfm, const u8 *key,
                        unsigned int keylen)
 {
@@ -64,8 +71,10 @@ int crypto_shash_setkey(struct crypto_sh
        else
                err = shash->setkey(tfm, key, keylen);
 
-       if (err)
+       if (unlikely(err)) {
+               shash_set_needkey(tfm, shash);
                return err;
+       }
 
        crypto_shash_clear_flags(tfm, CRYPTO_TFM_NEED_KEY);
        return 0;
@@ -367,7 +376,8 @@ int crypto_init_shash_ops_async(struct c
        crt->final = shash_async_final;
        crt->finup = shash_async_finup;
        crt->digest = shash_async_digest;
-       crt->setkey = shash_async_setkey;
+       if (crypto_shash_alg_has_setkey(alg))
+               crt->setkey = shash_async_setkey;
 
        crypto_ahash_set_flags(crt, crypto_shash_get_flags(shash) &
                                    CRYPTO_TFM_NEED_KEY);
@@ -534,9 +544,7 @@ static int crypto_shash_init_tfm(struct
 
        hash->descsize = alg->descsize;
 
-       if (crypto_shash_alg_has_setkey(alg) &&
-           !(alg->base.cra_flags & CRYPTO_ALG_OPTIONAL_KEY))
-               crypto_shash_set_flags(hash, CRYPTO_TFM_NEED_KEY);
+       shash_set_needkey(hash, alg);
 
        return 0;
 }

Reply via email to