On Fri, 2024-02-16 at 14:32 -0500, Stefan Berger wrote:
>
> 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.
> > >
> > > An issue with the current code in ecdsa and ecdh is that it assumes that
> > > input arrays providing key coordinates for example, are arrays of digits
> > > (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 where
> > > coordinates are 8 digits + 2 bytes long. So some of the changes deal with
> > > converting byte arrays to digits and digits to byte arrays.
> > >
> > >
> > > Regards,
> > > Stefan
> > >
> > > v2:
> > > - Reformulated some patch descriptions
> > > - Fixed issue detected by krobot
> > > - Some other small changes to the code
> > >
> > > 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 key
> > > crypto: ecc - Implement ecc_digits_to_bytes to convert digits to byte
> > > 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
> > >
> > > 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(-)
> >
> > Hi Stefan,
> > what kind of side-channel testing was performed on this code?
> > And what is the use case you are adding it for?
>
> We're using public keys for signature verification. I am not aware that
> public key usage is critical to side channels.
>
> The use case for adding it is primarily driven by closing a gap to
> complete the support for the common ECDSA NIST curves.
Is there an assumption the ECDH code uses exclusively ephemeral keys?
Simo.
--
Simo Sorce
Distinguished Engineer
RHEL Crypto Team
Red Hat, Inc
On 2/16/24 14:40, Simo Sorce wrote:
> On Fri, 2024-02-16 at 14:32 -0500, Stefan Berger wrote:
>>
>> 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.
>>>>
>>>> An issue with the current code in ecdsa and ecdh is that it assumes that
>>>> input arrays providing key coordinates for example, are arrays of digits
>>>> (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 where
>>>> coordinates are 8 digits + 2 bytes long. So some of the changes deal with
>>>> converting byte arrays to digits and digits to byte arrays.
>>>>
>>>>
>>>> Regards,
>>>> Stefan
>>>>
>>>> v2:
>>>> - Reformulated some patch descriptions
>>>> - Fixed issue detected by krobot
>>>> - Some other small changes to the code
>>>>
>>>> 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 key
>>>> crypto: ecc - Implement ecc_digits_to_bytes to convert digits to byte
>>>> 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
>>>>
>>>> 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(-)
>>>
>>> Hi Stefan,
>>> what kind of side-channel testing was performed on this code?
>>> And what is the use case you are adding it for?
>>
>> We're using public keys for signature verification. I am not aware that
>> public key usage is critical to side channels.
>>
>> The use case for adding it is primarily driven by closing a gap to
>> complete the support for the common ECDSA NIST curves.
>
> Is there an assumption the ECDH code uses exclusively ephemeral keys?
>
It can use both, provided keys and ephemeral keys. I think at this point
it's best to drop ecdh support from this series.
Stefan
> Simo.
>