1. 12 Jan, 2018 17 commits
    • Eric Biggers's avatar
      crypto: salsa20-generic - cleanup and convert to skcipher API · b62b3db7
      Eric Biggers authored
      Convert salsa20-generic from the deprecated "blkcipher" API to the
      "skcipher" API, in the process fixing it up to be thread-safe (as the
      crypto API expects) by maintaining each request's state separately from
      the transform context.
      
      Also remove the unnecessary cra_alignmask and tighten validation of the
      key size by accepting only 16 or 32 bytes, not anything in between.
      
      These changes bring the code close to the way chacha20-generic does
      things, so hopefully it will be easier to maintain in the future.
      
      However, the way Salsa20 interprets the IV is still slightly different;
      that was not changed.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      b62b3db7
    • Arnd Bergmann's avatar
      crypto: aes-generic - build with -Os on gcc-7+ · 148b974d
      Arnd Bergmann authored
      While testing other changes, I discovered that gcc-7.2.1 produces badly
      optimized code for aes_encrypt/aes_decrypt. This is especially true when
      CONFIG_UBSAN_SANITIZE_ALL is enabled, where it leads to extremely
      large stack usage that in turn might cause kernel stack overflows:
      
      crypto/aes_generic.c: In function 'aes_encrypt':
      crypto/aes_generic.c:1371:1: warning: the frame size of 4880 bytes is larger than 2048 bytes [-Wframe-larger-than=]
      crypto/aes_generic.c: In function 'aes_decrypt':
      crypto/aes_generic.c:1441:1: warning: the frame size of 4864 bytes is larger than 2048 bytes [-Wframe-larger-than=]
      
      I verified that this problem exists on all architectures that are
      supported by gcc-7.2, though arm64 in particular is less affected than
      the others. I also found that gcc-7.1 and gcc-8 do not show the extreme
      stack usage but still produce worse code than earlier versions for this
      file, apparently because of optimization passes that generally provide
      a substantial improvement in object code quality but understandably fail
      to find any shortcuts in the AES algorithm.
      
      Possible workarounds include
      
      a) disabling -ftree-pre and -ftree-sra optimizations, this was an earlier
         patch I tried, which reliably fixed the stack usage, but caused a
         serious performance regression in some versions, as later testing
         found.
      
      b) disabling UBSAN on this file or all ciphers, as suggested by Ard
         Biesheuvel. This would lead to massively better crypto performance in
         UBSAN-enabled kernels and avoid the stack usage, but there is a concern
         over whether we should exclude arbitrary files from UBSAN at all.
      
      c) Forcing the optimization level in a different way. Similar to a),
         but rather than deselecting specific optimization stages,
         this now uses "gcc -Os" for this file, regardless of the
         CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE/SIZE option. This is a reliable
         workaround for the stack consumption on all architecture, and I've
         retested the performance results now on x86, cycles/byte (lower is
         better) for cbc(aes-generic) with 256 bit keys:
      
      			-O2     -Os
      	gcc-6.3.1	14.9	15.1
      	gcc-7.0.1	14.7	15.3
      	gcc-7.1.1	15.3	14.7
      	gcc-7.2.1	16.8	15.9
      	gcc-8.0.0	15.5	15.6
      
      This implements the option c) by enabling forcing -Os on all compiler
      versions starting with gcc-7.1. As a workaround for PR83356, it would
      only be needed for gcc-7.2+ with UBSAN enabled, but since it also shows
      better performance on gcc-7.1 without UBSAN, it seems appropriate to
      use the faster version here as well.
      
      Side note: during testing, I also played with the AES code in libressl,
      which had a similar performance regression from gcc-6 to gcc-7.2,
      but was three times slower overall. It might be interesting to
      investigate that further and possibly port the Linux implementation
      into that.
      
      Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83356
      Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83651
      Cc: Richard Biener <rguenther@suse.de>
      Cc: Jakub Jelinek <jakub@gcc.gnu.org>
      Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Acked-by: default avatarArd Biesheuvel <ard.biesheuvel@linaro.org>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      148b974d
    • Eric Biggers's avatar
      crypto: aead - prevent using AEADs without setting key · dc26c17f
      Eric Biggers authored
      Similar to what was done for the hash API, update the AEAD API to track
      whether each transform has been keyed, and reject encryption/decryption
      if a key is needed but one hasn't been set.
      
      This isn't quite as important as the equivalent fix for the hash API
      because AEADs always require a key, so are unlikely to be used without
      one.  Still, tracking the key will prevent accidental unkeyed use.
      algif_aead also had to track the key anyway, so the new flag replaces
      that and slightly simplifies the algif_aead implementation.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      dc26c17f
    • Eric Biggers's avatar
      crypto: skcipher - prevent using skciphers without setting key · f8d33fac
      Eric Biggers authored
      Similar to what was done for the hash API, update the skcipher API to
      track whether each transform has been keyed, and reject
      encryption/decryption if a key is needed but one hasn't been set.
      
      This isn't as important as the equivalent fix for the hash API because
      symmetric ciphers almost always require a key (the "null cipher" is the
      only exception), so are unlikely to be used without one.  Still,
      tracking the key will prevent accidental unkeyed use.  algif_skcipher
      also had to track the key anyway, so the new flag replaces that and
      simplifies the algif_skcipher implementation.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      f8d33fac
    • Eric Biggers's avatar
      crypto: ghash - remove checks for key being set · 4e1d14bc
      Eric Biggers authored
      Now that the crypto API prevents a keyed hash from being used without
      setting the key, there's no need for GHASH to do this check itself.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      4e1d14bc
    • Eric Biggers's avatar
      crypto: hash - prevent using keyed hashes without setting key · 9fa68f62
      Eric Biggers authored
      Currently, almost none of the keyed hash algorithms check whether a key
      has been set before proceeding.  Some algorithms are okay with this and
      will effectively just use a key of all 0's or some other bogus default.
      However, others will severely break, as demonstrated using
      "hmac(sha3-512-generic)", the unkeyed use of which causes a kernel crash
      via a (potentially exploitable) stack buffer overflow.
      
      A while ago, this problem was solved for AF_ALG by pairing each hash
      transform with a 'has_key' bool.  However, there are still other places
      in the kernel where userspace can specify an arbitrary hash algorithm by
      name, and the kernel uses it as unkeyed hash without checking whether it
      is really unkeyed.  Examples of this include:
      
          - KEYCTL_DH_COMPUTE, via the KDF extension
          - dm-verity
          - dm-crypt, via the ESSIV support
          - dm-integrity, via the "internal hash" mode with no key given
          - drbd (Distributed Replicated Block Device)
      
      This bug is especially bad for KEYCTL_DH_COMPUTE as that requires no
      privileges to call.
      
      Fix the bug for all users by adding a flag CRYPTO_TFM_NEED_KEY to the
      ->crt_flags of each hash transform that indicates whether the transform
      still needs to be keyed or not.  Then, make the hash init, import, and
      digest functions return -ENOKEY if the key is still needed.
      
      The new flag also replaces the 'has_key' bool which algif_hash was
      previously using, thereby simplifying the algif_hash implementation.
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      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>
      9fa68f62
    • Eric Biggers's avatar
      crypto: hash - annotate algorithms taking optional key · a208fa8f
      Eric Biggers authored
      We need to consistently enforce that keyed hashes cannot be used without
      setting the key.  To do this we need a reliable way to determine whether
      a given hash algorithm is keyed or not.  AF_ALG currently does this by
      checking for the presence of a ->setkey() method.  However, this is
      actually slightly broken because the CRC-32 algorithms implement
      ->setkey() but can also be used without a key.  (The CRC-32 "key" is not
      actually a cryptographic key but rather represents the initial state.
      If not overridden, then a default initial state is used.)
      
      Prepare to fix this by introducing a flag CRYPTO_ALG_OPTIONAL_KEY which
      indicates that the algorithm has a ->setkey() method, but it is not
      required to be called.  Then set it on all the CRC-32 algorithms.
      
      The same also applies to the Adler-32 implementation in Lustre.
      
      Also, the cryptd and mcryptd templates have to pass through the flag
      from their underlying algorithm.
      
      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>
      a208fa8f
    • Eric Biggers's avatar
      crypto: poly1305 - remove ->setkey() method · a16e772e
      Eric Biggers authored
      Since Poly1305 requires a nonce per invocation, the Linux kernel
      implementations of Poly1305 don't use the crypto API's keying mechanism
      and instead expect the key and nonce as the first 32 bytes of the data.
      But ->setkey() is still defined as a stub returning an error code.  This
      prevents Poly1305 from being used through AF_ALG and will also break it
      completely once we start enforcing that all crypto API users (not just
      AF_ALG) call ->setkey() if present.
      
      Fix it by removing crypto_poly1305_setkey(), leaving ->setkey as NULL.
      
      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>
      a16e772e
    • Eric Biggers's avatar
      crypto: mcryptd - pass through absence of ->setkey() · fa59b92d
      Eric Biggers authored
      When the mcryptd template is used to wrap an unkeyed hash algorithm,
      don't install a ->setkey() method to the mcryptd instance.  This change
      is necessary for mcryptd to keep working with unkeyed hash algorithms
      once we start enforcing that ->setkey() is called when present.
      
      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>
      fa59b92d
    • Eric Biggers's avatar
      crypto: cryptd - pass through absence of ->setkey() · 841a3ff3
      Eric Biggers authored
      When the cryptd template is used to wrap an unkeyed hash algorithm,
      don't install a ->setkey() method to the cryptd instance.  This change
      is necessary for cryptd to keep working with unkeyed hash algorithms
      once we start enforcing that ->setkey() is called when present.
      
      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>
      841a3ff3
    • Eric Biggers's avatar
      crypto: hash - introduce crypto_hash_alg_has_setkey() · cd6ed77a
      Eric Biggers authored
      Templates that use an shash spawn can use crypto_shash_alg_has_setkey()
      to determine whether the underlying algorithm requires a key or not.
      But there was no corresponding function for ahash spawns.  Add it.
      
      Note that the new function actually has to support both shash and ahash
      algorithms, since the ahash API can be used with either.
      
      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>
      cd6ed77a
    • Colin Ian King's avatar
      crypto: tcrypt - free xoutbuf instead of axbuf · c6ba4f3e
      Colin Ian King authored
      There seems to be a cut-n-paste bug with the name of the buffer being
      free'd, xoutbuf should be used instead of axbuf.
      
      Detected by CoverityScan, CID#1463420 ("Copy-paste error")
      
      Fixes: 427988d9 ("crypto: tcrypt - add multibuf aead speed test")
      Signed-off-by: default avatarColin Ian King <colin.king@canonical.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      c6ba4f3e
    • Colin Ian King's avatar
      crypto: tcrypt - fix spelling mistake: "bufufer"-> "buffer" · 38dbe2d1
      Colin Ian King authored
      Trivial fix to spelling mistakes in pr_err error message text.
      Signed-off-by: default avatarColin Ian King <colin.king@canonical.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      38dbe2d1
    • Stephan Mueller's avatar
      crypto: af_alg - whitelist mask and type · bb30b884
      Stephan Mueller authored
      The user space interface allows specifying the type and mask field used
      to allocate the cipher. Only a subset of the possible flags are intended
      for user space. Therefore, white-list the allowed flags.
      
      In case the user space caller uses at least one non-allowed flag, EINVAL
      is returned.
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarStephan Mueller <smueller@chronox.de>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      bb30b884
    • Joey Pabalinas's avatar
      crypto: testmgr - change `guard` to unsigned char · da1729ce
      Joey Pabalinas authored
      When char is signed, storing the values 0xba (186) and 0xad (173) in the
      `guard` array produces signed overflow. Change the type of `guard` to
      static unsigned char to correct undefined behavior and reduce function
      stack usage.
      Signed-off-by: default avatarJoey Pabalinas <joeypabalinas@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      da1729ce
    • Eric Biggers's avatar
      crypto: chacha20 - use rol32() macro from bitops.h · 7660b1fb
      Eric Biggers authored
      For chacha20_block(), use the existing 32-bit left-rotate function
      instead of defining one ourselves.
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      7660b1fb
    • Himanshu Jha's avatar
      crypto: Use zeroing memory allocator instead of allocator/memset · 75d68369
      Himanshu Jha authored
      Use dma_zalloc_coherent for allocating zeroed
      memory and remove unnecessary memset function.
      
      Done using Coccinelle.
      Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
      0-day tested with no failures.
      Signed-off-by: default avatarHimanshu Jha <himanshujha199640@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      75d68369
  2. 05 Jan, 2018 14 commits
  3. 28 Dec, 2017 9 commits