lkml.org 
[lkml]   [2024]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 1/2] crypto: ecdh - Pass private key in proper byte order to check valid key
    On Thu, Apr 18, 2024 at 11:24:44AM -0400, Stefan Berger wrote:
    > ecc_is_key_valid expects a key with the most significant digit in the last
    > entry of the digit array. Currently ecdh_set_secret passes a reversed key
    > to ecc_is_key_valid that then passes the rather simple test checking
    > whether the private key is in range [2, n-3]. For all current ecdh-
    > supported curves (NIST P192/256/384) the 'n' parameter is a rather large
    > number, therefore easily passing this test.
    >
    > Throughout the ecdh and ecc codebase the variable 'priv' is used for a
    > private_key holding the bytes in proper byte order. Therefore, introduce
    > priv in ecdh_set_secret and copy the bytes from ctx->private_key into
    > priv in proper byte order by using ecc_swap_digits. Pass priv to
    > ecc_is_valid_key.
    >
    > Cc: Ard Biesheuvel <ardb@kernel.org>
    > Cc: Salvatore Benedetto <salvatore.benedetto@intel.com>
    > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
    > Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
    > ---
    > crypto/ecdh.c | 11 ++++++++---
    > 1 file changed, 8 insertions(+), 3 deletions(-)
    >
    > diff --git a/crypto/ecdh.c b/crypto/ecdh.c
    > index 3049f147e011..c02c9a2b9682 100644
    > --- a/crypto/ecdh.c
    > +++ b/crypto/ecdh.c
    > @@ -27,7 +27,9 @@ static int ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
    > unsigned int len)
    > {
    > struct ecdh_ctx *ctx = ecdh_get_ctx(tfm);
    > + u64 priv[ECC_MAX_DIGITS];
    > struct ecdh params;
    > + int ret = 0;
    >
    > if (crypto_ecdh_decode_key(buf, len, &params) < 0 ||
    > params.key_size > sizeof(u64) * ctx->ndigits)
    > @@ -40,13 +42,16 @@ static int ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
    > ctx->private_key);
    >
    > memcpy(ctx->private_key, params.key, params.key_size);
    > + ecc_swap_digits(ctx->private_key, priv, ctx->ndigits);

    These functions need to use our sparse marking mechanism correctly
    to prevent future occurences of such errors. They should not be
    passing around void * pointers or worse, treating u64 * arrays as
    big-endian.

    Cheers,
    --
    Email: Herbert Xu <herbert@gondor.apana.org.au>
    Home Page: http://gondor.apana.org.au/~herbert/
    PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

    \
     
     \ /
      Last update: 2024-05-27 18:04    [W:4.521 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site