| Message ID | 20240312183618.1211745-3-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for NIST P521 to ecdsa | expand |
On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.ibm.com> > > For NIST P192/256/384 the public key's x and y parameters could be copied > directly from a given array since both parameters filled 'ndigits' of We "could" do various things but a commit message should describe the gain or motivation of doing something, even if you think it should be obvious. > digits (a 'digit' is a u64). For support of NIST P521 the key parameters > need to have leading zeros prepended to the most significant digit since > only 2 bytes of the most significant digit are provided. > > Therefore, implement ecc_digits_from_bytes to convert a byte array into an > array of digits and use this function in ecdsa_set_pub_key where an input > byte array needs to be converted into digits. Please, dscribe the format of "array of digits" instead of waving hands here. > > Suggested-by: Lukas Wunner <lukas@wunner.de> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > Tested-by: Lukas Wunner <lukas@wunner.de> > --- > crypto/ecdsa.c | 14 +++++++++----- > include/crypto/internal/ecc.h | 21 +++++++++++++++++++++ > 2 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c > index fbd76498aba8..6653dec17327 100644 > --- a/crypto/ecdsa.c > +++ b/crypto/ecdsa.c > @@ -222,9 +222,8 @@ static int ecdsa_ecc_ctx_reset(struct ecc_ctx *ctx) > static int ecdsa_set_pub_key(struct crypto_akcipher *tfm, const void *key, unsigned int keylen) > { > struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm); > + unsigned int digitlen, ndigits; > const unsigned char *d = key; > - const u64 *digits = (const u64 *)&d[1]; > - unsigned int ndigits; > int ret; > > ret = ecdsa_ecc_ctx_reset(ctx); > @@ -238,12 +237,17 @@ static int ecdsa_set_pub_key(struct crypto_akcipher *tfm, const void *key, unsig > return -EINVAL; > > keylen--; > - ndigits = (keylen >> 1) / sizeof(u64); > + digitlen = keylen >> 1; > + > + ndigits = DIV_ROUND_UP(digitlen, sizeof(u64)); > if (ndigits != ctx->curve->g.ndigits) > return -EINVAL; > > - ecc_swap_digits(digits, ctx->pub_key.x, ndigits); > - ecc_swap_digits(&digits[ndigits], ctx->pub_key.y, ndigits); > + d++; This pointer increment is more confusing... > + > + ecc_digits_from_bytes(d, digitlen, ctx->pub_key.x, ndigits); ...than "&d[1]" which would also match better the earlier code. > + ecc_digits_from_bytes(&d[digitlen], digitlen, ctx->pub_key.y, ndigits); > + > ret = ecc_is_pubkey_valid_full(ctx->curve, &ctx->pub_key); > > ctx->pub_key_set = ret == 0; > diff --git a/include/crypto/internal/ecc.h b/include/crypto/internal/ecc.h > index 4f6c1a68882f..ab722a8986b7 100644 > --- a/include/crypto/internal/ecc.h > +++ b/include/crypto/internal/ecc.h > @@ -56,6 +56,27 @@ static inline void ecc_swap_digits(const void *in, u64 *out, unsigned int ndigit > out[i] = get_unaligned_be64(&src[ndigits - 1 - i]); > } > > +/** > + * ecc_digits_from_bytes() - Create ndigits-sized digits array from byte array > + * @in: Input byte array > + * @nbytes Size of input byte array > + * @out Output digits array > + * @ndigits: Number of digits to create from byte array > + */ > +static inline void ecc_digits_from_bytes(const u8 *in, unsigned int nbytes, > + u64 *out, unsigned int ndigits) > +{ > + unsigned int o = nbytes & 7; > + __be64 msd = 0; > + > + if (o) { > + memcpy((u8 *)&msd + sizeof(msd) - o, in, o); > + out[--ndigits] = be64_to_cpu(msd); > + in += o; > + } > + ecc_swap_digits(in, out, ndigits); > +} > + > /** > * ecc_is_key_valid() - Validate a given ECDH private key > * BR, Jarkko
On Mon, Mar 18, 2024 at 10:21:20PM +0200, Jarkko Sakkinen wrote: > On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote: > > From: Stefan Berger <stefanb@linux.ibm.com> > > > > For NIST P192/256/384 the public key's x and y parameters could be copied > > directly from a given array since both parameters filled 'ndigits' of > > We "could" do various things but a commit message should describe the > gain or motivation of doing something, even if you think it should be > obvious. "could" is past participle of the verb "can". We were able to copy directly but no longer can due to the odd key size of P521. Thanks, Lukas
On Mon Mar 18, 2024 at 10:35 PM EET, Lukas Wunner wrote: > On Mon, Mar 18, 2024 at 10:21:20PM +0200, Jarkko Sakkinen wrote: > > On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote: > > > From: Stefan Berger <stefanb@linux.ibm.com> > > > > > > For NIST P192/256/384 the public key's x and y parameters could be copied > > > directly from a given array since both parameters filled 'ndigits' of > > > > We "could" do various things but a commit message should describe the > > gain or motivation of doing something, even if you think it should be > > obvious. > > "could" is past participle of the verb "can". We were able to copy > directly but no longer can due to the odd key size of P521. OK this clarifies, thanks. BR, Jarkko
diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c index fbd76498aba8..6653dec17327 100644 --- a/crypto/ecdsa.c +++ b/crypto/ecdsa.c @@ -222,9 +222,8 @@ static int ecdsa_ecc_ctx_reset(struct ecc_ctx *ctx) static int ecdsa_set_pub_key(struct crypto_akcipher *tfm, const void *key, unsigned int keylen) { struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm); + unsigned int digitlen, ndigits; const unsigned char *d = key; - const u64 *digits = (const u64 *)&d[1]; - unsigned int ndigits; int ret; ret = ecdsa_ecc_ctx_reset(ctx); @@ -238,12 +237,17 @@ static int ecdsa_set_pub_key(struct crypto_akcipher *tfm, const void *key, unsig return -EINVAL; keylen--; - ndigits = (keylen >> 1) / sizeof(u64); + digitlen = keylen >> 1; + + ndigits = DIV_ROUND_UP(digitlen, sizeof(u64)); if (ndigits != ctx->curve->g.ndigits) return -EINVAL; - ecc_swap_digits(digits, ctx->pub_key.x, ndigits); - ecc_swap_digits(&digits[ndigits], ctx->pub_key.y, ndigits); + d++; + + ecc_digits_from_bytes(d, digitlen, ctx->pub_key.x, ndigits); + ecc_digits_from_bytes(&d[digitlen], digitlen, ctx->pub_key.y, ndigits); + ret = ecc_is_pubkey_valid_full(ctx->curve, &ctx->pub_key); ctx->pub_key_set = ret == 0; diff --git a/include/crypto/internal/ecc.h b/include/crypto/internal/ecc.h index 4f6c1a68882f..ab722a8986b7 100644 --- a/include/crypto/internal/ecc.h +++ b/include/crypto/internal/ecc.h @@ -56,6 +56,27 @@ static inline void ecc_swap_digits(const void *in, u64 *out, unsigned int ndigit out[i] = get_unaligned_be64(&src[ndigits - 1 - i]); } +/** + * ecc_digits_from_bytes() - Create ndigits-sized digits array from byte array + * @in: Input byte array + * @nbytes Size of input byte array + * @out Output digits array + * @ndigits: Number of digits to create from byte array + */ +static inline void ecc_digits_from_bytes(const u8 *in, unsigned int nbytes, + u64 *out, unsigned int ndigits) +{ + unsigned int o = nbytes & 7; + __be64 msd = 0; + + if (o) { + memcpy((u8 *)&msd + sizeof(msd) - o, in, o); + out[--ndigits] = be64_to_cpu(msd); + in += o; + } + ecc_swap_digits(in, out, ndigits); +} + /** * ecc_is_key_valid() - Validate a given ECDH private key *