| Message ID | 4348752.LvFx2qVVIh@positron.chronox.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | DH: SP800-56A rev 3 compliant validation checks | expand |
On Sun, Jul 12, 2020 at 06:39:26PM +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 Reviewed-by: Vitaly Chikunov <vt@altlinux.org> > 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 > > >
Reviewed-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com> Tested-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com> On Sun, Jul 12, 2020 at 06:39:26PM +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 > > > >
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(-)