| Message ID | 1759349.tdWV9SEqCh@positron.chronox.de (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | DH: SP800-56A rev 3 compliant validation checks | expand |
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 <smueller@chronox.de> > --- > crypto/ecc.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) This patch seems not changed from v2, thus Reviewed-by: Vitaly Chikunov <vt@altlinux.org> > > 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 > > >
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 <smueller@chronox.de> > --- > 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 <nhorman@redhat.com>
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);
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 <smueller@chronox.de> --- crypto/ecc.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)