Commit Graph

17 Commits

Author SHA1 Message Date
Ard Biesheuvel
ff7397add9 crypto: ecdh - avoid buffer overflow in ecdh_set_secret()
commit 0aa171e9b267ce7c52d3a3df7bc9c1fc0203dec5 upstream.

Pavel reports that commit 17858b140bf4 ("crypto: ecdh - avoid unaligned
accesses in ecdh_set_secret()") fixes one problem but introduces another:
the unconditional memcpy() introduced by that commit may overflow the
target buffer if the source data is invalid, which could be the result of
intentional tampering.

So check params.key_size explicitly against the size of the target buffer
before validating the key further.

Fixes: 17858b140bf4 ("crypto: ecdh - avoid unaligned accesses in ecdh_set_secret()")
Reported-by: Pavel Machek <pavel@denx.de>
Cc: <stable@vger.kernel.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2021-01-12 20:16:17 +01:00
Ard Biesheuvel
36a58bda87 crypto: ecdh - avoid unaligned accesses in ecdh_set_secret()
commit 17858b140bf49961b71d4e73f1c3ea9bc8e7dda0 upstream.

ecdh_set_secret() casts a void* pointer to a const u64* in order to
feed it into ecc_is_key_valid(). This is not generally permitted by
the C standard, and leads to actual misalignment faults on ARMv6
cores. In some cases, these are fixed up in software, but this still
leads to performance hits that are entirely avoidable.

So let's copy the key into the ctx buffer first, which we will do
anyway in the common case, and which guarantees correct alignment.

Cc: <stable@vger.kernel.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2020-12-30 11:51:35 +01:00
Thomas Gleixner
2874c5fd28 treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 152
Based on 1 normalized pattern(s):

  this program is free software you can redistribute it and or modify
  it under the terms of the gnu general public license as published by
  the free software foundation either version 2 of the license or at
  your option any later version

extracted by the scancode license scanner the SPDX license identifier

  GPL-2.0-or-later

has been chosen to replace the boilerplate/reference in 3029 file(s).

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070032.746973796@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2019-05-30 11:26:32 -07:00
Eric Biggers
c4741b2305 crypto: run initcalls for generic implementations earlier
Use subsys_initcall for registration of all templates and generic
algorithm implementations, rather than module_init.  Then change
cryptomgr to use arch_initcall, to place it before the subsys_initcalls.

This is needed so that when both a generic and optimized implementation
of an algorithm are built into the kernel (not loadable modules), the
generic implementation is registered before the optimized one.
Otherwise, the self-tests for the optimized implementation are unable to
allocate the generic implementation for the new comparison fuzz tests.

Note that on arm, a side effect of this change is that self-tests for
generic implementations may run before the unaligned access handler has
been installed.  So, unaligned accesses will crash the kernel.  This is
arguably a good thing as it makes it easier to detect that type of bug.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2019-04-18 22:15:03 +08:00
Kees Cook
d5c3b17898 crypto: ecc - Actually remove stack VLA usage
On the quest to remove all VLAs from the kernel[1], this avoids VLAs
by just using the maximum allocation size (4 bytes) for stack arrays.
All the VLAs in ecc were either 3 or 4 bytes (or a multiple), so just
make it 4 bytes all the time. Initialization routines are adjusted to
check that ndigits does not end up larger than the arrays.

This includes a removal of the earlier attempt at this fix from
commit a963834b4742 ("crypto/ecc: Remove stack VLA usage")

[1] https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2018-04-21 00:58:29 +08:00
James Bottomley
95ec01ba1e crypto: ecdh - fix to allow multi segment scatterlists
Apparently the ecdh use case was in bluetooth which always has single
element scatterlists, so the ecdh module was hard coded to expect
them.  Now we're using this in TPM, we need multi-element
scatterlists, so remove this limitation.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2018-03-09 22:45:49 +08:00
Tudor-Dan Ambarus
f560acc3bb crypto: ecdh - remove empty exit()
Pointer members of an object with static storage duration, if not
explicitly initialized, will be initialized to a NULL pointer. The crypto
API checks if this pointer is not NULL before using it, we are safe to
remove the function.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-11-06 14:45:04 +08:00
Tudor-Dan Ambarus
952035baed crypto: ecdh - fix concurrency on shared secret and pubkey
ecdh_ctx contained static allocated data for the shared secret
and public key.

The shared secret and the public key were doomed to concurrency
issues because they could be shared by multiple crypto requests.

The concurrency is fixed by replacing per-tfm shared secret and
public key with per-request dynamically allocated shared secret
and public key.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-08-03 13:47:22 +08:00
Tudor-Dan Ambarus
6755fd269d crypto: ecdh - add privkey generation support
Add support for generating ecc private keys.

Generation of ecc private keys is helpful in a user-space to kernel
ecdh offload because the keys are not revealed to user-space. Private
key generation is also helpful to implement forward secrecy.

If the user provides a NULL ecc private key, the kernel will generate it
and further use it for ecdh.

Move ecdh's object files below drbg's. drbg must be present in the kernel
at the time of calling.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Stephan Müller <smueller@chronox.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-06-10 12:04:35 +08:00
Tudor-Dan Ambarus
d0efb48b86 crypto: ecdh - comply with crypto_kpp_maxsize()
crypto_kpp_maxsize() asks for the output buffer size without
caring for errors. It allways assume that will be called after
a valid setkey. Comply with it and return what he wants.

nbytes has no sense now, remove it and directly return the maxsize.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-06-10 12:04:28 +08:00
Tudor-Dan Ambarus
7380c56d2f crypto: ecc - rename ecdh_make_pub_key()
Rename ecdh_make_pub_key() to ecc_make_pub_key().
ecdh_make_pub_key() is not dh specific and the reference
to dh is wrong.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-06-10 12:04:26 +08:00
Tudor-Dan Ambarus
ad26959710 crypto: ecc - remove unnecessary casts
ecc software implementation works with chunks of u64 data. There were some
unnecessary casts to u8 and then back to u64 for the ecc keys. This patch
removes the unnecessary casts.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-06-10 12:04:26 +08:00
Tudor-Dan Ambarus
099054d735 crypto: ecc - remove unused function arguments
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-06-10 12:04:25 +08:00
Tudor-Dan Ambarus
c0ca1215dc crypto: kpp, (ec)dh - fix typos
While here, add missing argument description (ndigits).

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-06-10 12:04:25 +08:00
Eric Biggers
5527dfb6dd crypto: kpp - constify buffer passed to crypto_kpp_set_secret()
Constify the buffer passed to crypto_kpp_set_secret() and
kpp_alg.set_secret, since it is never modified.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2017-03-09 18:34:27 +08:00
Stephen Rothwell
8f44df154d crypto: ecdh - make ecdh_shared_secret unique
There is another ecdh_shared_secret in net/bluetooth/ecc.c

Fixes: 3c4b23901a ("crypto: ecdh - Add ECDH software support")
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2016-06-24 21:24:59 +08:00
Salvatore Benedetto
3c4b23901a crypto: ecdh - Add ECDH software support
* Implement ECDH under kpp API
 * Provide ECC software support for curve P-192 and
   P-256.
 * Add kpp test for ECDH with data generated by OpenSSL

Signed-off-by: Salvatore Benedetto <salvatore.benedetto@intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2016-06-23 18:29:57 +08:00