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

[Lukas Wunner <lukas@wunner.de>]

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.

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