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
> -----Original Message-----
> From: Lukas Wunner <[email protected]>
> Sent: Monday, March 18, 2024 12:36 PM
> To: Bharat Bhushan <[email protected]>
> Cc: Stefan Berger <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Stefan Berger
> <[email protected]>
> 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