Re: [PATCH 3/3] add nist_p384 register and unregister to support nist_p384
From: Stefan Berger
Date: Fri Feb 19 2021 - 14:20:38 EST
On 2/19/21 1:57 PM, Saulo Alessandre wrote:
From: Saulo Alessandre <saulo.alessandre@xxxxxxxxxx>
* 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.