RE: [PATCH v6 12/13] crypto: asymmetric_keys - Adjust signature size calculation for NIST P521

Bharat Bhushan
2024-03-19T03:38:37Z

> -----Original Message-----
> From: Lukas Wunner <lukas@wunner.de>
> Sent: Monday, March 18, 2024 12:36 PM
> To: Bharat Bhushan <bbhushan2@marvell.com>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>; keyrings@vger.kernel.org;
> linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au;
> davem@davemloft.net; linux-kernel@vger.kernel.org;
> saulo.alessandre@tse.jus.br; jarkko@kernel.org; Stefan Berger
> <stefanb@linux.ibm.com>
> Subject: Re: [EXTERNAL] [PATCH v6 12/13] crypto: asymmetric_keys - Adjust
> signature size calculation for NIST P521
>
> On Mon, Mar 18, 2024 at 05:58:23AM +0000, Bharat Bhushan wrote:
> > > --- a/crypto/asymmetric_keys/public_key.c
> > > +++ b/crypto/asymmetric_keys/public_key.c
> > > @@ -233,6 +233,7 @@ static int software_key_query(const struct
> > > kernel_pkey_params *params,
> > > info->key_size = len * 8;
> > >
> > > if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
> > > + int slen = len;
> > > /*
> > > * ECDSA key sizes are much smaller than RSA, and thus could
> > > * operate on (hashed) inputs that are larger than key size.
> > > @@ -246,8 +247,19 @@ static int software_key_query(const struct
> > > kernel_pkey_params *params,
> > > * Verify takes ECDSA-Sig (described in RFC 5480) as input,
> > > * which is actually 2 'key_size'-bit integers encoded in
> > > * ASN.1. Account for the ASN.1 encoding overhead here.
> > > + *
> > > + * NIST P192/256/384 may prepend a '0' to a coordinate to
> > > + * indicate a positive integer. NIST P521 never needs it.
> > > */
> > > - info->max_sig_size = 2 * (len + 3) + 2;
> > > + if (strcmp(pkey->pkey_algo, "ecdsa-nist-p521") != 0)
> > > + slen += 1;
> > > + /* Length of encoding the x & y coordinates */
> > > + slen = 2 * (slen + 2);
> > > + /*
> > > + * If coordinate encoding takes at least 128 bytes then an
> > > + * additional byte for length encoding is needed.
> > > + */
> > > + info->max_sig_size = 1 + (slen >= 128) + 1 + slen;
> >
> > Is "(slen >= 128)" valid for P192/256/384 also?
>
> It is valid but never true for those.

Okay, just want to check if that was valid for P192/256/384 and this patch is fixing same as well.

Otherwise looks good to me as well.

Thanks
-Bharat

>
> The signature consists of two integers encoded in ASN.1.
> So each integer is prepended by 1 byte for the tag and 1 byte for the length.
>
> The two integers are bundled together in a "sequence", which in turn requires
> 1 byte for the tag and 1 byte for the length. However, for P521 the length of
> the sequence is at least 2*(1+1+66) = 136 bytes, which exceeds 128 bytes and
> therefore the length of the sequence occupies 2 bytes instead of 1.
>
> For the shorter key lengths, the sequence fits in less than 128 bytes and does
> not require the extra byte for the sequence length.
>
> So the code is fine AFAICS.
>
> Thanks,
>
> Lukas