2020-07-20 17:12:26

by Stephan Müller

Subject: [PATCH v3 1/5] crypto: ECDH - check validity of Z before export

SP800-56A rev3 section 5.7.1.2 step 2 mandates that the validity of the
calculated shared secret is verified before the data is returned to the
caller. Thus, the export function and the validity check functions are
reversed. In addition, the sensitive variables of priv and rand_z are
zeroized.

Signed-off-by: Stephan Mueller <[email protected]>
---
crypto/ecc.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/crypto/ecc.c b/crypto/ecc.c
index 02d35be7702b..52e2d49262f2 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -1495,11 +1495,16 @@ int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits,

ecc_point_mult(product, pk, priv, rand_z, curve, ndigits);

- ecc_swap_digits(product->x, secret, ndigits);
-
- if (ecc_point_is_zero(product))
+ if (ecc_point_is_zero(product)) {
ret = -EFAULT;
+ goto err_validity;
+ }
+
+ ecc_swap_digits(product->x, secret, ndigits);

+err_validity:
+ memzero_explicit(priv, sizeof(priv));
+ memzero_explicit(rand_z, sizeof(rand_z));
ecc_free_point(product);
err_alloc_product:
ecc_free_point(pk);
--
2.26.2

2020-07-22 13:12:02

by Vitaly Chikunov

Subject: Re: [PATCH v3 1/5] crypto: ECDH - check validity of Z before export

On Mon, Jul 20, 2020 at 07:07:48PM +0200, Stephan Müller wrote:
> SP800-56A rev3 section 5.7.1.2 step 2 mandates that the validity of the
> calculated shared secret is verified before the data is returned to the
> caller. Thus, the export function and the validity check functions are
> reversed. In addition, the sensitive variables of priv and rand_z are
> zeroized.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> crypto/ecc.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)

This patch seems not changed from v2, thus

Reviewed-by: Vitaly Chikunov <[email protected]>

>
> diff --git a/crypto/ecc.c b/crypto/ecc.c
> index 02d35be7702b..52e2d49262f2 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -1495,11 +1495,16 @@ int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits,
>
> ecc_point_mult(product, pk, priv, rand_z, curve, ndigits);
>
> - ecc_swap_digits(product->x, secret, ndigits);
> -
> - if (ecc_point_is_zero(product))
> + if (ecc_point_is_zero(product)) {
> ret = -EFAULT;
> + goto err_validity;
> + }
> +
> + ecc_swap_digits(product->x, secret, ndigits);
>
> +err_validity:
> + memzero_explicit(priv, sizeof(priv));
> + memzero_explicit(rand_z, sizeof(rand_z));
> ecc_free_point(product);
> err_alloc_product:
> ecc_free_point(pk);
> --
> 2.26.2
>
>
>

2020-07-24 17:48:43

by Neil Horman

Subject: Re: [PATCH v3 1/5] crypto: ECDH - check validity of Z before export

On Mon, Jul 20, 2020 at 07:07:48PM +0200, Stephan M?ller wrote:
> SP800-56A rev3 section 5.7.1.2 step 2 mandates that the validity of the
> calculated shared secret is verified before the data is returned to the
> caller. Thus, the export function and the validity check functions are
> reversed. In addition, the sensitive variables of priv and rand_z are
> zeroized.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> crypto/ecc.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/ecc.c b/crypto/ecc.c
> index 02d35be7702b..52e2d49262f2 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -1495,11 +1495,16 @@ int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits,
>
> ecc_point_mult(product, pk, priv, rand_z, curve, ndigits);
>
> - ecc_swap_digits(product->x, secret, ndigits);
> -
> - if (ecc_point_is_zero(product))
> + if (ecc_point_is_zero(product)) {
> ret = -EFAULT;
> + goto err_validity;
> + }
> +
> + ecc_swap_digits(product->x, secret, ndigits);
>
> +err_validity:
> + memzero_explicit(priv, sizeof(priv));
> + memzero_explicit(rand_z, sizeof(rand_z));
> ecc_free_point(product);
> err_alloc_product:
> ecc_free_point(pk);
> --
> 2.26.2
>
>
>
>
Acked-by: Neil Horman <[email protected]>