Skip to content
Snippets Groups Projects
  1. Feb 08, 2019
    • Eric Biggers's avatar
      crypto: ahash - fix another early termination in hash walk · 77568e53
      Eric Biggers authored
      
      Hash algorithms with an alignmask set, e.g. "xcbc(aes-aesni)" and
      "michael_mic", fail the improved hash tests because they sometimes
      produce the wrong digest.  The bug is that in the case where a
      scatterlist element crosses pages, not all the data is actually hashed
      because the scatterlist walk terminates too early.  This happens because
      the 'nbytes' variable in crypto_hash_walk_done() is assigned the number
      of bytes remaining in the page, then later interpreted as the number of
      bytes remaining in the scatterlist element.  Fix it.
      
      Fixes: 900a081f ("crypto: ahash - Fix early termination in hash walk")
      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>
      77568e53
  2. Jan 18, 2019
    • 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
  3. Dec 07, 2018
  4. Nov 09, 2018
    • Eric Biggers's avatar
      crypto: user - clean up report structure copying · 37db69e0
      Eric Biggers authored
      There have been a pretty ridiculous number of issues with initializing
      the report structures that are copied to userspace by NETLINK_CRYPTO.
      Commit 4473710d ("crypto: user - Prepare for CRYPTO_MAX_ALG_NAME
      expansion") replaced some strncpy()s with strlcpy()s, thereby
      introducing information leaks.  Later two other people tried to replace
      other strncpy()s with strlcpy() too, which would have introduced even
      more information leaks:
      
          - https://lore.kernel.org/patchwork/patch/954991/
          - https://patchwork.kernel.org/patch/10434351/
      
      
      
      Commit cac5818c ("crypto: user - Implement a generic crypto
      statistics") also uses the buggy strlcpy() approach and therefore leaks
      uninitialized memory to userspace.  A fix was proposed, but it was
      originally incomplete.
      
      Seeing as how apparently no one can get this right with the current
      approach, change all the reporting functions to:
      
      - Start by memsetting the report structure to 0.  This guarantees it's
        always initialized, regardless of what happens later.
      - Initialize all strings using strscpy().  This is safe after the
        memset, ensures null termination of long strings, avoids unnecessary
        work, and avoids the -Wstringop-truncation warnings from gcc.
      - Use sizeof(var) instead of sizeof(type).  This is more robust against
        copy+paste errors.
      
      For simplicity, also reuse the -EMSGSIZE return value from nla_put().
      
      Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      37db69e0
  5. Sep 28, 2018
  6. Sep 04, 2018
  7. Mar 30, 2018
  8. Feb 15, 2018
  9. Jan 12, 2018
    • 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 - 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
  10. Nov 03, 2017
  11. Aug 22, 2017
  12. Apr 10, 2017
    • Herbert Xu's avatar
      crypto: ahash - Fix EINPROGRESS notification callback · ef0579b6
      Herbert Xu authored
      
      The ahash API modifies the request's callback function in order
      to clean up after itself in some corner cases (unaligned final
      and missing finup).
      
      When the request is complete ahash will restore the original
      callback and everything is fine.  However, when the request gets
      an EBUSY on a full queue, an EINPROGRESS callback is made while
      the request is still ongoing.
      
      In this case the ahash API will incorrectly call its own callback.
      
      This patch fixes the problem by creating a temporary request
      object on the stack which is used to relay EINPROGRESS back to
      the original completion function.
      
      This patch also adds code to preserve the original flags value.
      
      Fixes: ab6bf4e5 ("crypto: hash - Fix the pointer voodoo in...")
      Cc: <stable@vger.kernel.org>
      Reported-by: default avatarSabrina Dubroca <sd@queasysnail.net>
      Tested-by: default avatarSabrina Dubroca <sd@queasysnail.net>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      ef0579b6
  13. Jan 12, 2017
    • Gideon Israel Dsouza's avatar
      crypto: Replaced gcc specific attributes with macros from compiler.h · d8c34b94
      Gideon Israel Dsouza authored
      
      Continuing from this commit: 52f5684c
      ("kernel: use macros from compiler.h instead of __attribute__((...))")
      
      I submitted 4 total patches. They are part of task I've taken up to
      increase compiler portability in the kernel. I've cleaned up the
      subsystems under /kernel /mm /block and /security, this patch targets
      /crypto.
      
      There is <linux/compiler.h> which provides macros for various gcc specific
      constructs. Eg: __weak for __attribute__((weak)). I've cleaned all
      instances of gcc specific attributes with the right macros for the crypto
      subsystem.
      
      I had to make one additional change into compiler-gcc.h for the case when
      one wants to use this: __attribute__((aligned) and not specify an alignment
      factor. From the gcc docs, this will result in the largest alignment for
      that data type on the target machine so I've named the macro
      __aligned_largest. Please advise if another name is more appropriate.
      
      Signed-off-by: default avatarGideon Israel Dsouza <gidisrael@gmail.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      d8c34b94
  14. Jul 01, 2016
  15. May 05, 2016
  16. Feb 06, 2016
  17. Jan 25, 2016
  18. Jan 18, 2016
  19. Oct 13, 2015
  20. Jan 26, 2015
  21. Dec 22, 2014
  22. Aug 25, 2014
  23. May 21, 2014
    • Herbert Xu's avatar
      crypto: hash - Add real ahash walk interface · 75ecb231
      Herbert Xu authored
      
      Although the existing hash walk interface has already been used
      by a number of ahash crypto drivers, it turns out that none of
      them were really asynchronous.  They were all essentially polling
      for completion.
      
      That's why nobody has noticed until now that the walk interface
      couldn't work with a real asynchronous driver since the memory
      is mapped using kmap_atomic.
      
      As we now have a use-case for a real ahash implementation on x86,
      this patch creates a minimal ahash walk interface.  Basically it
      just calls kmap instead of kmap_atomic and does away with the
      crypto_yield call.  Real ahash crypto drivers don't need to yield
      since by definition they won't be hogging the CPU.
      
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      75ecb231
  24. Mar 21, 2014
    • Marek Vasut's avatar
      crypto: hash - Simplify the ahash_finup implementation · d4a7a0fb
      Marek Vasut authored
      
      The ahash_def_finup() can make use of the request save/restore functions,
      thus make it so. This simplifies the code a little and unifies the code
      paths.
      
      Note that the same remark about free()ing the req->priv applies here, the
      req->priv can only be free()'d after the original request was restored.
      
      Finally, squash a bug in the invocation of completion in the ASYNC path.
      In both ahash_def_finup_done{1,2}, the function areq->base.complete(X, err);
      was called with X=areq->base.data . This is incorrect , as X=&areq->base
      is the correct value. By analysis of the data structures, we see the areq is
      of type 'struct ahash_request' , areq->base is of type 'struct crypto_async_request'
      and areq->base.completion is of type crypto_completion_t, which is defined in
      include/linux/crypto.h as:
      
        typedef void (*crypto_completion_t)(struct crypto_async_request *req, int err);
      
      This is one lead that the X should be &areq->base . Next up, we can inspect
      other code which calls the completion callback to give us kind-of statistical
      idea of how this callback is used. We can try:
      
        $ git grep base\.complete\( drivers/crypto/
      
      Finally, by inspecting ahash_request_set_callback() implementation defined
      in include/crypto/hash.h , we observe that the .data entry of 'struct
      crypto_async_request' is intended for arbitrary data, not for completion
      argument.
      
      Signed-off-by: default avatarMarek Vasut <marex@denx.de>
      Cc: David S. Miller <davem@davemloft.net>
      Cc: Fabio Estevam <fabio.estevam@freescale.com>
      Cc: Herbert Xu <herbert@gondor.apana.org.au>
      Cc: Shawn Guo <shawn.guo@linaro.org>
      Cc: Tom Lendacky <thomas.lendacky@amd.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      d4a7a0fb
    • Marek Vasut's avatar
      crypto: hash - Pull out the functions to save/restore request · 1ffc9fbd
      Marek Vasut authored
      
      The functions to save original request within a newly adjusted request
      and it's counterpart to restore the original request can be re-used by
      more code in the crypto/ahash.c file. Pull these functions out from the
      code so they're available.
      
      Signed-off-by: default avatarMarek Vasut <marex@denx.de>
      Cc: David S. Miller <davem@davemloft.net>
      Cc: Fabio Estevam <fabio.estevam@freescale.com>
      Cc: Herbert Xu <herbert@gondor.apana.org.au>
      Cc: Shawn Guo <shawn.guo@linaro.org>
      Cc: Tom Lendacky <thomas.lendacky@amd.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      1ffc9fbd
    • Marek Vasut's avatar
      crypto: hash - Fix the pointer voodoo in unaligned ahash · ab6bf4e5
      Marek Vasut authored
      
      Add documentation for the pointer voodoo that is happening in crypto/ahash.c
      in ahash_op_unaligned(). This code is quite confusing, so add a beefy chunk
      of documentation.
      
      Moreover, make sure the mangled request is completely restored after finishing
      this unaligned operation. This means restoring all of .result, .base.data
      and .base.complete .
      
      Also, remove the crypto_completion_t complete = ... line present in the
      ahash_op_unaligned_done() function. This type actually declares a function
      pointer, which is very confusing.
      
      Finally, yet very important nonetheless, make sure the req->priv is free()'d
      only after the original request is restored in ahash_op_unaligned_done().
      The req->priv data must not be free()'d before that in ahash_op_unaligned_finish(),
      since we would be accessing previously free()'d data in ahash_op_unaligned_done()
      and cause corruption.
      
      Signed-off-by: default avatarMarek Vasut <marex@denx.de>
      Cc: David S. Miller <davem@davemloft.net>
      Cc: Fabio Estevam <fabio.estevam@freescale.com>
      Cc: Herbert Xu <herbert@gondor.apana.org.au>
      Cc: Shawn Guo <shawn.guo@linaro.org>
      Cc: Tom Lendacky <thomas.lendacky@amd.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      ab6bf4e5
  25. Jan 05, 2014
    • Marek Vasut's avatar
      crypto: ahash - Fully restore ahash request before completing · 1d9a394b
      Marek Vasut authored
      
      When finishing the ahash request, the ahash_op_unaligned_done() will
      call complete() on the request. Yet, this will not call the correct
      complete callback. The correct complete callback was previously stored
      in the requests' private data, as seen in ahash_op_unaligned(). This
      patch restores the correct complete callback and .data field of the
      request before calling complete() on it.
      
      Signed-off-by: default avatarMarek Vasut <marex@denx.de>
      Cc: David S. Miller <davem@davemloft.net>
      Cc: Fabio Estevam <fabio.estevam@freescale.com>
      Cc: Shawn Guo <shawn.guo@linaro.org>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      1d9a394b
  26. Feb 19, 2013
    • Mathias Krause's avatar
      crypto: user - fix info leaks in report API · 9a5467bf
      Mathias Krause authored
      
      Three errors resulting in kernel memory disclosure:
      
      1/ The structures used for the netlink based crypto algorithm report API
      are located on the stack. As snprintf() does not fill the remainder of
      the buffer with null bytes, those stack bytes will be disclosed to users
      of the API. Switch to strncpy() to fix this.
      
      2/ crypto_report_one() does not initialize all field of struct
      crypto_user_alg. Fix this to fix the heap info leak.
      
      3/ For the module name we should copy only as many bytes as
      module_name() returns -- not as much as the destination buffer could
      hold. But the current code does not and therefore copies random data
      from behind the end of the module name, as the module name is always
      shorter than CRYPTO_MAX_ALG_NAME.
      
      Also switch to use strncpy() to copy the algorithm's name and
      driver_name. They are strings, after all.
      
      Signed-off-by: default avatarMathias Krause <minipli@googlemail.com>
      Cc: Steffen Klassert <steffen.klassert@secunet.com>
      Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
      9a5467bf
  27. Apr 02, 2012
  28. Mar 20, 2012
  29. Nov 10, 2011
  30. Oct 21, 2011
  31. Aug 06, 2010
  32. Mar 03, 2010
  33. Jul 24, 2009
  34. Jul 15, 2009
  35. Jul 14, 2009
Loading