Received-SPF: pass (google.com: domain of linux-crypto+bounces-2729-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Mon, 18 Mar 2024 23:12:10 +0200
Message-Id: <CZX6OOKP5RQD.3BN0EDSAF7QXR@kernel.org>
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Stefan Berger" <stefanb@linux.vnet.ibm.com>,
 <keyrings@vger.kernel.org>, <linux-crypto@vger.kernel.org>,
 <herbert@gondor.apana.org.au>, <davem@davemloft.net>
Cc: <linux-kernel@vger.kernel.org>, <saulo.alessandre@tse.jus.br>,
 <lukas@wunner.de>, <bbhushan2@marvell.com>, "Stefan Berger"
 <stefanb@linux.ibm.com>
Subject: Re: [PATCH v6 12/13] crypto: asymmetric_keys - Adjust signature
 size calculation for NIST P521
References: <20240312183618.1211745-1-stefanb@linux.vnet.ibm.com>
 <20240312183618.1211745-13-stefanb@linux.vnet.ibm.com>
In-Reply-To: <20240312183618.1211745-13-stefanb@linux.vnet.ibm.com>

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Adjust the calculation of the maximum signature size for support of
> NIST P521. While existing curves may prepend a 0 byte to their coordinate=
s
> (to make the number positive), NIST P521 will not do this since only the
> first bit in the most significant byte is used.
>
> If the encoding of the x & y coordinates requires at least 128 bytes then
> an additional byte is needed for the encoding of the length. Take this in=
to
> account when calculating the maximum signature size.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/asymmetric_keys/public_key.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys=
/public_key.c
> index e5f22691febd..16cc0be28929 100644
> --- 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_pke=
y_params *params,
>  	info->key_size =3D len * 8;
> =20
>  	if (strncmp(pkey->pkey_algo, "ecdsa", 5) =3D=3D 0) {
> +		int slen =3D 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_pk=
ey_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 =3D 2 * (len + 3) + 2;
> +		if (strcmp(pkey->pkey_algo, "ecdsa-nist-p521") !=3D 0)
> +			slen +=3D 1;

Just wondering the logic of picking between these:

1. "strncmp"
2. "strcmp"

Now the "ecdsa" is matched with strncmp and "ecdsa-nist-p521" is
compared with strcmp.

So is there a good reason to use different function in these
cases?

I'd guess both could be using strcmp since comparing against
constant...

> +		/* Length of encoding the x & y coordinates */
> +		slen =3D 2 * (slen + 2);
> +		/*
> +		 * If coordinate encoding takes at least 128 bytes then an
> +		 * additional byte for length encoding is needed.
> +		 */
> +		info->max_sig_size =3D 1 + (slen >=3D 128) + 1 + slen;
>  	} else {
>  		info->max_data_size =3D len;
>  		info->max_sig_size =3D len;


BR, Jarkko