Re: [PATCH v9 3/7] crypto: move curve_id of ECDH from the key to algorithm name

[yumeng]

在 2021/2/24 18:15, Tudor.Ambarus@xxxxxxxxxxxxx 写道:
> On 2/24/21 3:29 AM, yumeng wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > 在 2021/2/23 18:44, Tudor.Ambarus@xxxxxxxxxxxxx 写道:
> > > Hi,
> > >
> > > On 2/23/21 9:10 AM, Meng Yu wrote:
> > > > --- a/drivers/crypto/atmel-ecc.c
> > > > +++ b/drivers/crypto/atmel-ecc.c
> > > > @@ -104,7 +104,7 @@ static int atmel_ecdh_set_secret(struct crypto_kpp *tfm, const void *buf,
> > > >                   return -EINVAL;
> > > >           }
> > > >
> > > > -       ctx->n_sz = atmel_ecdh_supported_curve(params.curve_id);
> > > > +       ctx->n_sz = atmel_ecdh_supported_curve(ctx->curve_id);
> > > >           if (!ctx->n_sz || params.key_size) {
> > > >                   /* fallback to ecdh software implementation */
> > > >                   ctx->do_fallback = true;
> > >
> > > Now that you moved the curve id info into the alg name, and it is
> > > no longer dynamically discovered when decoding the key, does it
> > > still make sense to keep the curve id, the key size checks, and
> > > the fallback to the software implementation?
> > > I think we can keep the curve id, the key size check if 'atmel-ecc' may
> > support other curves in the future, and if you're sure P256 is the only
> > curve 'atmel-ecc' uses, and it will be changed, we can delete it.
> >
> > And fallback to ecdh software implementation is needed when
> > params.key_size is zero.
>
> I've read the code again, now I remember. The fallback is needed indeed,
> but for other reason. ecdh-nist-p256 will be handled in the crypto IP
> only when its user provides a zero length private key: we will use a
> pre-provisioned private key that can't be read outside of the device.
> The fallback was needed for ecdh-nist-p256 when the user provides a
> non-zero private key, or for other curve IDs.
>
> Since the atecc508 and atecc608 (for which there isn't support in kernel
> as of now) both support just NIST Standard P256 Elliptic Curve, and the
> curve id is now unique per alg, the ctx->curve_id handling does no longer
> make sense. So please remove the ctx->curve_id handling. ctx->n_sz can be
> removed too and use instead directly ATMEL_ECC_NIST_P256_N_SIZE, similar
> to how ATMEL_ECC_PUBKEY_SIZE is used.

OK, thanks, I will remove it. But I'd like to wait 2 more days, as maybe 
the owners of other modules (like crypto or bluetooth) have any comments.

thanks.