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

[Bharat Bhushan]

> -----Original Message-----
> From: Lukas Wunner <lukas@xxxxxxxxx>
> Sent: Monday, March 18, 2024 12:36 PM
> To: Bharat Bhushan <bbhushan2@xxxxxxxxxxx>
> Cc: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>; keyrings@xxxxxxxxxxxxxxx;
> linux-crypto@xxxxxxxxxxxxxxx; herbert@xxxxxxxxxxxxxxxxxxx;
> davem@xxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> saulo.alessandre@xxxxxxxxxx; jarkko@xxxxxxxxxx; Stefan Berger
> <stefanb@xxxxxxxxxxxxx>
> 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