Received-SPF: pass (google.com: domain of linux-crypto+bounces-2134-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <c7feceb3a7816c2f8686a907fbea2028477464a0.camel@redhat.com>
Subject: Re: [PATCH v2 00/14] Add support for NIST P521 to ecdsa and ecdh
From: Simo Sorce <simo@redhat.com>
To: Stefan Berger <stefanb@linux.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
Date: Fri, 16 Feb 2024 14:40:23 -0500
In-Reply-To: <b507fd52-a807-4325-981b-3852f4f6190b@linux.ibm.com>
References: <20240215231414.3857320-1-stefanb@linux.ibm.com>
	 <3bdb1c9e0ac35c7dc3fbba1233bc7df80ac466a2.camel@redhat.com>
	 <b507fd52-a807-4325-981b-3852f4f6190b@linux.ibm.com>
Organization: Red Hat
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
Precedence: bulk
MIME-Version: 1.0

On Fri, 2024-02-16 at 14:32 -0500, Stefan Berger wrote:
>=20
> On 2/16/24 14:27, Simo Sorce wrote:
> > On Thu, 2024-02-15 at 18:13 -0500, Stefan Berger wrote:
> > > This series of patches adds support for the NIST P521 curve to ecdsa =
and
> > > ecdh. Test cases for NIST P521 are added to both modules.
> > >=20
> > > An issue with the current code in ecdsa and ecdh is that it assumes t=
hat
> > > input arrays providing key coordinates for example, are arrays of dig=
its
> > > (a 'digit' is a 'u64'). This works well for all currently supported
> > > curves, such as NIST P192/256/384, but does not work for NIST P521 wh=
ere
> > > coordinates are 8 digits + 2 bytes long. So some of the changes deal =
with
> > > converting byte arrays to digits and digits to byte arrays.
> > >=20
> > >=20
> > > Regards,
> > >     Stefan
> > >=20
> > > v2:
> > >   - Reformulated some patch descriptions
> > >   - Fixed issue detected by krobot
> > >   - Some other small changes to the code
> > >=20
> > > Stefan Berger (14):
> > >    crypto: ecdsa - Convert byte arrays with key coordinates to digits
> > >    crypto: ecdsa - Adjust tests on length of key parameters
> > >    crypto: ecdsa - Extend res.x mod n calculation for NIST P521
> > >    crypto: ecc - Implement vli_mmod_fast_521 for NIST p521
> > >    crypto: ecc - For NIST P521 use vli_num_bits to get number of bits
> > >    crypto: ecc - Add NIST P521 curve parameters
> > >    crypto: ecdsa - Register NIST P521 and extend test suite
> > >    x509: Add OID for NIST P521 and extend parser for it
> > >    crypto: ecdh - Use properly formatted digits to check for valid ke=
y
> > >    crypto: ecc - Implement ecc_digits_to_bytes to convert digits to b=
yte
> > >      array
> > >    crypto: Add nbits field to ecc_curve structure
> > >    crypto: ecc - Implement and use ecc_curve_get_nbytes to get curve'=
s
> > >      nbytes
> > >    crypto: ecdh - Use functions to copy digits from and to byte array
> > >    crypto: ecdh - Add support for NIST P521 and add test case
> > >=20
> > >   crypto/asymmetric_keys/x509_cert_parser.c |   3 +
> > >   crypto/ecc.c                              |  71 +++++--
> > >   crypto/ecc_curve_defs.h                   |  45 +++++
> > >   crypto/ecdh.c                             |  59 +++++-
> > >   crypto/ecdsa.c                            |  48 ++++-
> > >   crypto/testmgr.c                          |  14 ++
> > >   crypto/testmgr.h                          | 225 +++++++++++++++++++=
+++
> > >   include/crypto/ecc_curve.h                |   3 +
> > >   include/crypto/ecdh.h                     |   1 +
> > >   include/crypto/internal/ecc.h             |  61 +++++-
> > >   include/linux/oid_registry.h              |   1 +
> > >   11 files changed, 495 insertions(+), 36 deletions(-)
> >=20
> > Hi Stefan,
> > what kind of side-channel testing was performed on this code?
> > And what is the use case you are adding it for?
>=20
> We're using public keys for signature verification. I am not aware that
> public key usage is critical to side channels.
>=20
> The use case for adding it is primarily driven by closing a gap to=20
> complete the support for the common ECDSA NIST curves.

Is there an assumption the ECDH code uses exclusively ephemeral keys?

Simo.

--=20
Simo Sorce
Distinguished Engineer
RHEL Crypto Team
Red Hat, Inc