1. 18 Jan, 2019 6 commits
    • 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 30 commits
  3. 10 Jan, 2019 4 commits
    • Eric Biggers's avatar
      crypto: sm3 - fix undefined shift by >= width of value · d45a90cb
      Eric Biggers authored
      sm3_compress() calls rol32() with shift >= 32, which causes undefined
      behavior.  This is easily detected by enabling CONFIG_UBSAN.
      
      Explicitly AND with 31 to make the behavior well defined.
      
      Fixes: 4f0fc160 ("crypto: sm3 - add OSCCA SM3 secure hash")
      Cc: <stable@vger.kernel.org> # v4.15+
      Cc: Gilad Ben-Yossef <gilad@benyossef.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      d45a90cb
    • Christophe Leroy's avatar
      crypto: talitos - fix ablkcipher for CONFIG_VMAP_STACK · 1bea445b
      Christophe Leroy authored
      [    2.364486] WARNING: CPU: 0 PID: 60 at ./arch/powerpc/include/asm/io.h:837 dma_nommu_map_page+0x44/0xd4
      [    2.373579] CPU: 0 PID: 60 Comm: cryptomgr_test Tainted: G        W         4.20.0-rc5-00560-g6bfb52e23a00-dirty #531
      [    2.384740] NIP:  c000c540 LR: c000c584 CTR: 00000000
      [    2.389743] REGS: c95abab0 TRAP: 0700   Tainted: G        W          (4.20.0-rc5-00560-g6bfb52e23a00-dirty)
      [    2.400042] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 24042204  XER: 00000000
      [    2.406669]
      [    2.406669] GPR00: c02f2244 c95abb60 c6262990 c95abd80 0000256a 00000001 00000001 00000001
      [    2.406669] GPR08: 00000000 00002000 00000010 00000010 24042202 00000000 00000100 c95abd88
      [    2.406669] GPR16: 00000000 c05569d4 00000001 00000010 c95abc88 c0615664 00000004 00000000
      [    2.406669] GPR24: 00000010 c95abc88 c95abc88 00000000 c61ae210 c7ff6d40 c61ae210 00003d68
      [    2.441559] NIP [c000c540] dma_nommu_map_page+0x44/0xd4
      [    2.446720] LR [c000c584] dma_nommu_map_page+0x88/0xd4
      [    2.451762] Call Trace:
      [    2.454195] [c95abb60] [82000808] 0x82000808 (unreliable)
      [    2.459572] [c95abb80] [c02f2244] talitos_edesc_alloc+0xbc/0x3c8
      [    2.465493] [c95abbb0] [c02f2600] ablkcipher_edesc_alloc+0x4c/0x5c
      [    2.471606] [c95abbd0] [c02f4ed0] ablkcipher_encrypt+0x20/0x64
      [    2.477389] [c95abbe0] [c02023b0] __test_skcipher+0x4bc/0xa08
      [    2.483049] [c95abe00] [c0204b60] test_skcipher+0x2c/0xcc
      [    2.488385] [c95abe20] [c0204c48] alg_test_skcipher+0x48/0xbc
      [    2.494064] [c95abe40] [c0205cec] alg_test+0x164/0x2e8
      [    2.499142] [c95abf00] [c0200dec] cryptomgr_test+0x48/0x50
      [    2.504558] [c95abf10] [c0039ff4] kthread+0xe4/0x110
      [    2.509471] [c95abf40] [c000e1d0] ret_from_kernel_thread+0x14/0x1c
      [    2.515532] Instruction dump:
      [    2.518468] 7c7e1b78 7c9d2378 7cbf2b78 41820054 3d20c076 8089c200 3d20c076 7c84e850
      [    2.526127] 8129c204 7c842e70 7f844840 419c0008 <0fe00000> 2f9e0000 54847022 7c84fa14
      [    2.533960] ---[ end trace bf78d94af73fe3b8 ]---
      [    2.539123] talitos ff020000.crypto: master data transfer error
      [    2.544775] talitos ff020000.crypto: TEA error: ISR 0x20000000_00000040
      [    2.551625] alg: skcipher: encryption failed on test 1 for ecb-aes-talitos: ret=22
      
      IV cannot be on stack when CONFIG_VMAP_STACK is selected because the stack
      cannot be DMA mapped anymore.
      
      This patch copies the IV into the extended descriptor.
      
      Fixes: 4de9d0b5 ("crypto: talitos - Add ablkcipher algorithms")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarChristophe Leroy <christophe.leroy@c-s.fr>
      Reviewed-by: default avatarHoria Geantă <horia.geanta@nxp.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      1bea445b
    • Christophe Leroy's avatar
      crypto: talitos - reorder code in talitos_edesc_alloc() · c56c2e17
      Christophe Leroy authored
      This patch moves the mapping of IV after the kmalloc(). This
      avoids having to unmap in case kmalloc() fails.
      Signed-off-by: default avatarChristophe Leroy <christophe.leroy@c-s.fr>
      Reviewed-by: default avatarHoria Geantă <horia.geanta@nxp.com>
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      c56c2e17
    • Eric Biggers's avatar
      crypto: adiantum - initialize crypto_spawn::inst · 6db43410
      Eric Biggers authored
      crypto_grab_*() doesn't set crypto_spawn::inst, so templates must set it
      beforehand.  Otherwise it will be left NULL, which causes a crash in
      certain cases where algorithms are dynamically loaded/unloaded.  E.g.
      with CONFIG_CRYPTO_CHACHA20_X86_64=m, the following caused a crash:
      
          insmod chacha-x86_64.ko
          python -c 'import socket; socket.socket(socket.AF_ALG, 5, 0).bind(("skcipher", "adiantum(xchacha12,aes)"))'
          rmmod chacha-x86_64.ko
          python -c 'import socket; socket.socket(socket.AF_ALG, 5, 0).bind(("skcipher", "adiantum(xchacha12,aes)"))'
      
      Fixes: 059c2a4d ("crypto: adiantum - add Adiantum support")
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      6db43410