Messages in this thread | | | Subject | Re: [PATCH 3/3] add nist_p384 register and unregister to support nist_p384 | From | Stefan Berger <> | Date | Fri, 19 Feb 2021 14:19:12 -0500 |
| |
On 2/19/21 1:57 PM, Saulo Alessandre wrote: > From: Saulo Alessandre <saulo.alessandre@tse.jus.br> > > * crypto/ecdsa.c > - add ecdsa_nist_p384_init_tfm > - register and unregister p384 tfm > > * crypto/testmgr.c > - add test vector for p384 on vector of tests > > * crypto/testmgr.h > - add test vector params for p384(sha1, sha224, sha256, sha384 and sha512) > --- > crypto/asymmetric_keys/x509_cert_parser.c | 2 +- > crypto/ecc.c | 2 +- > crypto/ecc.h | 2 +- > crypto/ecdsa.c | 46 +++++-- > crypto/testmgr.c | 6 + > crypto/testmgr.h | 157 ++++++++++++++++++++++ > 6 files changed, 204 insertions(+), 11 deletions(-) > > diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c > index 03535bd8b8ef..1d94c23e9678 100644 > --- a/crypto/asymmetric_keys/x509_cert_parser.c > +++ b/crypto/asymmetric_keys/x509_cert_parser.c > @@ -278,7 +278,7 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, > ecdsa: > ctx->cert->sig->pkey_algo = "ecdsa"; > ctx->cert->sig->encoding = "x962"; > - pr_info("Found stephan %s(%s) X509 certificate\n", ctx->cert->sig->pkey_algo, > + pr_info("Found %s(%s) X509 certificate\n", ctx->cert->sig->pkey_algo, > ctx->cert->sig->hash_algo);
This patch doesn't apply on my tree because of this change.
> ctx->algo_oid = ctx->last_oid; > return 0; > diff --git a/crypto/ecc.c b/crypto/ecc.c > index aab57dcf26c6..0f41ccc10ceb 100644 > --- a/crypto/ecc.c > +++ b/crypto/ecc.c > @@ -904,7 +904,7 @@ static bool vli_mmod_fast(u64 *result, u64 *product, > const unsigned int ndigits = curve->g.ndigits; > > /* Currently, all NIST have name nist_.* */ > - if (strncmp(curve->name, "nist_", 5) != 0) { > + if (curve && curve->name && strncmp(curve->name, "nist_", 5) != 0) {
Can you actually call this function with curve = NULL? You already accessed 'curve' above by ndigits = curve->g.ndigits , so if that check was necessary it's too late to avoid a NULL pointer exception.
> /* Try to handle Pseudo-Marsenne primes. */ > if (curve_prime[ndigits - 1] == -1ull) { > vli_mmod_special(result, product, curve_prime, > diff --git a/crypto/ecc.h b/crypto/ecc.h > index 861de67b538f..9a668594012b 100644 > --- a/crypto/ecc.h > +++ b/crypto/ecc.h > @@ -30,7 +30,7 @@ > #define ECC_CURVE_NIST_P192_DIGITS 3 > #define ECC_CURVE_NIST_P256_DIGITS 4 > #define ECC_CURVE_NIST_P384_DIGITS 6 > -#define ECC_MAX_DIGITS (ECC_CURVE_NIST_P384_DIGITS) > +#define ECC_MAX_DIGITS (512/64) > > #define ECC_DIGITS_TO_BYTES_SHIFT 3 > > diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c > index 4b45230276b3..4dfbf8f32a0b 100644 > --- a/crypto/ecdsa.c > +++ b/crypto/ecdsa.c > @@ -101,7 +101,7 @@ int ecdsa_get_signature_r(void *context, size_t hdrlen, unsigned char tag, > struct ecdsa_signature_ctx *sig = context; > > return ecdsa_get_signature_rs(sig->r, hdrlen, tag, value, vlen, > - sig->curve->g.ndigits); > + sig->curve->g.ndigits);
I don't think this reformatting and the ones below are necessary here.
> } > > int ecdsa_get_signature_s(void *context, size_t hdrlen, unsigned char tag, > @@ -110,7 +110,7 @@ int ecdsa_get_signature_s(void *context, size_t hdrlen, unsigned char tag, > struct ecdsa_signature_ctx *sig = context; > > return ecdsa_get_signature_rs(sig->s, hdrlen, tag, value, vlen, > - sig->curve->g.ndigits); > + sig->curve->g.ndigits); > } > > static int _ecdsa_verify(struct ecc_ctx *ctx, const u64 *hash, > @@ -127,7 +127,7 @@ static int _ecdsa_verify(struct ecc_ctx *ctx, const u64 *hash, > > /* 0 < r < n and 0 < s < n */ > if (vli_is_zero(r, ndigits) || vli_cmp(r, curve->n, ndigits) >= 0 || > - vli_is_zero(s, ndigits) || vli_cmp(s, curve->n, ndigits) >= 0) > + vli_is_zero(s, ndigits) || vli_cmp(s, curve->n, ndigits) >= 0) > return -EBADMSG; > > /* hash is given */ > @@ -183,7 +183,7 @@ static int ecdsa_verify(struct akcipher_request *req) > buffer, req->src_len + req->dst_len, 0); > > ret = asn1_ber_decoder(&ecdsasignature_decoder, &sig_ctx, > - buffer, req->src_len); > + buffer, req->src_len); > if (ret < 0) > goto error; > > @@ -233,17 +233,19 @@ static int ecdsa_ecc_ctx_reset(struct ecc_ctx *ctx) > ret = ecdsa_ecc_ctx_init(ctx, curve_id); > if (ret == 0) > ctx->pub_key = ECC_POINT_INIT(ctx->x, ctx->y, > - ctx->curve->g.ndigits); > + ctx->curve->g.ndigits); > return ret; > } > > +#define UNPACKED_KEY_ID 0x04 > + > /* > * Set the public key given the raw uncompressed key data from an X509 > * certificate. The key data contain the concatenated X and Y coordinates of > * the public key. > */ > static int ecdsa_set_pub_key(struct crypto_akcipher *tfm, > - const void *key, unsigned int keylen) > + const void *key, unsigned int keylen)
I don't that's necessary here.
> { > struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm); > const unsigned char *d = key; > @@ -258,7 +260,7 @@ static int ecdsa_set_pub_key(struct crypto_akcipher *tfm, > if (keylen < 1 || (((keylen - 1) >> 1) % sizeof(u64)) != 0) > return -EINVAL; > /* we only accept uncompressed format */ > - if (d[0] != 4) > + if (d[0] != UNPACKED_KEY_ID) > return -EINVAL;
I should probably introduce a constant in my v8 then.
> > keylen--; > @@ -289,6 +291,28 @@ static unsigned int ecdsa_max_size(struct crypto_akcipher *tfm) > return ctx->pub_key.ndigits << ECC_DIGITS_TO_BYTES_SHIFT; > } > > +static int ecdsa_nist_p384_init_tfm(struct crypto_akcipher *tfm) > +{ > + struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm); > + > + return ecdsa_ecc_ctx_init(ctx, ECC_CURVE_NIST_P384); > +} > + > +static struct akcipher_alg ecdsa_nist_p384 = { > + .verify = ecdsa_verify, > + .set_pub_key = ecdsa_set_pub_key, > + .max_size = ecdsa_max_size, > + .init = ecdsa_nist_p384_init_tfm, > + .exit = ecdsa_exit_tfm, > + .base = { > + .cra_name = "ecdsa-nist-p384", > + .cra_driver_name = "ecdsa-nist-p384-generic", > + .cra_priority = 100, > + .cra_module = THIS_MODULE, > + .cra_ctxsize = sizeof(struct ecc_ctx), > + }, > +}; > + > static int ecdsa_nist_p256_init_tfm(struct crypto_akcipher *tfm) > { > struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm); > @@ -342,7 +366,12 @@ static int ecdsa_init(void) > ret = crypto_register_akcipher(&ecdsa_nist_p192); > ecdsa_nist_p192_registered = ret == 0; > > - return crypto_register_akcipher(&ecdsa_nist_p256); > + ret = crypto_register_akcipher(&ecdsa_nist_p256); > + if (ret != 0) > + return ret;
There is an existing bug in my v8. I have to unregister nist_p192 if nist_p256 registration fails. Let me fix this in a V9.
| |