1. 18 Jan, 2019 23 commits
    • Eric Biggers's avatar
      crypto: testmgr - unify the AEAD encryption and decryption test vectors · a0d608ee
      Eric Biggers authored
      Currently testmgr has separate encryption and decryption test vectors
      for AEADs.  That's massively redundant, since usually the decryption
      tests are identical to the encryption tests, just with the input/result
      swapped.  And for some algorithms it was forgotten to add decryption
      test vectors, so for them currently only encryption is being tested.
      
      Therefore, eliminate the redundancy by removing the AEAD decryption test
      vectors and updating testmgr to test both AEAD encryption and decryption
      using what used to be the encryption test vectors.  Naming is adjusted
      accordingly: each aead_testvec now has a 'ptext' (plaintext), 'plen'
      (plaintext length), 'ctext' (ciphertext), and 'clen' (ciphertext length)
      instead of an 'input', 'ilen', 'result', and 'rlen'.  "Ciphertext" here
      refers to the full ciphertext, including the authentication tag.
      
      For now the scatterlist divisions are just given for the plaintext
      length, not also the ciphertext length.  For decryption, the last
      scatterlist element is just extended by the authentication tag length.
      
      In total, this removes over 5000 lines from testmgr.h, with no reduction
      in test coverage since prior patches already copied the few unique
      decryption test vectors into the encryption test vectors.
      
      The testmgr.h portion of this patch was automatically generated using
      the following awk script, except that I also manually updated the
      definition of 'struct aead_testvec' and fixed the location of the
      comment describing the AEGIS-128 test vectors.
      
          BEGIN { OTHER = 0; ENCVEC = 1; DECVEC = 2; DECVEC_TAIL = 3; mode = OTHER }
      
          /^static const struct aead_testvec.*_enc_/ { sub("_enc", ""); mode = ENCVEC }
          /^static const struct aead_testvec.*_dec_/ { mode = DECVEC }
          mode == ENCVEC {
              sub(/\.input[[:space:]]*=/,     ".ptext\t=")
              sub(/\.result[[:space:]]*=/,    ".ctext\t=")
              sub(/\.ilen[[:space:]]*=/,      ".plen\t=")
              sub(/\.rlen[[:space:]]*=/,      ".clen\t=")
              print
          }
          mode == DECVEC_TAIL && /[^[:space:]]/ { mode = OTHER }
          mode == OTHER                         { print }
          mode == ENCVEC && /^};/               { mode = OTHER }
          mode == DECVEC && /^};/               { mode = DECVEC_TAIL }
      
      Note that git's default diff algorithm gets confused by the testmgr.h
      portion of this patch, and reports too many lines added and removed.
      It's better viewed with 'git diff --minimal' (or 'git show --minimal'),
      which reports "2 files changed, 1235 insertions(+), 6491 deletions(-)".
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      a0d608ee
    • Eric Biggers's avatar
      crypto: testmgr - add rfc4543(gcm(aes)) decryption test to encryption tests · d7250b41
      Eric Biggers authored
      One "rfc4543(gcm(aes))" decryption test vector doesn't exactly match any of the
      encryption test vectors with input and result swapped.  In preparation
      for removing the AEAD decryption test vectors and testing AEAD
      decryption using the encryption test vectors, add this to the encryption
      test vectors, so we don't lose any test coverage.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      d7250b41
    • Eric Biggers's avatar
      crypto: testmgr - add gcm(aes) decryption tests to encryption tests · f38e8885
      Eric Biggers authored
      Some "gcm(aes)" decryption test vectors don't exactly match any of the
      encryption test vectors with input and result swapped.  In preparation
      for removing the AEAD decryption test vectors and testing AEAD
      decryption using the encryption test vectors, add these to the
      encryption test vectors, so we don't lose any test coverage.
      
      In the case of the chunked test vector, I truncated the last scatterlist
      element to the end of the plaintext.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      f38e8885
    • Eric Biggers's avatar
      crypto: testmgr - add ccm(aes) decryption tests to encryption tests · de845da9
      Eric Biggers authored
      Some "ccm(aes)" decryption test vectors don't exactly match any of the
      encryption test vectors with input and result swapped.  In preparation
      for removing the AEAD decryption test vectors and testing AEAD
      decryption using the encryption test vectors, add these to the
      encryption test vectors, so we don't lose any test coverage.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      de845da9
    • Eric Biggers's avatar
      crypto: testmgr - skip AEAD encryption test vectors with novrfy set · 5bc3de58
      Eric Biggers authored
      In preparation for unifying the AEAD encryption and decryption test
      vectors, skip AEAD test vectors with the 'novrfy' (verification failure
      expected) flag set when testing encryption rather than decryption.
      These test vectors only make sense for decryption.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      5bc3de58
    • Eric Biggers's avatar
      crypto: af_alg - remove redundant initializations of sk_family · 6d0d6cfb
      Eric Biggers authored
      sk_alloc() already sets sock::sk_family to PF_ALG which is passed as the
      'family' argument, so there's no need to set it again.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      6d0d6cfb
    • Eric Biggers's avatar
      crypto: af_alg - use list_for_each_entry() in af_alg_count_tsgl() · 7c39edfb
      Eric Biggers authored
      af_alg_count_tsgl() iterates through a list without modifying it, so use
      list_for_each_entry() rather than list_for_each_entry_safe().  Also make
      the pointers 'const' to make it clearer that nothing is modified.
      
      No actual change in behavior.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      7c39edfb
    • Eric Biggers's avatar
      crypto: af_alg - make some functions static · 466e0759
      Eric Biggers authored
      Some exported functions in af_alg.c aren't used outside of that file.
      Therefore, un-export them and make them 'static'.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      466e0759
    • Eric Biggers's avatar
      crypto: stat - remove unused mutex · 554557ce
      Eric Biggers authored
      crypto_cfg_mutex in crypto_user_stat.c is unused.  Remove it.
      
      Cc: Corentin Labbe <clabbe@baylibre.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      554557ce
    • Eric Biggers's avatar
      crypto: tgr192 - fix unaligned memory access · f990f7fb
      Eric Biggers authored
      Fix an unaligned memory access in tgr192_transform() by using the
      unaligned access helpers.
      
      Fixes: 06ace7a9 ("[CRYPTO] Use standard byte order macros wherever possible")
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      f990f7fb
    • Eric Biggers's avatar
      crypto: x86/aesni-gcm - make 'struct aesni_gcm_tfm_s' static const · 793ff5ff
      Eric Biggers authored
      Add missing static keywords to fix the following sparse warnings:
      
          arch/x86/crypto/aesni-intel_glue.c:197:24: warning: symbol 'aesni_gcm_tfm_sse' was not declared. Should it be static?
          arch/x86/crypto/aesni-intel_glue.c:246:24: warning: symbol 'aesni_gcm_tfm_avx_gen2' was not declared. Should it be static?
          arch/x86/crypto/aesni-intel_glue.c:291:24: warning: symbol 'aesni_gcm_tfm_avx_gen4' was not declared. Should it be static?
      
      I also made the affected structures 'const', and adjusted the
      indentation in the struct definition to not be insane.
      
      Cc: Dave Watson <davejwatson@fb.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      793ff5ff
    • Eric Biggers's avatar
      crypto: user - forward declare crypto_nlsk · e17568e1
      Eric Biggers authored
      Move the declaration of crypto_nlsk into internal/cryptouser.h.  This
      fixes the following sparse warning:
      
          crypto/crypto_user_base.c:41:13: warning: symbol 'crypto_nlsk' was not declared. Should it be static?
      
      Cc: Corentin Labbe <clabbe@baylibre.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      e17568e1
    • Eric Biggers's avatar
      crypto: testmgr - handle endianness correctly in alg_test_crc32c() · cb9dde88
      Eric Biggers authored
      The crc32c context is in CPU endianness, whereas the final digest is
      little endian.  alg_test_crc32c() got this mixed up.  Fix it.
      
      The test passes both before and after, but this patch fixes the
      following sparse warning:
      
          crypto/testmgr.c:1912:24: warning: cast to restricted __le32
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      cb9dde88
    • Eric Biggers's avatar
      crypto: streebog - use correct endianness type · 73381da5
      Eric Biggers authored
      streebog_uint512::qword needs to be __le64, not u64.  This fixes a large
      number of sparse warnings:
      
          crypto/streebog_generic.c:25:9: warning: incorrect type in initializer (different base types)
          crypto/streebog_generic.c:25:9:    expected unsigned long long
          crypto/streebog_generic.c:25:9:    got restricted __le64 [usertype]
          [omitted many similar warnings]
      
      No actual change in behavior.
      
      Cc: Vitaly Chikunov <vt@altlinux.org>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      73381da5
    • Eric Biggers's avatar
      crypto: rsa-pkcs1pad - include <crypto/internal/rsa.h> · a1180cff
      Eric Biggers authored
      Include internal/rsa.h in rsa-pkcs1pad.c to get the declaration of
      rsa_pkcs1pad_tmpl.  This fixes the following sparse warning:
      
          crypto/rsa-pkcs1pad.c:698:24: warning: symbol 'rsa_pkcs1pad_tmpl' was not declared. Should it be static?
      
      Cc: Andrzej Zaborowski <andrew.zaborowski@intel.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      a1180cff
    • Eric Biggers's avatar
      crypto: gcm - use correct endianness type in gcm_hash_len() · 18666550
      Eric Biggers authored
      In gcm_hash_len(), use be128 rather than u128.  This fixes the following
      sparse warnings:
      
          crypto/gcm.c:252:19: warning: incorrect type in assignment (different base types)
          crypto/gcm.c:252:19:    expected unsigned long long [usertype] a
          crypto/gcm.c:252:19:    got restricted __be64 [usertype]
          crypto/gcm.c:253:19: warning: incorrect type in assignment (different base types)
          crypto/gcm.c:253:19:    expected unsigned long long [usertype] b
          crypto/gcm.c:253:19:    got restricted __be64 [usertype]
      
      No actual change in behavior.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      18666550
    • YueHaibing's avatar
      crypto: brcm - Fix some set-but-not-used warning · 707d0cf8
      YueHaibing authored
      Fixes gcc '-Wunused-but-set-variable' warning:
      
      drivers/crypto/bcm/cipher.c: In function 'handle_ahash_req':
      drivers/crypto/bcm/cipher.c:720:15: warning:
       variable 'chunk_start' set but not used [-Wunused-but-set-variable]
      
      drivers/crypto/bcm/cipher.c: In function 'spu_rx_callback':
      drivers/crypto/bcm/cipher.c:1679:31: warning:
       variable 'areq' set but not used [-Wunused-but-set-variable]
      
      drivers/crypto/bcm/cipher.c:1678:22: warning:
       variable 'ctx' set but not used [-Wunused-but-set-variable]
      
      Fixes: 9d12ba86 ("crypto: brcm - Add Broadcom SPU driver")
      Signed-off-by: default avatarYueHaibing <yuehaibing@huawei.com>
      Reviewed-by: default avatarRaveendra Padasalagi <raveendra.padasalagi@broadcom.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      707d0cf8
    • Vitaly Chikunov's avatar
      crypto: testmgr - split akcipher tests by a key type · 0507de94
      Vitaly Chikunov authored
      Before this, if akcipher_testvec have `public_key_vec' set to true
      (i.e. having a public key) only sign/encrypt test is performed, but
      verify/decrypt test is skipped.
      
      With a public key we could do encrypt and verify, but to sign and decrypt
      a private key is required.
      
      This logic is correct for encrypt/decrypt tests (decrypt is skipped if
      no private key). But incorrect for sign/verify tests - sign is performed
      no matter if there is no private key, but verify is skipped if there is
      a public key.
      
      Rework `test_akcipher_one' to arrange tests properly depending on value
      of `public_key_vec` and `siggen_sigver_test'.
      
      No tests were missed since there is only one sign/verify test (which
      have `siggen_sigver_test' set to true) and it has a private key, but
      future tests could benefit from this improvement.
      Signed-off-by: default avatarVitaly Chikunov <vt@altlinux.org>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      0507de94
    • Eric Biggers's avatar
      crypto: shash - remove pointless checks of shash_alg::{export,import} · 2b091e32
      Eric Biggers authored
      crypto_init_shash_ops_async() only gives the ahash tfm non-NULL
      ->export() and ->import() if the underlying shash alg has these
      non-NULL.  This doesn't make sense because when an shash algorithm is
      registered, shash_prepare_alg() sets a default ->export() and ->import()
      if the implementor didn't provide them.  And elsewhere it's assumed that
      all shash algs and ahash tfms have non-NULL ->export() and ->import().
      
      Therefore, remove these unnecessary, always-true conditions.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      2b091e32
    • Eric Biggers's avatar
      crypto: shash - require neither or both ->export() and ->import() · 41a2e94f
      Eric Biggers authored
      Prevent registering shash algorithms that implement ->export() but not
      ->import(), or ->import() but not ->export().  Such cases don't make
      sense and could confuse the check that shash_prepare_alg() does for just
      ->export().
      
      I don't believe this affects any existing algorithms; this is just
      preventing future mistakes.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      41a2e94f
    • Eric Biggers's avatar
      crypto: aead - set CRYPTO_TFM_NEED_KEY if ->setkey() fails · 6ebc9700
      Eric Biggers authored
      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.
      
      For example, in gcm.c, if the kzalloc() fails due to lack of memory,
      then the CTR part of GCM will have the new key but GHASH will not.
      
      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, to prevent the tfm from being
      used until a new key is set.
      
      [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: dc26c17f ("crypto: aead - prevent using AEADs without setting key")
      Cc: <stable@vger.kernel.org> # v4.16+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      6ebc9700
    • Eric Biggers's avatar
      crypto: skcipher - set CRYPTO_TFM_NEED_KEY if ->setkey() fails · b1f6b4bf
      Eric Biggers authored
      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.
      
      For example, in lrw.c, if gf128mul_init_64k_bbe() fails due to lack of
      memory, then priv::table will be left NULL.  After that, encryption with
      that tfm will cause a NULL pointer dereference.
      
      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.
      
      [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: f8d33fac ("crypto: skcipher - prevent using skciphers without setting key")
      Cc: <stable@vger.kernel.org> # v4.16+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      b1f6b4bf
    • Eric Biggers's avatar
      crypto: hash - set CRYPTO_TFM_NEED_KEY if ->setkey() fails · ba7d7433
      Eric Biggers authored
      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: 9fa68f62 ("crypto: hash - prevent using keyed hashes without setting key")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      ba7d7433
  2. 11 Jan, 2019 17 commits