1. 13 Nov, 2018 2 commits
  2. 10 Nov, 2018 1 commit
    • Jia-Ju Bai's avatar
      crypto: shash - Fix a sleep-in-atomic bug in shash_setkey_unaligned · 79d47dd6
      Jia-Ju Bai authored
      [ Upstream commit 9039f3ef ]
      
      The SCTP program may sleep under a spinlock, and the function call path is:
      sctp_generate_t3_rtx_event (acquire the spinlock)
        sctp_do_sm
          sctp_side_effects
            sctp_cmd_interpreter
              sctp_make_init_ack
                sctp_pack_cookie
                  crypto_shash_setkey
                    shash_setkey_unaligned
                      kmalloc(GFP_KERNEL)
      
      For the same reason, the orinoco driver may sleep in interrupt handler,
      and the function call path is:
      orinoco_rx_isr_tasklet
        orinoco_rx
          orinoco_mic
            crypto_shash_setkey
              shash_setkey_unaligned
                kmalloc(GFP_KERNEL)
      
      To fix it, GFP_KERNEL is replaced with GFP_ATOMIC.
      This bug is found by my static analysis tool and my code review.
      Signed-off-by: default avatarJia-Ju Bai <baijiaju1990@163.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
      79d47dd6
  3. 04 Oct, 2018 1 commit
    • Stafford Horne's avatar
      crypto: skcipher - Fix -Wstringop-truncation warnings · 58104852
      Stafford Horne authored
      [ Upstream commit cefd769fd0192c84d638f66da202459ed8ad63ba ]
      
      As of GCC 9.0.0 the build is reporting warnings like:
      
          crypto/ablkcipher.c: In function ‘crypto_ablkcipher_report’:
          crypto/ablkcipher.c:374:2: warning: ‘strncpy’ specified bound 64 equals destination size [-Wstringop-truncation]
            strncpy(rblkcipher.geniv, alg->cra_ablkcipher.geniv ?: "<default>",
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
             sizeof(rblkcipher.geniv));
             ~~~~~~~~~~~~~~~~~~~~~~~~~
      
      This means the strnycpy might create a non null terminated string.  Fix this by
      explicitly performing '\0' termination.
      
      Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
      Cc: Arnd Bergmann <arnd@arndb.de>
      Cc: Max Filippov <jcmvbkbc@gmail.com>
      Cc: Eric Biggers <ebiggers3@gmail.com>
      Cc: Nick Desaulniers <nick.desaulniers@gmail.com>
      Signed-off-by: default avatarStafford Horne <shorne@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarSasha Levin <alexander.levin@microsoft.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      58104852
  4. 26 Sep, 2018 1 commit
  5. 09 Sep, 2018 1 commit
  6. 17 Aug, 2018 4 commits
    • Eric Biggers's avatar
      crypto: ablkcipher - fix crash flushing dcache in error path · b7c2b699
      Eric Biggers authored
      commit 318abdfbe708aaaa652c79fb500e9bd60521f9dc upstream.
      
      Like the skcipher_walk and blkcipher_walk cases:
      
      scatterwalk_done() is only meant to be called after a nonzero number of
      bytes have been processed, since scatterwalk_pagedone() will flush the
      dcache of the *previous* page.  But in the error case of
      ablkcipher_walk_done(), e.g. if the input wasn't an integer number of
      blocks, scatterwalk_done() was actually called after advancing 0 bytes.
      This caused a crash ("BUG: unable to handle kernel paging request")
      during '!PageSlab(page)' on architectures like arm and arm64 that define
      ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE, provided that the input was
      page-aligned as in that case walk->offset == 0.
      
      Fix it by reorganizing ablkcipher_walk_done() to skip the
      scatterwalk_advance() and scatterwalk_done() if an error has occurred.
      Reported-by: default avatarLiu Chao <liuchao741@huawei.com>
      Fixes: bf06099d ("crypto: skcipher - Add ablkcipher_walk interfaces")
      Cc: <stable@vger.kernel.org> # v2.6.35+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      b7c2b699
    • Eric Biggers's avatar
      crypto: blkcipher - fix crash flushing dcache in error path · afd5c42d
      Eric Biggers authored
      commit 0868def3e4100591e7a1fdbf3eed1439cc8f7ca3 upstream.
      
      Like the skcipher_walk case:
      
      scatterwalk_done() is only meant to be called after a nonzero number of
      bytes have been processed, since scatterwalk_pagedone() will flush the
      dcache of the *previous* page.  But in the error case of
      blkcipher_walk_done(), e.g. if the input wasn't an integer number of
      blocks, scatterwalk_done() was actually called after advancing 0 bytes.
      This caused a crash ("BUG: unable to handle kernel paging request")
      during '!PageSlab(page)' on architectures like arm and arm64 that define
      ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE, provided that the input was
      page-aligned as in that case walk->offset == 0.
      
      Fix it by reorganizing blkcipher_walk_done() to skip the
      scatterwalk_advance() and scatterwalk_done() if an error has occurred.
      
      This bug was found by syzkaller fuzzing.
      
      Reproducer, assuming ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE:
      
      	#include <linux/if_alg.h>
      	#include <sys/socket.h>
      	#include <unistd.h>
      
      	int main()
      	{
      		struct sockaddr_alg addr = {
      			.salg_type = "skcipher",
      			.salg_name = "ecb(aes-generic)",
      		};
      		char buffer[4096] __attribute__((aligned(4096))) = { 0 };
      		int fd;
      
      		fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
      		bind(fd, (void *)&addr, sizeof(addr));
      		setsockopt(fd, SOL_ALG, ALG_SET_KEY, buffer, 16);
      		fd = accept(fd, NULL, NULL);
      		write(fd, buffer, 15);
      		read(fd, buffer, 15);
      	}
      Reported-by: default avatarLiu Chao <liuchao741@huawei.com>
      Fixes: 5cde0af2 ("[CRYPTO] cipher: Added block cipher type")
      Cc: <stable@vger.kernel.org> # v2.6.19+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      afd5c42d
    • Eric Biggers's avatar
      crypto: vmac - separate tfm and request context · 81ad8a8e
      Eric Biggers authored
      commit bb29648102335586e9a66289a1d98a0cb392b6e5 upstream.
      
      syzbot reported a crash in vmac_final() when multiple threads
      concurrently use the same "vmac(aes)" transform through AF_ALG.  The bug
      is pretty fundamental: the VMAC template doesn't separate per-request
      state from per-tfm (per-key) state like the other hash algorithms do,
      but rather stores it all in the tfm context.  That's wrong.
      
      Also, vmac_final() incorrectly zeroes most of the state including the
      derived keys and cached pseudorandom pad.  Therefore, only the first
      VMAC invocation with a given key calculates the correct digest.
      
      Fix these bugs by splitting the per-tfm state from the per-request state
      and using the proper init/update/final sequencing for requests.
      
      Reproducer for the crash:
      
          #include <linux/if_alg.h>
          #include <sys/socket.h>
          #include <unistd.h>
      
          int main()
          {
                  int fd;
                  struct sockaddr_alg addr = {
                          .salg_type = "hash",
                          .salg_name = "vmac(aes)",
                  };
                  char buf[256] = { 0 };
      
                  fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
                  bind(fd, (void *)&addr, sizeof(addr));
                  setsockopt(fd, SOL_ALG, ALG_SET_KEY, buf, 16);
                  fork();
                  fd = accept(fd, NULL, NULL);
                  for (;;)
                          write(fd, buf, 256);
          }
      
      The immediate cause of the crash is that vmac_ctx_t.partial_size exceeds
      VMAC_NHBYTES, causing vmac_final() to memset() a negative length.
      
      Reported-by: syzbot+264bca3a6e8d645550d3@syzkaller.appspotmail.com
      Fixes: f1939f7c ("crypto: vmac - New hash algorithm for intel_txt support")
      Cc: <stable@vger.kernel.org> # v2.6.32+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      81ad8a8e
    • Eric Biggers's avatar
      crypto: vmac - require a block cipher with 128-bit block size · 371c35cb
      Eric Biggers authored
      commit 73bf20ef3df262026c3470241ae4ac8196943ffa upstream.
      
      The VMAC template assumes the block cipher has a 128-bit block size, but
      it failed to check for that.  Thus it was possible to instantiate it
      using a 64-bit block size cipher, e.g. "vmac(cast5)", causing
      uninitialized memory to be used.
      
      Add the needed check when instantiating the template.
      
      Fixes: f1939f7c ("crypto: vmac - New hash algorithm for intel_txt support")
      Cc: <stable@vger.kernel.org> # v2.6.32+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      371c35cb
  7. 03 Aug, 2018 2 commits
  8. 03 Jul, 2018 1 commit
    • Maciej S. Szmigiero's avatar
      X.509: unpack RSA signatureValue field from BIT STRING · 41b1d57a
      Maciej S. Szmigiero authored
      commit b65c32ec5a942ab3ada93a048089a938918aba7f upstream.
      
      The signatureValue field of a X.509 certificate is encoded as a BIT STRING.
      For RSA signatures this BIT STRING is of so-called primitive subtype, which
      contains a u8 prefix indicating a count of unused bits in the encoding.
      
      We have to strip this prefix from signature data, just as we already do for
      key data in x509_extract_key_data() function.
      
      This wasn't noticed earlier because this prefix byte is zero for RSA key
      sizes divisible by 8. Since BIT STRING is a big-endian encoding adding zero
      prefixes has no bearing on its value.
      
      The signature length, however was incorrect, which is a problem for RSA
      implementations that need it to be exactly correct (like AMD CCP).
      Signed-off-by: default avatarMaciej S. Szmigiero <mail@maciej.szmigiero.name>
      Fixes: c26fd69f ("X.509: Add a crypto key parser for binary (DER) X.509 certificates")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarJames Morris <james.morris@microsoft.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      41b1d57a
  9. 30 May, 2018 1 commit
    • Eric Biggers's avatar
      PKCS#7: fix direct verification of SignerInfo signature · 4d7388a1
      Eric Biggers authored
      [ Upstream commit 6459ae38 ]
      
      If none of the certificates in a SignerInfo's certificate chain match a
      trusted key, nor is the last certificate signed by a trusted key, then
      pkcs7_validate_trust_one() tries to check whether the SignerInfo's
      signature was made directly by a trusted key.  But, it actually fails to
      set the 'sig' variable correctly, so it actually verifies the last
      signature seen.  That will only be the SignerInfo's signature if the
      certificate chain is empty; otherwise it will actually be the last
      certificate's signature.
      
      This is not by itself a security problem, since verifying any of the
      certificates in the chain should be sufficient to verify the SignerInfo.
      Still, it's not working as intended so it should be fixed.
      
      Fix it by setting 'sig' correctly for the direct verification case.
      
      Fixes: 757932e6 ("PKCS#7: Handle PKCS#7 messages that contain no X.509 certs")
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Signed-off-by: default avatarSasha Levin <alexander.levin@microsoft.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      4d7388a1
  10. 16 May, 2018 1 commit
  11. 01 May, 2018 1 commit
  12. 13 Apr, 2018 2 commits
  13. 08 Apr, 2018 1 commit
  14. 28 Feb, 2018 3 commits
    • Eric Biggers's avatar
      X.509: fix NULL dereference when restricting key with unsupported_sig · f2915986
      Eric Biggers authored
      commit 4b34968e upstream.
      
      The asymmetric key type allows an X.509 certificate to be added even if
      its signature's hash algorithm is not available in the crypto API.  In
      that case 'payload.data[asym_auth]' will be NULL.  But the key
      restriction code failed to check for this case before trying to use the
      signature, resulting in a NULL pointer dereference in
      key_or_keyring_common() or in restrict_link_by_signature().
      
      Fix this by returning -ENOPKG when the signature is unsupported.
      
      Reproducer when all the CONFIG_CRYPTO_SHA512* options are disabled and
      keyctl has support for the 'restrict_keyring' command:
      
          keyctl new_session
          keyctl restrict_keyring @s asymmetric builtin_trusted
          openssl req -new -sha512 -x509 -batch -nodes -outform der \
              | keyctl padd asymmetric desc @s
      
      Fixes: a511e1af ("KEYS: Move the point of trust determination to __key_link()")
      Cc: <stable@vger.kernel.org> # v4.7+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      f2915986
    • Eric Biggers's avatar
      PKCS#7: fix certificate chain verification · e4b02ca6
      Eric Biggers authored
      commit 971b42c0 upstream.
      
      When pkcs7_verify_sig_chain() is building the certificate chain for a
      SignerInfo using the certificates in the PKCS#7 message, it is passing
      the wrong arguments to public_key_verify_signature().  Consequently,
      when the next certificate is supposed to be used to verify the previous
      certificate, the next certificate is actually used to verify itself.
      
      An attacker can use this bug to create a bogus certificate chain that
      has no cryptographic relationship between the beginning and end.
      
      Fortunately I couldn't quite find a way to use this to bypass the
      overall signature verification, though it comes very close.  Here's the
      reasoning: due to the bug, every certificate in the chain beyond the
      first actually has to be self-signed (where "self-signed" here refers to
      the actual key and signature; an attacker might still manipulate the
      certificate fields such that the self_signed flag doesn't actually get
      set, and thus the chain doesn't end immediately).  But to pass trust
      validation (pkcs7_validate_trust()), either the SignerInfo or one of the
      certificates has to actually be signed by a trusted key.  Since only
      self-signed certificates can be added to the chain, the only way for an
      attacker to introduce a trusted signature is to include a self-signed
      trusted certificate.
      
      But, when pkcs7_validate_trust_one() reaches that certificate, instead
      of trying to verify the signature on that certificate, it will actually
      look up the corresponding trusted key, which will succeed, and then try
      to verify the *previous* certificate, which will fail.  Thus, disaster
      is narrowly averted (as far as I could tell).
      
      Fixes: 6c2dc5ae ("X.509: Extract signature digest and make self-signed cert checks earlier")
      Cc: <stable@vger.kernel.org> # v4.7+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      e4b02ca6
    • Eric Biggers's avatar
      X.509: fix BUG_ON() when hash algorithm is unsupported · c60e246f
      Eric Biggers authored
      commit 437499ee upstream.
      
      The X.509 parser mishandles the case where the certificate's signature's
      hash algorithm is not available in the crypto API.  In this case,
      x509_get_sig_params() doesn't allocate the cert->sig->digest buffer;
      this part seems to be intentional.  However,
      public_key_verify_signature() is still called via
      x509_check_for_self_signed(), which triggers the 'BUG_ON(!sig->digest)'.
      
      Fix this by making public_key_verify_signature() return -ENOPKG if the
      hash buffer has not been allocated.
      
      Reproducer when all the CONFIG_CRYPTO_SHA512* options are disabled:
      
          openssl req -new -sha512 -x509 -batch -nodes -outform der \
              | keyctl padd asymmetric desc @s
      
      Fixes: 6c2dc5ae ("X.509: Extract signature digest and make self-signed cert checks earlier")
      Reported-by: default avatarPaolo Valente <paolo.valente@linaro.org>
      Cc: Paolo Valente <paolo.valente@linaro.org>
      Cc: <stable@vger.kernel.org> # v4.7+
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      c60e246f
  15. 25 Feb, 2018 2 commits
    • Eric Biggers's avatar
      crypto: hash - prevent using keyed hashes without setting key · adf26e87
      Eric Biggers authored
      commit 9fa68f62 upstream.
      
      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>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      
      adf26e87
    • Eric Biggers's avatar
      crypto: hash - annotate algorithms taking optional key · b392a53b
      Eric Biggers authored
      commit a208fa8f upstream.
      
      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>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      
      b392a53b
  16. 17 Feb, 2018 4 commits
  17. 13 Feb, 2018 1 commit
  18. 03 Feb, 2018 3 commits
  19. 17 Jan, 2018 1 commit
    • Eric Biggers's avatar
      crypto: algapi - fix NULL dereference in crypto_remove_spawns() · 3752d2fb
      Eric Biggers authored
      commit 9a006742 upstream.
      
      syzkaller triggered a NULL pointer dereference in crypto_remove_spawns()
      via a program that repeatedly and concurrently requests AEADs
      "authenc(cmac(des3_ede-asm),pcbc-aes-aesni)" and hashes "cmac(des3_ede)"
      through AF_ALG, where the hashes are requested as "untested"
      (CRYPTO_ALG_TESTED is set in ->salg_mask but clear in ->salg_feat; this
      causes the template to be instantiated for every request).
      
      Although AF_ALG users really shouldn't be able to request an "untested"
      algorithm, the NULL pointer dereference is actually caused by a
      longstanding race condition where crypto_remove_spawns() can encounter
      an instance which has had spawn(s) "grabbed" but hasn't yet been
      registered, resulting in ->cra_users still being NULL.
      
      We probably should properly initialize ->cra_users earlier, but that
      would require updating many templates individually.  For now just fix
      the bug in a simple way that can easily be backported: make
      crypto_remove_spawns() treat a NULL ->cra_users list as empty.
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      3752d2fb
  20. 10 Jan, 2018 2 commits
    • Eric Biggers's avatar
      crypto: pcrypt - fix freeing pcrypt instances · c195a4c0
      Eric Biggers authored
      commit d76c6810 upstream.
      
      pcrypt is using the old way of freeing instances, where the ->free()
      method specified in the 'struct crypto_template' is passed a pointer to
      the 'struct crypto_instance'.  But the crypto_instance is being
      kfree()'d directly, which is incorrect because the memory was actually
      allocated as an aead_instance, which contains the crypto_instance at a
      nonzero offset.  Thus, the wrong pointer was being kfree()'d.
      
      Fix it by switching to the new way to free aead_instance's where the
      ->free() method is specified in the aead_instance itself.
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Fixes: 0496f560 ("crypto: pcrypt - Add support for new AEAD interface")
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      c195a4c0
    • Eric Biggers's avatar
      crypto: chacha20poly1305 - validate the digest size · 868f50b9
      Eric Biggers authored
      commit e57121d0 upstream.
      
      If the rfc7539 template was instantiated with a hash algorithm with
      digest size larger than 16 bytes (POLY1305_DIGEST_SIZE), then the digest
      overran the 'tag' buffer in 'struct chachapoly_req_ctx', corrupting the
      subsequent memory, including 'cryptlen'.  This caused a crash during
      crypto_skcipher_decrypt().
      
      Fix it by, when instantiating the template, requiring that the
      underlying hash algorithm has the digest size expected for Poly1305.
      
      Reproducer:
      
          #include <linux/if_alg.h>
          #include <sys/socket.h>
          #include <unistd.h>
      
          int main()
          {
                  int algfd, reqfd;
                  struct sockaddr_alg addr = {
                          .salg_type = "aead",
                          .salg_name = "rfc7539(chacha20,sha256)",
                  };
                  unsigned char buf[32] = { 0 };
      
                  algfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
                  bind(algfd, (void *)&addr, sizeof(addr));
                  setsockopt(algfd, SOL_ALG, ALG_SET_KEY, buf, sizeof(buf));
                  reqfd = accept(algfd, 0, 0);
                  write(reqfd, buf, 16);
                  read(reqfd, buf, 16);
          }
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Fixes: 71ebc4d1 ("crypto: chacha20poly1305 - Add a ChaCha20-Poly1305 AEAD construction, RFC7539")
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      868f50b9
  21. 29 Dec, 2017 1 commit
    • Sebastian Andrzej Siewior's avatar
      crypto: mcryptd - protect the per-CPU queue with a lock · e81cff1c
      Sebastian Andrzej Siewior authored
      commit 9abffc6f upstream.
      
      mcryptd_enqueue_request() grabs the per-CPU queue struct and protects
      access to it with disabled preemption. Then it schedules a worker on the
      same CPU. The worker in mcryptd_queue_worker() guards access to the same
      per-CPU variable with disabled preemption.
      
      If we take CPU-hotplug into account then it is possible that between
      queue_work_on() and the actual invocation of the worker the CPU goes
      down and the worker will be scheduled on _another_ CPU. And here the
      preempt_disable() protection does not work anymore. The easiest thing is
      to add a spin_lock() to guard access to the list.
      
      Another detail: mcryptd_queue_worker() is not processing more than
      MCRYPTD_BATCH invocation in a row. If there are still items left, then
      it will invoke queue_work() to proceed with more later. *I* would
      suggest to simply drop that check because it does not use a system
      workqueue and the workqueue is already marked as "CPU_INTENSIVE". And if
      preemption is required then the scheduler should do it.
      However if queue_work() is used then the work item is marked as CPU
      unbound. That means it will try to run on the local CPU but it may run
      on another CPU as well. Especially with CONFIG_DEBUG_WQ_FORCE_RR_CPU=y.
      Again, the preempt_disable() won't work here but lock which was
      introduced will help.
      In order to keep work-item on the local CPU (and avoid RR) I changed it
      to queue_work_on().
      Signed-off-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      e81cff1c
  22. 20 Dec, 2017 4 commits
    • Robert Baronescu's avatar
      crypto: tcrypt - fix buffer lengths in test_aead_speed() · 18498f1c
      Robert Baronescu authored
      
      [ Upstream commit 7aacbfcb ]
      
      Fix the way the length of the buffers used for
      encryption / decryption are computed.
      For e.g. in case of encryption, input buffer does not contain
      an authentication tag.
      Signed-off-by: default avatarRobert Baronescu <robert.baronescu@nxp.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarSasha Levin <alexander.levin@verizon.com>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      18498f1c
    • Eric Biggers's avatar
      crypto: salsa20 - fix blkcipher_walk API usage · c32e053a
      Eric Biggers authored
      commit ecaaab56 upstream.
      
      When asked to encrypt or decrypt 0 bytes, both the generic and x86
      implementations of Salsa20 crash in blkcipher_walk_done(), either when
      doing 'kfree(walk->buffer)' or 'free_page((unsigned long)walk->page)',
      because walk->buffer and walk->page have not been initialized.
      
      The bug is that Salsa20 is calling blkcipher_walk_done() even when
      nothing is in 'walk.nbytes'.  But blkcipher_walk_done() is only meant to
      be called when a nonzero number of bytes have been provided.
      
      The broken code is part of an optimization that tries to make only one
      call to salsa20_encrypt_bytes() to process inputs that are not evenly
      divisible by 64 bytes.  To fix the bug, just remove this "optimization"
      and use the blkcipher_walk API the same way all the other users do.
      
      Reproducer:
      
          #include <linux/if_alg.h>
          #include <sys/socket.h>
          #include <unistd.h>
      
          int main()
          {
                  int algfd, reqfd;
                  struct sockaddr_alg addr = {
                          .salg_type = "skcipher",
                          .salg_name = "salsa20",
                  };
                  char key[16] = { 0 };
      
                  algfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
                  bind(algfd, (void *)&addr, sizeof(addr));
                  reqfd = accept(algfd, 0, 0);
                  setsockopt(algfd, SOL_ALG, ALG_SET_KEY, key, sizeof(key));
                  read(reqfd, key, sizeof(key));
          }
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Fixes: eb6f13eb ("[CRYPTO] salsa20_generic: Fix multi-page processing")
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      c32e053a
    • Eric Biggers's avatar
      crypto: hmac - require that the underlying hash algorithm is unkeyed · 43259d07
      Eric Biggers authored
      commit af3ff804 upstream.
      
      Because the HMAC template didn't check that its underlying hash
      algorithm is unkeyed, trying to use "hmac(hmac(sha3-512-generic))"
      through AF_ALG or through KEYCTL_DH_COMPUTE resulted in the inner HMAC
      being used without having been keyed, resulting in sha3_update() being
      called without sha3_init(), causing a stack buffer overflow.
      
      This is a very old bug, but it seems to have only started causing real
      problems when SHA-3 support was added (requires CONFIG_CRYPTO_SHA3)
      because the innermost hash's state is ->import()ed from a zeroed buffer,
      and it just so happens that other hash algorithms are fine with that,
      but SHA-3 is not.  However, there could be arch or hardware-dependent
      hash algorithms also affected; I couldn't test everything.
      
      Fix the bug by introducing a function crypto_shash_alg_has_setkey()
      which tests whether a shash algorithm is keyed.  Then update the HMAC
      template to require that its underlying hash algorithm is unkeyed.
      
      Here is a reproducer:
      
          #include <linux/if_alg.h>
          #include <sys/socket.h>
      
          int main()
          {
              int algfd;
              struct sockaddr_alg addr = {
                  .salg_type = "hash",
                  .salg_name = "hmac(hmac(sha3-512-generic))",
              };
              char key[4096] = { 0 };
      
              algfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
              bind(algfd, (const struct sockaddr *)&addr, sizeof(addr));
              setsockopt(algfd, SOL_ALG, ALG_SET_KEY, key, sizeof(key));
          }
      
      Here was the KASAN report from syzbot:
      
          BUG: KASAN: stack-out-of-bounds in memcpy include/linux/string.h:341  [inline]
          BUG: KASAN: stack-out-of-bounds in sha3_update+0xdf/0x2e0  crypto/sha3_generic.c:161
          Write of size 4096 at addr ffff8801cca07c40 by task syzkaller076574/3044
      
          CPU: 1 PID: 3044 Comm: syzkaller076574 Not tainted 4.14.0-mm1+ #25
          Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  Google 01/01/2011
          Call Trace:
            __dump_stack lib/dump_stack.c:17 [inline]
            dump_stack+0x194/0x257 lib/dump_stack.c:53
            print_address_description+0x73/0x250 mm/kasan/report.c:252
            kasan_report_error mm/kasan/report.c:351 [inline]
            kasan_report+0x25b/0x340 mm/kasan/report.c:409
            check_memory_region_inline mm/kasan/kasan.c:260 [inline]
            check_memory_region+0x137/0x190 mm/kasan/kasan.c:267
            memcpy+0x37/0x50 mm/kasan/kasan.c:303
            memcpy include/linux/string.h:341 [inline]
            sha3_update+0xdf/0x2e0 crypto/sha3_generic.c:161
            crypto_shash_update+0xcb/0x220 crypto/shash.c:109
            shash_finup_unaligned+0x2a/0x60 crypto/shash.c:151
            crypto_shash_finup+0xc4/0x120 crypto/shash.c:165
            hmac_finup+0x182/0x330 crypto/hmac.c:152
            crypto_shash_finup+0xc4/0x120 crypto/shash.c:165
            shash_digest_unaligned+0x9e/0xd0 crypto/shash.c:172
            crypto_shash_digest+0xc4/0x120 crypto/shash.c:186
            hmac_setkey+0x36a/0x690 crypto/hmac.c:66
            crypto_shash_setkey+0xad/0x190 crypto/shash.c:64
            shash_async_setkey+0x47/0x60 crypto/shash.c:207
            crypto_ahash_setkey+0xaf/0x180 crypto/ahash.c:200
            hash_setkey+0x40/0x90 crypto/algif_hash.c:446
            alg_setkey crypto/af_alg.c:221 [inline]
            alg_setsockopt+0x2a1/0x350 crypto/af_alg.c:254
            SYSC_setsockopt net/socket.c:1851 [inline]
            SyS_setsockopt+0x189/0x360 net/socket.c:1830
            entry_SYSCALL_64_fastpath+0x1f/0x96
      Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      43259d07
    • Eric Biggers's avatar
      crypto: rsa - fix buffer overread when stripping leading zeroes · cd9b5986
      Eric Biggers authored
      commit d2890c37 upstream.
      
      In rsa_get_n(), if the buffer contained all 0's and "FIPS mode" is
      enabled, we would read one byte past the end of the buffer while
      scanning the leading zeroes.  Fix it by checking 'n_sz' before '!*ptr'.
      
      This bug was reachable by adding a specially crafted key of type
      "asymmetric" (requires CONFIG_RSA and CONFIG_X509_CERTIFICATE_PARSER).
      
      KASAN report:
      
          BUG: KASAN: slab-out-of-bounds in rsa_get_n+0x19e/0x1d0 crypto/rsa_helper.c:33
          Read of size 1 at addr ffff88003501a708 by task keyctl/196
      
          CPU: 1 PID: 196 Comm: keyctl Not tainted 4.14.0-09238-g1d3b78bb #26
          Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014
          Call Trace:
           rsa_get_n+0x19e/0x1d0 crypto/rsa_helper.c:33
           asn1_ber_decoder+0x82a/0x1fd0 lib/asn1_decoder.c:328
           rsa_set_pub_key+0xd3/0x320 crypto/rsa.c:278
           crypto_akcipher_set_pub_key ./include/crypto/akcipher.h:364 [inline]
           pkcs1pad_set_pub_key+0xae/0x200 crypto/rsa-pkcs1pad.c:117
           crypto_akcipher_set_pub_key ./include/crypto/akcipher.h:364 [inline]
           public_key_verify_signature+0x270/0x9d0 crypto/asymmetric_keys/public_key.c:106
           x509_check_for_self_signed+0x2ea/0x480 crypto/asymmetric_keys/x509_public_key.c:141
           x509_cert_parse+0x46a/0x620 crypto/asymmetric_keys/x509_cert_parser.c:129
           x509_key_preparse+0x61/0x750 crypto/asymmetric_keys/x509_public_key.c:174
           asymmetric_key_preparse+0xa4/0x150 crypto/asymmetric_keys/asymmetric_type.c:388
           key_create_or_update+0x4d4/0x10a0 security/keys/key.c:850
           SYSC_add_key security/keys/keyctl.c:122 [inline]
           SyS_add_key+0xe8/0x290 security/keys/keyctl.c:62
           entry_SYSCALL_64_fastpath+0x1f/0x96
      
          Allocated by task 196:
           __do_kmalloc mm/slab.c:3711 [inline]
           __kmalloc_track_caller+0x118/0x2e0 mm/slab.c:3726
           kmemdup+0x17/0x40 mm/util.c:118
           kmemdup ./include/linux/string.h:414 [inline]
           x509_cert_parse+0x2cb/0x620 crypto/asymmetric_keys/x509_cert_parser.c:106
           x509_key_preparse+0x61/0x750 crypto/asymmetric_keys/x509_public_key.c:174
           asymmetric_key_preparse+0xa4/0x150 crypto/asymmetric_keys/asymmetric_type.c:388
           key_create_or_update+0x4d4/0x10a0 security/keys/key.c:850
           SYSC_add_key security/keys/keyctl.c:122 [inline]
           SyS_add_key+0xe8/0x290 security/keys/keyctl.c:62
           entry_SYSCALL_64_fastpath+0x1f/0x96
      
      Fixes: 5a7de973 ("crypto: rsa - return raw integers for the ASN.1 parser")
      Cc: Tudor Ambarus <tudor-dan.ambarus@nxp.com>
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Reviewed-by: default avatarJames Morris <james.l.morris@oracle.com>
      Reviewed-by: default avatarDavid Howells <dhowells@redhat.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      cd9b5986