[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4aa3459c-e029-40fa-a7f5-f858ba0e3c0f@linux.ibm.com>
Date: Fri, 19 Apr 2024 13:23:35 -0400
From: Stefan Berger <stefanb@...ux.ibm.com>
To: linux-crypto@...r.kernel.org, herbert@...dor.apana.org.au,
davem@...emloft.net
Cc: linux-kernel@...r.kernel.org, jarkko@...nel.org, ardb@...nel.org,
git@...sn.com, hkario@...hat.com, simo@...hat.com,
Salvatore Benedetto <salvatore.benedetto@...el.com>
Subject: Re: [PATCH v3 1/2] crypto: ecdh - Pass private key in proper byte
order to check valid key
On 4/18/24 11:24, 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.
>
Fixes: 3c4b23901a0c ("crypto: ecdh - Add ECDH software support")
> Cc: Ard Biesheuvel <ardb@...nel.org>
> Cc: Salvatore Benedetto <salvatore.benedetto@...el.com>
> Signed-off-by: Stefan Berger <stefanb@...ux.ibm.com>
> Acked-by: Jarkko Sakkinen <jarkko@...nel.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, ¶ms) < 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);
>
> if (ecc_is_key_valid(ctx->curve_id, ctx->ndigits,
> - ctx->private_key, params.key_size) < 0) {
> + priv, params.key_size) < 0) {
> memzero_explicit(ctx->private_key, params.key_size);
> - return -EINVAL;
> + ret = -EINVAL;
> }
> - return 0;
> + memzero_explicit(priv, sizeof(priv));
> +
> + return ret;
> }
>
> static int ecdh_compute_value(struct kpp_request *req)
Powered by blists - more mailing lists