Message ID | 20240215231414.3857320-1-stefanb@linux.ibm.com (mailing list archive) |
---|---|
Headers | show |
Series | Add support for NIST P521 to ecdsa and ecdh | expand |
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? Thanks, Simo.
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. Stefan > > Thanks, > Simo. >
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.
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. >