[PATCH v6 00/13] Add support for NIST P521 to ecdsa

[PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

This series adds support for the NIST P521 curve to the ecdsa module
to enable signature verification with it.

An issue with the current code in ecdsa 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 adjusting tests on input byte
array lengths to tolerate arrays not providing multiples of 8 bytes.

Regards,
   Stefan

v6:
 - Use existing #defines for number of digits rather than plain numbers
   (1/13, 6/13) following Bharat's suggestion
 - Initialize result from lowest 521 bits of product rather than going
   through tmp variable (6/13)

v5:
 - Simplified ecc_digits_from_bytes as suggested by Lukas (1/12)
 - Using nbits == 521 to detect NIST P521 curve rather than strcmp()
   (5,6/12)
 - Nits in patch description and comments (11/12)

v4:
 - Followed suggestions by Lukas Wummer (1,5,8/12)
 - Use nbits rather than ndigits where needed (8/12)
 - Renaming 'keylen' variablest to bufsize where necessary (9/12)
 - Adjust signature size calculation for NIST P521 (11/12)

v3:
 - Dropped ecdh support
 - Use ecc_get_curve_nbits for getting number of bits in NIST P521 curve
   in ecc_point_mult (7/10)

v2:
 - Reformulated some patch descriptions
 - Fixed issue detected by krobot
 - Some other small changes to the code



Stefan Berger (13):
  crypto: ecc - Use ECC_CURVE_NIST_P192/256/384_DIGITS where possible
  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 - Add nbits field to ecc_curve structure
  crypto: ecc - Implement vli_mmod_fast_521 for NIST p521
  crypto: ecc - Add special case for NIST P521 in ecc_point_mult
  crypto: ecc - Add NIST P521 curve parameters
  crypto: ecdsa - Replace ndigits with nbits where precision is needed
  crypto: ecdsa - Rename keylen to bufsize where necessary
  crypto: ecdsa - Register NIST P521 and extend test suite
  crypto: asymmetric_keys - Adjust signature size calculation for NIST
    P521
  crypto: x509 - Add OID for NIST P521 and extend parser for it

 crypto/asymmetric_keys/public_key.c       |  14 ++-
 crypto/asymmetric_keys/x509_cert_parser.c |   3 +
 crypto/ecc.c                              |  44 +++++--
 crypto/ecc_curve_defs.h                   |  49 ++++++++
 crypto/ecdsa.c                            |  62 ++++++---
 crypto/ecrdsa_defs.h                      |   5 +
 crypto/testmgr.c                          |   7 ++
 crypto/testmgr.h                          | 146 ++++++++++++++++++++++
 include/crypto/ecc_curve.h                |   2 +
 include/crypto/ecdh.h                     |   1 +
 include/crypto/internal/ecc.h             |  24 +++-
 include/linux/oid_registry.h              |   1 +
 12 files changed, 335 insertions(+), 23 deletions(-)

-- 
2.43.0

[PATCH v6 01/13] crypto: ecc - Use ECC_CURVE_NIST_P192/256/384_DIGITS where possible

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Replace hard-coded numbers with ECC_CURVE_NIST_P192/256/384_DIGITS where
possible.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 crypto/ecc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/crypto/ecc.c b/crypto/ecc.c
index f53fb4d6af99..415a2f4e7291 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -689,7 +689,7 @@ static void vli_mmod_barrett(u64 *result, u64 *product, const u64 *mod,
 static void vli_mmod_fast_192(u64 *result, const u64 *product,
 			      const u64 *curve_prime, u64 *tmp)
 {
-	const unsigned int ndigits = 3;
+	const unsigned int ndigits = ECC_CURVE_NIST_P192_DIGITS;
 	int carry;
 
 	vli_set(result, product, ndigits);
@@ -717,7 +717,7 @@ static void vli_mmod_fast_256(u64 *result, const u64 *product,
 			      const u64 *curve_prime, u64 *tmp)
 {
 	int carry;
-	const unsigned int ndigits = 4;
+	const unsigned int ndigits = ECC_CURVE_NIST_P256_DIGITS;
 
 	/* t */
 	vli_set(result, product, ndigits);
@@ -800,7 +800,7 @@ static void vli_mmod_fast_384(u64 *result, const u64 *product,
 				const u64 *curve_prime, u64 *tmp)
 {
 	int carry;
-	const unsigned int ndigits = 6;
+	const unsigned int ndigits = ECC_CURVE_NIST_P384_DIGITS;
 
 	/* t */
 	vli_set(result, product, ndigits);
@@ -932,13 +932,13 @@ static bool vli_mmod_fast(u64 *result, u64 *product,
 	}
 
 	switch (ndigits) {
-	case 3:
+	case ECC_CURVE_NIST_P192_DIGITS:
 		vli_mmod_fast_192(result, product, curve_prime, tmp);
 		break;
-	case 4:
+	case ECC_CURVE_NIST_P256_DIGITS:
 		vli_mmod_fast_256(result, product, curve_prime, tmp);
 		break;
-	case 6:
+	case ECC_CURVE_NIST_P384_DIGITS:
 		vli_mmod_fast_384(result, product, curve_prime, tmp);
 		break;
 	default:
-- 
2.43.0

[PATCH v6 02/13] crypto: ecdsa - Convert byte arrays with key coordinates to digits

[Stefan Berger]

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
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.

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++;
+
+	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
  *
-- 
2.43.0

[PATCH v6 03/13] crypto: ecdsa - Adjust tests on length of key parameters

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

In preparation for support of NIST P521, adjust the basic tests on the
length of the provided key parameters to only ensure that the length of the
x plus y coordinates parameter array is not an odd number and that each
coordinate fits into an array of 'ndigits' digits. Mathematical tests on
the key's parameters are then done in ecc_is_pubkey_valid_full rejecting
invalid keys.

The change is necessary since NIST P521 keys do not have keys with
coordinates that each fully require 'full' digits (= u64), unlike
NIST P192/256/384 that all require multiple 'full' digits.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/ecdsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
index 6653dec17327..64e1e69d53ba 100644
--- a/crypto/ecdsa.c
+++ b/crypto/ecdsa.c
@@ -230,7 +230,7 @@ static int ecdsa_set_pub_key(struct crypto_akcipher *tfm, const void *key, unsig
 	if (ret < 0)
 		return ret;
 
-	if (keylen < 1 || (((keylen - 1) >> 1) % sizeof(u64)) != 0)
+	if (keylen < 1 || ((keylen - 1) & 1) != 0)
 		return -EINVAL;
 	/* we only accept uncompressed format indicated by '4' */
 	if (d[0] != 4)
-- 
2.43.0

[PATCH v6 04/13] crypto: ecdsa - Extend res.x mod n calculation for NIST P521

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

res.x has been calculated by ecc_point_mult_shamir, which uses
'mod curve_prime'. The curve_prime 'p' is typically larger than the
curve_order 'n' and therefore it is possible that p > res.x >= n.

If res.x >= n then res.x mod n can be calculated by iteratively sub-
tracting n from res.x until n > res.x. For NIST P192/256/384 this can be
done in a single subtraction. This can also be done in a single
subtraction for NIST P521.

The mathematical reason why a single subtraction is sufficient is
due to the values of 'p' and 'n' of the NIST curves where the following
holds true:

   note: max(res.x) = p - 1

   max(res.x) - n < n
       p - 1  - n < n
       p - 1      < 2n  => true for the NIST curves

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/ecdsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
index 64e1e69d53ba..1814f009f971 100644
--- a/crypto/ecdsa.c
+++ b/crypto/ecdsa.c
@@ -122,7 +122,7 @@ static int _ecdsa_verify(struct ecc_ctx *ctx, const u64 *hash, const u64 *r, con
 
 	/* res.x = res.x mod n (if res.x > order) */
 	if (unlikely(vli_cmp(res.x, curve->n, ndigits) == 1))
-		/* faster alternative for NIST p384, p256 & p192 */
+		/* faster alternative for NIST p521, p384, p256 & p192 */
 		vli_sub(res.x, res.x, curve->n, ndigits);
 
 	if (!vli_cmp(res.x, r, ndigits))
-- 
2.43.0

[PATCH v6 05/13] crypto: ecc - Add nbits field to ecc_curve structure

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Add the number of bits a curve has to the ecc_curve definition and set it
on all curve definitions.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/ecc_curve_defs.h    | 4 ++++
 crypto/ecrdsa_defs.h       | 5 +++++
 include/crypto/ecc_curve.h | 2 ++
 3 files changed, 11 insertions(+)

diff --git a/crypto/ecc_curve_defs.h b/crypto/ecc_curve_defs.h
index 9719934c9428..ab1ef3d94be5 100644
--- a/crypto/ecc_curve_defs.h
+++ b/crypto/ecc_curve_defs.h
@@ -17,6 +17,7 @@ static u64 nist_p192_b[] = { 0xFEB8DEECC146B9B1ull, 0x0FA7E9AB72243049ull,
 				0x64210519E59C80E7ull };
 static struct ecc_curve nist_p192 = {
 	.name = "nist_192",
+	.nbits = 192,
 	.g = {
 		.x = nist_p192_g_x,
 		.y = nist_p192_g_y,
@@ -43,6 +44,7 @@ static u64 nist_p256_b[] = { 0x3BCE3C3E27D2604Bull, 0x651D06B0CC53B0F6ull,
 				0xB3EBBD55769886BCull, 0x5AC635D8AA3A93E7ull };
 static struct ecc_curve nist_p256 = {
 	.name = "nist_256",
+	.nbits = 256,
 	.g = {
 		.x = nist_p256_g_x,
 		.y = nist_p256_g_y,
@@ -75,6 +77,7 @@ static u64 nist_p384_b[] = { 0x2a85c8edd3ec2aefull, 0xc656398d8a2ed19dull,
 				0x988e056be3f82d19ull, 0xb3312fa7e23ee7e4ull };
 static struct ecc_curve nist_p384 = {
 	.name = "nist_384",
+	.nbits = 384,
 	.g = {
 		.x = nist_p384_g_x,
 		.y = nist_p384_g_y,
@@ -95,6 +98,7 @@ static u64 curve25519_a[] = { 0x000000000001DB41, 0x0000000000000000,
 				0x0000000000000000, 0x0000000000000000 };
 static const struct ecc_curve ecc_25519 = {
 	.name = "curve25519",
+	.nbits = 255,
 	.g = {
 		.x = curve25519_g_x,
 		.ndigits = 4,
diff --git a/crypto/ecrdsa_defs.h b/crypto/ecrdsa_defs.h
index 0056335b9d03..1c2c2449e331 100644
--- a/crypto/ecrdsa_defs.h
+++ b/crypto/ecrdsa_defs.h
@@ -47,6 +47,7 @@ static u64 cp256a_b[] = {
 
 static struct ecc_curve gost_cp256a = {
 	.name = "cp256a",
+	.nbits = 256,
 	.g = {
 		.x = cp256a_g_x,
 		.y = cp256a_g_y,
@@ -80,6 +81,7 @@ static u64 cp256b_b[] = {
 
 static struct ecc_curve gost_cp256b = {
 	.name = "cp256b",
+	.nbits = 256,
 	.g = {
 		.x = cp256b_g_x,
 		.y = cp256b_g_y,
@@ -117,6 +119,7 @@ static u64 cp256c_b[] = {
 
 static struct ecc_curve gost_cp256c = {
 	.name = "cp256c",
+	.nbits = 256,
 	.g = {
 		.x = cp256c_g_x,
 		.y = cp256c_g_y,
@@ -166,6 +169,7 @@ static u64 tc512a_b[] = {
 
 static struct ecc_curve gost_tc512a = {
 	.name = "tc512a",
+	.nbits = 512,
 	.g = {
 		.x = tc512a_g_x,
 		.y = tc512a_g_y,
@@ -211,6 +215,7 @@ static u64 tc512b_b[] = {
 
 static struct ecc_curve gost_tc512b = {
 	.name = "tc512b",
+	.nbits = 512,
 	.g = {
 		.x = tc512b_g_x,
 		.y = tc512b_g_y,
diff --git a/include/crypto/ecc_curve.h b/include/crypto/ecc_curve.h
index 70964781eb68..63d5754e7614 100644
--- a/include/crypto/ecc_curve.h
+++ b/include/crypto/ecc_curve.h
@@ -23,6 +23,7 @@ struct ecc_point {
  * struct ecc_curve - definition of elliptic curve
  *
  * @name:	Short name of the curve.
+ * @nbits:	The number of bits of a curve.
  * @g:		Generator point of the curve.
  * @p:		Prime number, if Barrett's reduction is used for this curve
  *		pre-calculated value 'mu' is appended to the @p after ndigits.
@@ -34,6 +35,7 @@ struct ecc_point {
  */
 struct ecc_curve {
 	char *name;
+	unsigned int nbits;
 	struct ecc_point g;
 	u64 *p;
 	u64 *n;
-- 
2.43.0

[PATCH v6 06/13] crypto: ecc - Implement vli_mmod_fast_521 for NIST p521

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Implement vli_mmod_fast_521 following the description for how to calculate
the modulus for NIST P521 in the NIST publication "Recommendations for
Discrete Logarithm-Based Cryptography: Elliptic Curve Domain Parameters"
section G.1.4.

NIST p521 requires 9 64bit digits, so increase the ECC_MAX_DIGITS so that
arrays fit the larger numbers.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 crypto/ecc.c                  | 25 +++++++++++++++++++++++++
 include/crypto/internal/ecc.h |  3 ++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/crypto/ecc.c b/crypto/ecc.c
index 415a2f4e7291..99d41887c005 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -902,6 +902,28 @@ static void vli_mmod_fast_384(u64 *result, const u64 *product,
 #undef AND64H
 #undef AND64L
 
+/*
+ * Computes result = product % curve_prime
+ * from "Recommendations for Discrete Logarithm-Based Cryptography:
+ *       Elliptic Curve Domain Parameters" section G.1.4
+ */
+static void vli_mmod_fast_521(u64 *result, const u64 *product,
+			      const u64 *curve_prime, u64 *tmp)
+{
+	const unsigned int ndigits = ECC_CURVE_NIST_P521_DIGITS;
+	size_t i;
+
+	/* Initialize result with lowest 521 bits from product */
+	vli_set(result, product, ndigits);
+	result[8] &= 0x1ff;
+
+	for (i = 0; i < ndigits; i++)
+		tmp[i] = (product[8 + i] >> 9) | (product[9 + i] << 55);
+	tmp[8] &= 0x1ff;
+
+	vli_mod_add(result, result, tmp, curve_prime, ndigits);
+}
+
 /* Computes result = product % curve_prime for different curve_primes.
  *
  * Note that curve_primes are distinguished just by heuristic check and
@@ -941,6 +963,9 @@ static bool vli_mmod_fast(u64 *result, u64 *product,
 	case ECC_CURVE_NIST_P384_DIGITS:
 		vli_mmod_fast_384(result, product, curve_prime, tmp);
 		break;
+	case ECC_CURVE_NIST_P521_DIGITS:
+		vli_mmod_fast_521(result, product, curve_prime, tmp);
+		break;
 	default:
 		pr_err_ratelimited("ecc: unsupported digits size!\n");
 		return false;
diff --git a/include/crypto/internal/ecc.h b/include/crypto/internal/ecc.h
index ab722a8986b7..4e2f5f938e91 100644
--- a/include/crypto/internal/ecc.h
+++ b/include/crypto/internal/ecc.h
@@ -33,7 +33,8 @@
 #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              (512 / 64) /* due to ecrdsa */
+#define ECC_CURVE_NIST_P521_DIGITS  9
+#define ECC_MAX_DIGITS              DIV_ROUND_UP(521, 64) /* NIST P521 */
 
 #define ECC_DIGITS_TO_BYTES_SHIFT 3
 
-- 
2.43.0

[PATCH v6 07/13] crypto: ecc - Add special case for NIST P521 in ecc_point_mult

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

In ecc_point_mult use the number of bits of the NIST P521 curve + 2. The
change is required specifically for NIST P521 to pass mathematical tests
on the public key.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/ecc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/crypto/ecc.c b/crypto/ecc.c
index 99d41887c005..ead40b5ebb46 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -1320,7 +1320,10 @@ static void ecc_point_mult(struct ecc_point *result,
 	carry = vli_add(sk[0], scalar, curve->n, ndigits);
 	vli_add(sk[1], sk[0], curve->n, ndigits);
 	scalar = sk[!carry];
-	num_bits = sizeof(u64) * ndigits * 8 + 1;
+	if (curve->nbits == 521)	/* NIST P521 */
+		num_bits = curve->nbits + 2;
+	else
+		num_bits = sizeof(u64) * ndigits * 8 + 1;
 
 	vli_set(rx[1], point->x, ndigits);
 	vli_set(ry[1], point->y, ndigits);
-- 
2.43.0

[PATCH v6 08/13] crypto: ecc - Add NIST P521 curve parameters

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Add the parameters for the NIST P521 curve and define a new curve ID
for it. Make the curve available in ecc_get_curve.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/ecc.c            |  2 ++
 crypto/ecc_curve_defs.h | 45 +++++++++++++++++++++++++++++++++++++++++
 include/crypto/ecdh.h   |  1 +
 3 files changed, 48 insertions(+)

diff --git a/crypto/ecc.c b/crypto/ecc.c
index ead40b5ebb46..4f6fa8617308 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -60,6 +60,8 @@ const struct ecc_curve *ecc_get_curve(unsigned int curve_id)
 		return &nist_p256;
 	case ECC_CURVE_NIST_P384:
 		return &nist_p384;
+	case ECC_CURVE_NIST_P521:
+		return &nist_p521;
 	default:
 		return NULL;
 	}
diff --git a/crypto/ecc_curve_defs.h b/crypto/ecc_curve_defs.h
index ab1ef3d94be5..0ecade7d02f5 100644
--- a/crypto/ecc_curve_defs.h
+++ b/crypto/ecc_curve_defs.h
@@ -89,6 +89,51 @@ static struct ecc_curve nist_p384 = {
 	.b = nist_p384_b
 };
 
+/* NIST P-521 */
+static u64 nist_p521_g_x[] = { 0xf97e7e31c2e5bd66ull, 0x3348b3c1856a429bull,
+				0xfe1dc127a2ffa8deull, 0xa14b5e77efe75928ull,
+				0xf828af606b4d3dbaull, 0x9c648139053fb521ull,
+				0x9e3ecb662395b442ull, 0x858e06b70404e9cdull,
+				0xc6ull };
+static u64 nist_p521_g_y[] = { 0x88be94769fd16650ull, 0x353c7086a272c240ull,
+				0xc550b9013fad0761ull, 0x97ee72995ef42640ull,
+				0x17afbd17273e662cull, 0x98f54449579b4468ull,
+				0x5c8a5fb42c7d1bd9ull, 0x39296a789a3bc004ull,
+				0x118ull };
+static u64 nist_p521_p[] = { 0xffffffffffffffffull, 0xffffffffffffffffull,
+				0xffffffffffffffffull, 0xffffffffffffffffull,
+				0xffffffffffffffffull, 0xffffffffffffffffull,
+				0xffffffffffffffffull, 0xffffffffffffffffull,
+				0x1ffull };
+static u64 nist_p521_n[] = { 0xbb6fb71e91386409ull, 0x3bb5c9b8899c47aeull,
+				0x7fcc0148f709a5d0ull, 0x51868783bf2f966bull,
+				0xfffffffffffffffaull, 0xffffffffffffffffull,
+				0xffffffffffffffffull, 0xffffffffffffffffull,
+				0x1ffull };
+static u64 nist_p521_a[] = { 0xfffffffffffffffcull, 0xffffffffffffffffull,
+				0xffffffffffffffffull, 0xffffffffffffffffull,
+				0xffffffffffffffffull, 0xffffffffffffffffull,
+				0xffffffffffffffffull, 0xffffffffffffffffull,
+				0x1ffull };
+static u64 nist_p521_b[] = { 0xef451fd46b503f00ull, 0x3573df883d2c34f1ull,
+				0x1652c0bd3bb1bf07ull, 0x56193951ec7e937bull,
+				0xb8b489918ef109e1ull, 0xa2da725b99b315f3ull,
+				0x929a21a0b68540eeull, 0x953eb9618e1c9a1full,
+				0x051ull };
+static struct ecc_curve nist_p521 = {
+	.name = "nist_521",
+	.nbits = 521,
+	.g = {
+		.x = nist_p521_g_x,
+		.y = nist_p521_g_y,
+		.ndigits = 9,
+	},
+	.p = nist_p521_p,
+	.n = nist_p521_n,
+	.a = nist_p521_a,
+	.b = nist_p521_b
+};
+
 /* curve25519 */
 static u64 curve25519_g_x[] = { 0x0000000000000009, 0x0000000000000000,
 				0x0000000000000000, 0x0000000000000000 };
diff --git a/include/crypto/ecdh.h b/include/crypto/ecdh.h
index a9f98078d29c..9784ecdd2fb4 100644
--- a/include/crypto/ecdh.h
+++ b/include/crypto/ecdh.h
@@ -26,6 +26,7 @@
 #define ECC_CURVE_NIST_P192	0x0001
 #define ECC_CURVE_NIST_P256	0x0002
 #define ECC_CURVE_NIST_P384	0x0003
+#define ECC_CURVE_NIST_P521	0x0004
 
 /**
  * struct ecdh - define an ECDH private key
-- 
2.43.0

[PATCH v6 09/13] crypto: ecdsa - Replace ndigits with nbits where precision is needed

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Replace the usage of ndigits with nbits where more precise space
calculations are needed, such as in ecdsa_max_size where the length of a
coordinate is determined.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/ecdsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
index 1814f009f971..4daefb40c37a 100644
--- a/crypto/ecdsa.c
+++ b/crypto/ecdsa.c
@@ -266,7 +266,7 @@ static unsigned int ecdsa_max_size(struct crypto_akcipher *tfm)
 {
 	struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm);
 
-	return ctx->pub_key.ndigits << ECC_DIGITS_TO_BYTES_SHIFT;
+	return DIV_ROUND_UP(ctx->curve->nbits, 8);
 }
 
 static int ecdsa_nist_p384_init_tfm(struct crypto_akcipher *tfm)
-- 
2.43.0

[PATCH v6 10/13] crypto: ecdsa - Rename keylen to bufsize where necessary

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

In some cases the name keylen does not reflect the purpose of the variable
anymore once NIST P521 is used but it is the size of the buffer. There-
for, rename keylen to bufsize where appropriate.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/ecdsa.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
index 4daefb40c37a..4e847b59622a 100644
--- a/crypto/ecdsa.c
+++ b/crypto/ecdsa.c
@@ -35,8 +35,8 @@ struct ecdsa_signature_ctx {
 static int ecdsa_get_signature_rs(u64 *dest, size_t hdrlen, unsigned char tag,
 				  const void *value, size_t vlen, unsigned int ndigits)
 {
-	size_t keylen = ndigits * sizeof(u64);
-	ssize_t diff = vlen - keylen;
+	size_t bufsize = ndigits * sizeof(u64);
+	ssize_t diff = vlen - bufsize;
 	const char *d = value;
 	u8 rs[ECC_MAX_BYTES];
 
@@ -58,7 +58,7 @@ static int ecdsa_get_signature_rs(u64 *dest, size_t hdrlen, unsigned char tag,
 		if (diff)
 			return -EINVAL;
 	}
-	if (-diff >= keylen)
+	if (-diff >= bufsize)
 		return -EINVAL;
 
 	if (diff) {
@@ -138,7 +138,7 @@ static int ecdsa_verify(struct akcipher_request *req)
 {
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm);
-	size_t keylen = ctx->curve->g.ndigits * sizeof(u64);
+	size_t bufsize = ctx->curve->g.ndigits * sizeof(u64);
 	struct ecdsa_signature_ctx sig_ctx = {
 		.curve = ctx->curve,
 	};
@@ -165,14 +165,14 @@ static int ecdsa_verify(struct akcipher_request *req)
 		goto error;
 
 	/* if the hash is shorter then we will add leading zeros to fit to ndigits */
-	diff = keylen - req->dst_len;
+	diff = bufsize - req->dst_len;
 	if (diff >= 0) {
 		if (diff)
 			memset(rawhash, 0, diff);
 		memcpy(&rawhash[diff], buffer + req->src_len, req->dst_len);
 	} else if (diff < 0) {
 		/* given hash is longer, we take the left-most bytes */
-		memcpy(&rawhash, buffer + req->src_len, keylen);
+		memcpy(&rawhash, buffer + req->src_len, bufsize);
 	}
 
 	ecc_swap_digits((u64 *)rawhash, hash, ctx->curve->g.ndigits);
-- 
2.43.0

[PATCH v6 11/13] crypto: ecdsa - Register NIST P521 and extend test suite

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Register NIST P521 as an akcipher and extend the testmgr with
NIST P521-specific test vectors.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/ecdsa.c   |  30 ++++++++++
 crypto/testmgr.c |   7 +++
 crypto/testmgr.h | 146 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 183 insertions(+)

diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
index 4e847b59622a..894599f1885f 100644
--- a/crypto/ecdsa.c
+++ b/crypto/ecdsa.c
@@ -269,6 +269,28 @@ static unsigned int ecdsa_max_size(struct crypto_akcipher *tfm)
 	return DIV_ROUND_UP(ctx->curve->nbits, 8);
 }
 
+static int ecdsa_nist_p521_init_tfm(struct crypto_akcipher *tfm)
+{
+	struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm);
+
+	return ecdsa_ecc_ctx_init(ctx, ECC_CURVE_NIST_P521);
+}
+
+static struct akcipher_alg ecdsa_nist_p521 = {
+	.verify = ecdsa_verify,
+	.set_pub_key = ecdsa_set_pub_key,
+	.max_size = ecdsa_max_size,
+	.init = ecdsa_nist_p521_init_tfm,
+	.exit = ecdsa_exit_tfm,
+	.base = {
+		.cra_name = "ecdsa-nist-p521",
+		.cra_driver_name = "ecdsa-nist-p521-generic",
+		.cra_priority = 100,
+		.cra_module = THIS_MODULE,
+		.cra_ctxsize = sizeof(struct ecc_ctx),
+	},
+};
+
 static int ecdsa_nist_p384_init_tfm(struct crypto_akcipher *tfm)
 {
 	struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm);
@@ -352,8 +374,15 @@ static int __init ecdsa_init(void)
 	if (ret)
 		goto nist_p384_error;
 
+	ret = crypto_register_akcipher(&ecdsa_nist_p521);
+	if (ret)
+		goto nist_p521_error;
+
 	return 0;
 
+nist_p521_error:
+	crypto_unregister_akcipher(&ecdsa_nist_p384);
+
 nist_p384_error:
 	crypto_unregister_akcipher(&ecdsa_nist_p256);
 
@@ -369,6 +398,7 @@ static void __exit ecdsa_exit(void)
 		crypto_unregister_akcipher(&ecdsa_nist_p192);
 	crypto_unregister_akcipher(&ecdsa_nist_p256);
 	crypto_unregister_akcipher(&ecdsa_nist_p384);
+	crypto_unregister_akcipher(&ecdsa_nist_p521);
 }
 
 subsys_initcall(ecdsa_init);
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index c26aeda85787..a017b4ad119b 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -5097,6 +5097,13 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.akcipher = __VECS(ecdsa_nist_p384_tv_template)
 		}
+	}, {
+		.alg = "ecdsa-nist-p521",
+		.test = alg_test_akcipher,
+		.fips_allowed = 1,
+		.suite = {
+			.akcipher = __VECS(ecdsa_nist_p521_tv_template)
+		}
 	}, {
 		.alg = "ecrdsa",
 		.test = alg_test_akcipher,
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 986f331a5fc2..9bde04be8df9 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -991,6 +991,152 @@ static const struct akcipher_testvec ecdsa_nist_p384_tv_template[] = {
 	},
 };
 
+static const struct akcipher_testvec ecdsa_nist_p521_tv_template[] = {
+	{
+	.key = /* secp521r1(sha224) */
+	"\x04\x01\x4f\x43\x18\xb6\xa9\xc9\x5d\x68\xd3\xa9\x42\xf8\x98\xc0"
+	"\xd2\xd1\xa9\x50\x3b\xe8\xc4\x40\xe6\x11\x78\x88\x4b\xbd\x76\xa7"
+	"\x9a\xe0\xdd\x31\xa4\x67\x78\x45\x33\x9e\x8c\xd1\xc7\x44\xac\x61"
+	"\x68\xc8\x04\xe7\x5c\x79\xb1\xf1\x41\x0c\x71\xc0\x53\xa8\xbc\xfb"
+	"\xf5\xca\xd4\x01\x40\xfd\xa3\x45\xda\x08\xe0\xb4\xcb\x28\x3b\x0a"
+	"\x02\x35\x5f\x02\x9f\x3f\xcd\xef\x08\x22\x40\x97\x74\x65\xb7\x76"
+	"\x85\xc7\xc0\x5c\xfb\x81\xe1\xa5\xde\x0c\x4e\x8b\x12\x31\xb6\x47"
+	"\xed\x37\x0f\x99\x3f\x26\xba\xa3\x8e\xff\x79\x34\x7c\x3a\xfe\x1f"
+	"\x3b\x83\x82\x2f\x14",
+	.key_len = 133,
+	.params =
+	"\x30\x10\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x05\x2b\x81\x04"
+	"\x00\x23",
+	.param_len = 18,
+	.m =
+	"\xa2\x3a\x6a\x8c\x7b\x3c\xf2\x51\xf8\xbe\x5f\x4f\x3b\x15\x05\xc4"
+	"\xb5\xbc\x19\xe7\x21\x85\xe9\x23\x06\x33\x62\xfb",
+	.m_size = 28,
+	.algo = OID_id_ecdsa_with_sha224,
+	.c =
+	"\x30\x81\x86\x02\x41\x01\xd6\x43\xe7\xff\x42\xb2\xba\x74\x35\xf6"
+	"\xdc\x6d\x02\x7b\x22\xac\xe2\xef\x07\x92\xee\x60\x94\x06\xf8\x3f"
+	"\x59\x0f\x74\xf0\x3f\xd8\x18\xc6\x37\x8a\xcb\xa7\xd8\x7d\x98\x85"
+	"\x29\x88\xff\x0b\x94\x94\x6c\xa6\x9b\x89\x8b\x1e\xfd\x09\x46\x6b"
+	"\xc7\xaf\x7a\xb9\x19\x0a\x02\x41\x3a\x26\x0d\x55\xcd\x23\x1e\x7d"
+	"\xa0\x5e\xf9\x88\xf3\xd2\x32\x90\x57\x0f\xf8\x65\x97\x6b\x09\x4d"
+	"\x22\x26\x0b\x5f\x49\x32\x6b\x91\x99\x30\x90\x0f\x1c\x8f\x78\xd3"
+	"\x9f\x0e\x64\xcc\xc4\xe8\x43\xd9\x0e\x1c\xad\x22\xda\x82\x00\x35"
+	"\xa3\x50\xb1\xa5\x98\x92\x2a\xa5\x52",
+	.c_size = 137,
+	.public_key_vec = true,
+	.siggen_sigver_test = true,
+	},
+	{
+	.key = /* secp521r1(sha256) */
+	"\x04\x01\x05\x3a\x6b\x3b\x5a\x0f\xa7\xb9\xb7\x32\x53\x4e\xe2\xae"
+	"\x0a\x52\xc5\xda\xdd\x5a\x79\x1c\x30\x2d\x33\x07\x79\xd5\x70\x14"
+	"\x61\x0c\xec\x26\x4d\xd8\x35\x57\x04\x1d\x88\x33\x4d\xce\x05\x36"
+	"\xa5\xaf\x56\x84\xfa\x0b\x9e\xff\x7b\x30\x4b\x92\x1d\x06\xf8\x81"
+	"\x24\x1e\x51\x00\x09\x21\x51\xf7\x46\x0a\x77\xdb\xb5\x0c\xe7\x9c"
+	"\xff\x27\x3c\x02\x71\xd7\x85\x36\xf1\xaa\x11\x59\xd8\xb8\xdc\x09"
+	"\xdc\x6d\x5a\x6f\x63\x07\x6c\xe1\xe5\x4d\x6e\x0f\x6e\xfb\x7c\x05"
+	"\x8a\xe9\x53\xa8\xcf\xce\x43\x0e\x82\x20\x86\xbc\x88\x9c\xb7\xe3"
+	"\xe6\x77\x1e\x1f\x8a",
+	.key_len = 133,
+	.params =
+	"\x30\x10\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x05\x2b\x81\x04"
+	"\x00\x23",
+	.param_len = 18,
+	.m =
+	"\xcc\x97\x73\x0c\x73\xa2\x53\x2b\xfa\xd7\x83\x1d\x0c\x72\x1b\x39"
+	"\x80\x71\x8d\xdd\xc5\x9b\xff\x55\x32\x98\x25\xa2\x58\x2e\xb7\x73",
+	.m_size = 32,
+	.algo = OID_id_ecdsa_with_sha256,
+	.c =
+	"\x30\x81\x88\x02\x42\x00\xcd\xa5\x5f\x57\x52\x27\x78\x3a\xb5\x06"
+	"\x0f\xfd\x83\xfc\x0e\xd9\xce\x50\x9f\x7d\x1f\xca\x8b\xa8\x2d\x56"
+	"\x3c\xf6\xf0\xd8\xe1\xb7\x5d\x95\x35\x6f\x02\x0e\xaf\xe1\x4c\xae"
+	"\xce\x54\x76\x9a\xc2\x8f\xb8\x38\x1f\x46\x0b\x04\x64\x34\x79\xde"
+	"\x7e\xd7\x59\x10\xe9\xd9\xd5\x02\x42\x01\xcf\x50\x85\x38\xf9\x15"
+	"\x83\x18\x04\x6b\x35\xae\x65\xb5\x99\x12\x0a\xa9\x79\x24\xb9\x37"
+	"\x35\xdd\xa0\xe0\x87\x2c\x44\x4b\x5a\xee\xaf\xfa\x10\xdd\x9b\xfb"
+	"\x36\x1a\x31\x03\x42\x02\x5f\x50\xf0\xa2\x0d\x1c\x57\x56\x8f\x12"
+	"\xb7\x1d\x91\x55\x38\xb6\xf6\x34\x65\xc7\xbd",
+	.c_size = 139,
+	.public_key_vec = true,
+	.siggen_sigver_test = true,
+	},
+	{
+	.key = /* secp521r1(sha384) */
+	"\x04\x00\x2e\xd6\x21\x04\x75\xc3\xdc\x7d\xff\x0e\xf3\x70\x25\x2b"
+	"\xad\x72\xfc\x5a\x91\xf1\xd5\x9c\x64\xf3\x1f\x47\x11\x10\x62\x33"
+	"\xfd\x2e\xe8\x32\xca\x9e\x6f\x0a\x4c\x5b\x35\x9a\x46\xc5\xe7\xd4"
+	"\x38\xda\xb2\xf0\xf4\x87\xf3\x86\xf4\xea\x70\xad\x1e\xd4\x78\x8c"
+	"\x36\x18\x17\x00\xa2\xa0\x34\x1b\x2e\x6a\xdf\x06\xd6\x99\x2d\x47"
+	"\x50\x92\x1a\x8a\x72\x9c\x23\x44\xfa\xa7\xa9\xed\xa6\xef\x26\x14"
+	"\xb3\x9d\xfe\x5e\xa3\x8c\xd8\x29\xf8\xdf\xad\xa6\xab\xfc\xdd\x46"
+	"\x22\x6e\xd7\x35\xc7\x23\xb7\x13\xae\xb6\x34\xff\xd7\x80\xe5\x39"
+	"\xb3\x3b\x5b\x1b\x94",
+	.key_len = 133,
+	.params =
+	"\x30\x10\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x05\x2b\x81\x04"
+	"\x00\x23",
+	.param_len = 18,
+	.m =
+	"\x36\x98\xd6\x82\xfa\xad\xed\x3c\xb9\x40\xb6\x4d\x9e\xb7\x04\x26"
+	"\xad\x72\x34\x44\xd2\x81\xb4\x9b\xbe\x01\x04\x7a\xd8\x50\xf8\x59"
+	"\xba\xad\x23\x85\x6b\x59\xbe\xfb\xf6\x86\xd4\x67\xa8\x43\x28\x76",
+	.m_size = 48,
+	.algo = OID_id_ecdsa_with_sha384,
+	.c =
+	"\x30\x81\x88\x02\x42\x00\x93\x96\x76\x3c\x27\xea\xaa\x9c\x26\xec"
+	"\x51\xdc\xe8\x35\x5e\xae\x16\xf2\x4b\x64\x98\xf7\xec\xda\xc7\x7e"
+	"\x42\x71\x86\x57\x2d\xf1\x7d\xe4\xdf\x9b\x7d\x9e\x47\xca\x33\x32"
+	"\x76\x06\xd0\xf9\xc0\xe4\xe6\x84\x59\xfd\x1a\xc4\x40\xdd\x43\xb8"
+	"\x6a\xdd\xfb\xe6\x63\x4e\x28\x02\x42\x00\xff\xc3\x6a\x87\x6e\xb5"
+	"\x13\x1f\x20\x55\xce\x37\x97\xc9\x05\x51\xe5\xe4\x3c\xbc\x93\x65"
+	"\x57\x1c\x30\xda\xa7\xcd\x26\x28\x76\x3b\x52\xdf\xc4\xc0\xdb\x54"
+	"\xdb\x8a\x0d\x6a\xc3\xf3\x7a\xd1\xfa\xe7\xa7\xe5\x5a\x94\x56\xcf"
+	"\x8f\xb4\x22\xc6\x4f\xab\x2b\x62\xc1\x42\xb1",
+	.c_size = 139,
+	.public_key_vec = true,
+	.siggen_sigver_test = true,
+	},
+	{
+	.key = /* secp521r1(sha512) */
+	"\x04\x00\xc7\x65\xee\x0b\x86\x7d\x8f\x02\xf1\x74\x5b\xb0\x4c\x3f"
+	"\xa6\x35\x60\x9f\x55\x23\x11\xcc\xdf\xb8\x42\x99\xee\x6c\x96\x6a"
+	"\x27\xa2\x56\xb2\x2b\x03\xad\x0f\xe7\x97\xde\x09\x5d\xb4\xc5\x5f"
+	"\xbd\x87\x37\xbf\x5a\x16\x35\x56\x08\xfd\x6f\x06\x1a\x1c\x84\xee"
+	"\xc3\x64\xb3\x00\x9e\xbd\x6e\x60\x76\xee\x69\xfd\x3a\xb8\xcd\x7e"
+	"\x91\x68\x53\x57\x44\x13\x2e\x77\x09\x2a\xbe\x48\xbd\x91\xd8\xf6"
+	"\x21\x16\x53\x99\xd5\xf0\x40\xad\xa6\xf8\x58\x26\xb6\x9a\xf8\x77"
+	"\xfe\x3a\x05\x1a\xdb\xa9\x0f\xc0\x6c\x76\x30\x8c\xd8\xde\x44\xae"
+	"\xd0\x17\xdf\x49\x6a",
+	.key_len = 133,
+	.params =
+	"\x30\x10\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x05\x2b\x81\x04"
+	"\x00\x23",
+	.param_len = 18,
+	.m =
+	"\x5c\xa6\xbc\x79\xb8\xa0\x1e\x11\x83\xf7\xe9\x05\xdf\xba\xf7\x69"
+	"\x97\x22\x32\xe4\x94\x7c\x65\xbd\x74\xc6\x9a\x8b\xbd\x0d\xdc\xed"
+	"\xf5\x9c\xeb\xe1\xc5\x68\x40\xf2\xc7\x04\xde\x9e\x0d\x76\xc5\xa3"
+	"\xf9\x3c\x6c\x98\x08\x31\xbd\x39\xe8\x42\x7f\x80\x39\x6f\xfe\x68",
+	.m_size = 64,
+	.algo = OID_id_ecdsa_with_sha512,
+	.c =
+	"\x30\x81\x88\x02\x42\x01\x5c\x71\x86\x96\xac\x21\x33\x7e\x4e\xaa"
+	"\x86\xec\xa8\x05\x03\x52\x56\x63\x0e\x02\xcc\x94\xa9\x05\xb9\xfb"
+	"\x62\x1e\x42\x03\x6c\x74\x8a\x1f\x12\x3e\xb7\x7e\x51\xff\x7f\x27"
+	"\x93\xe8\x6c\x49\x7d\x28\xfc\x80\xa6\x13\xfc\xb6\x90\xf7\xbb\x28"
+	"\xb5\x04\xb0\xb6\x33\x1c\x7e\x02\x42\x01\x70\x43\x52\x1d\xe3\xc6"
+	"\xbd\x5a\x40\x95\x35\x89\x4f\x41\x5f\x9e\x19\x88\x05\x3e\x43\x39"
+	"\x01\xbd\xb7\x7a\x76\x37\x51\x47\x49\x98\x12\x71\xd0\xe9\xca\xa7"
+	"\xc0\xcb\xaa\x00\x55\xbb\x6a\xb4\x73\x00\xd2\x72\x74\x13\x63\x39"
+	"\xa6\xe5\x25\x46\x1e\x77\x44\x78\xe0\xd1\x04",
+	.c_size = 139,
+	.public_key_vec = true,
+	.siggen_sigver_test = true,
+	},
+};
+
 /*
  * EC-RDSA test vectors are generated by gost-engine.
  */
-- 
2.43.0

[PATCH v6 12/13] crypto: asymmetric_keys - Adjust signature size calculation for NIST P521

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Adjust the calculation of the maximum signature size for support of
NIST P521. While existing curves may prepend a 0 byte to their coordinates
(to make the number positive), NIST P521 will not do this since only the
first bit in the most significant byte is used.

If the encoding of the x & y coordinates requires at least 128 bytes then
an additional byte is needed for the encoding of the length. Take this into
account when calculating the maximum signature size.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/asymmetric_keys/public_key.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index e5f22691febd..16cc0be28929 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -233,6 +233,7 @@ static int software_key_query(const struct kernel_pkey_params *params,
 	info->key_size = len * 8;
 
 	if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
+		int slen = len;
 		/*
 		 * ECDSA key sizes are much smaller than RSA, and thus could
 		 * operate on (hashed) inputs that are larger than key size.
@@ -246,8 +247,19 @@ static int software_key_query(const struct kernel_pkey_params *params,
 		 * Verify takes ECDSA-Sig (described in RFC 5480) as input,
 		 * which is actually 2 'key_size'-bit integers encoded in
 		 * ASN.1.  Account for the ASN.1 encoding overhead here.
+		 *
+		 * NIST P192/256/384 may prepend a '0' to a coordinate to
+		 * indicate a positive integer. NIST P521 never needs it.
 		 */
-		info->max_sig_size = 2 * (len + 3) + 2;
+		if (strcmp(pkey->pkey_algo, "ecdsa-nist-p521") != 0)
+			slen += 1;
+		/* Length of encoding the x & y coordinates */
+		slen = 2 * (slen + 2);
+		/*
+		 * If coordinate encoding takes at least 128 bytes then an
+		 * additional byte for length encoding is needed.
+		 */
+		info->max_sig_size = 1 + (slen >= 128) + 1 + slen;
 	} else {
 		info->max_data_size = len;
 		info->max_sig_size = len;
-- 
2.43.0

[PATCH v6 13/13] crypto: x509 - Add OID for NIST P521 and extend parser for it

[Stefan Berger]

From: Stefan Berger <stefanb@linux.ibm.com>

Enable the x509 parser to accept NIST P521 certificates and add the
OID for ansip521r1, which is the identifier for NIST P521.

Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/asymmetric_keys/x509_cert_parser.c | 3 +++
 include/linux/oid_registry.h              | 1 +
 2 files changed, 4 insertions(+)

diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 487204d39426..99f809b7910b 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -538,6 +538,9 @@ int x509_extract_key_data(void *context, size_t hdrlen,
 		case OID_id_ansip384r1:
 			ctx->cert->pub->pkey_algo = "ecdsa-nist-p384";
 			break;
+		case OID_id_ansip521r1:
+			ctx->cert->pub->pkey_algo = "ecdsa-nist-p521";
+			break;
 		default:
 			return -ENOPKG;
 		}
diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
index 3921fbed0b28..af16d96fbbf2 100644
--- a/include/linux/oid_registry.h
+++ b/include/linux/oid_registry.h
@@ -65,6 +65,7 @@ enum OID {
 	OID_Scram,			/* 1.3.6.1.5.5.14 */
 	OID_certAuthInfoAccess,		/* 1.3.6.1.5.5.7.1.1 */
 	OID_id_ansip384r1,		/* 1.3.132.0.34 */
+	OID_id_ansip521r1,		/* 1.3.132.0.35 */
 	OID_sha256,			/* 2.16.840.1.101.3.4.2.1 */
 	OID_sha384,			/* 2.16.840.1.101.3.4.2.2 */
 	OID_sha512,			/* 2.16.840.1.101.3.4.2.3 */
-- 
2.43.0

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Stefan Berger]

On 3/12/24 14:36, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> This series adds support for the NIST P521 curve to the ecdsa module
> to enable signature verification with it.
> 
> An issue with the current code in ecdsa 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 adjusting tests on input byte
> array lengths to tolerate arrays not providing multiples of 8 bytes.
> 

FYI: A test suite for the NIST ECC algorithms is here: 
https://github.com/stefanberger/eckey-testing

Script to test valid and invalid certs and signatures: 
test-ecc-kernel-keys.sh

> Regards,
>     Stefan
> 
> v6:
>   - Use existing #defines for number of digits rather than plain numbers
>     (1/13, 6/13) following Bharat's suggestion
>   - Initialize result from lowest 521 bits of product rather than going
>     through tmp variable (6/13)
> 
> v5:
>   - Simplified ecc_digits_from_bytes as suggested by Lukas (1/12)
>   - Using nbits == 521 to detect NIST P521 curve rather than strcmp()
>     (5,6/12)
>   - Nits in patch description and comments (11/12)
> 
> v4:
>   - Followed suggestions by Lukas Wummer (1,5,8/12)
>   - Use nbits rather than ndigits where needed (8/12)
>   - Renaming 'keylen' variablest to bufsize where necessary (9/12)
>   - Adjust signature size calculation for NIST P521 (11/12)
> 
> v3:
>   - Dropped ecdh support
>   - Use ecc_get_curve_nbits for getting number of bits in NIST P521 curve
>     in ecc_point_mult (7/10)
> 
> v2:
>   - Reformulated some patch descriptions
>   - Fixed issue detected by krobot
>   - Some other small changes to the code
> 
> 
> 
> Stefan Berger (13):
>    crypto: ecc - Use ECC_CURVE_NIST_P192/256/384_DIGITS where possible
>    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 - Add nbits field to ecc_curve structure
>    crypto: ecc - Implement vli_mmod_fast_521 for NIST p521
>    crypto: ecc - Add special case for NIST P521 in ecc_point_mult
>    crypto: ecc - Add NIST P521 curve parameters
>    crypto: ecdsa - Replace ndigits with nbits where precision is needed
>    crypto: ecdsa - Rename keylen to bufsize where necessary
>    crypto: ecdsa - Register NIST P521 and extend test suite
>    crypto: asymmetric_keys - Adjust signature size calculation for NIST
>      P521
>    crypto: x509 - Add OID for NIST P521 and extend parser for it
> 
>   crypto/asymmetric_keys/public_key.c       |  14 ++-
>   crypto/asymmetric_keys/x509_cert_parser.c |   3 +
>   crypto/ecc.c                              |  44 +++++--
>   crypto/ecc_curve_defs.h                   |  49 ++++++++
>   crypto/ecdsa.c                            |  62 ++++++---
>   crypto/ecrdsa_defs.h                      |   5 +
>   crypto/testmgr.c                          |   7 ++
>   crypto/testmgr.h                          | 146 ++++++++++++++++++++++
>   include/crypto/ecc_curve.h                |   2 +
>   include/crypto/ecdh.h                     |   1 +
>   include/crypto/internal/ecc.h             |  24 +++-
>   include/linux/oid_registry.h              |   1 +
>   12 files changed, 335 insertions(+), 23 deletions(-)
> 

RE: [EXTERNAL] [PATCH v6 06/13] crypto: ecc - Implement vli_mmod_fast_521 for NIST p521

[Bharat Bhushan]

> -----Original Message-----
> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Sent: Wednesday, March 13, 2024 12:06 AM
> To: keyrings@vger.kernel.org; linux-crypto@vger.kernel.org;
> herbert@gondor.apana.org.au; davem@davemloft.net
> Cc: linux-kernel@vger.kernel.org; saulo.alessandre@tse.jus.br;
> lukas@wunner.de; Bharat Bhushan <bbhushan2@marvell.com>;
> jarkko@kernel.org; Stefan Berger <stefanb@linux.ibm.com>
> Subject: [EXTERNAL] [PATCH v6 06/13] crypto: ecc - Implement
> vli_mmod_fast_521 for NIST p521
> 
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
> 
> ----------------------------------------------------------------------
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Implement vli_mmod_fast_521 following the description for how to calculate
> the modulus for NIST P521 in the NIST publication "Recommendations for
> Discrete Logarithm-Based Cryptography: Elliptic Curve Domain Parameters"
> section G.1.4.
> 
> NIST p521 requires 9 64bit digits, so increase the ECC_MAX_DIGITS so that
> arrays fit the larger numbers.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  crypto/ecc.c                  | 25 +++++++++++++++++++++++++
>  include/crypto/internal/ecc.h |  3 ++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/ecc.c b/crypto/ecc.c index 415a2f4e7291..99d41887c005
> 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -902,6 +902,28 @@ static void vli_mmod_fast_384(u64 *result, const
> u64 *product,  #undef AND64H  #undef AND64L
> 
> +/*
> + * Computes result = product % curve_prime
> + * from "Recommendations for Discrete Logarithm-Based Cryptography:
> + *       Elliptic Curve Domain Parameters" section G.1.4
> + */
> +static void vli_mmod_fast_521(u64 *result, const u64 *product,
> +			      const u64 *curve_prime, u64 *tmp) {
> +	const unsigned int ndigits = ECC_CURVE_NIST_P521_DIGITS;
> +	size_t i;
> +
> +	/* Initialize result with lowest 521 bits from product */
> +	vli_set(result, product, ndigits);
> +	result[8] &= 0x1ff;
> +
> +	for (i = 0; i < ndigits; i++)
> +		tmp[i] = (product[8 + i] >> 9) | (product[9 + i] << 55);
> +	tmp[8] &= 0x1ff;

Can we get away from this hardcoding, like 9, 55, 0x1ff etc.
Or at least add comment about these.

> +
> +	vli_mod_add(result, result, tmp, curve_prime, ndigits); }
> +
>  /* Computes result = product % curve_prime for different curve_primes.
>   *
>   * Note that curve_primes are distinguished just by heuristic check and @@ -
> 941,6 +963,9 @@ static bool vli_mmod_fast(u64 *result, u64 *product,
>  	case ECC_CURVE_NIST_P384_DIGITS:
>  		vli_mmod_fast_384(result, product, curve_prime, tmp);
>  		break;
> +	case ECC_CURVE_NIST_P521_DIGITS:
> +		vli_mmod_fast_521(result, product, curve_prime, tmp);
> +		break;
>  	default:
>  		pr_err_ratelimited("ecc: unsupported digits size!\n");
>  		return false;
> diff --git a/include/crypto/internal/ecc.h b/include/crypto/internal/ecc.h index
> ab722a8986b7..4e2f5f938e91 100644
> --- a/include/crypto/internal/ecc.h
> +++ b/include/crypto/internal/ecc.h
> @@ -33,7 +33,8 @@
>  #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              (512 / 64) /* due to ecrdsa */
> +#define ECC_CURVE_NIST_P521_DIGITS  9

Maybe these can be defined as:
#define ECC_CURVE_NIST_P521_DIGITS  (DIV_ROUND_UP(521, 64) /* NIST P521 */)

> +#define ECC_MAX_DIGITS              DIV_ROUND_UP(521, 64) /* NIST P521 */

/* NIST_P521 is max digits */
#define ECC_MAX_DIGITS              ECC_CURVE_ _DIGITS

Thanks
-Bharat

> 
>  #define ECC_DIGITS_TO_BYTES_SHIFT 3
> 
> --
> 2.43.0

RE: [EXTERNAL] [PATCH v6 12/13] crypto: asymmetric_keys - Adjust signature size calculation for NIST P521

[Bharat Bhushan]

> -----Original Message-----
> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Sent: Wednesday, March 13, 2024 12:06 AM
> To: keyrings@vger.kernel.org; linux-crypto@vger.kernel.org;
> herbert@gondor.apana.org.au; davem@davemloft.net
> Cc: linux-kernel@vger.kernel.org; saulo.alessandre@tse.jus.br;
> lukas@wunner.de; Bharat Bhushan <bbhushan2@marvell.com>;
> jarkko@kernel.org; Stefan Berger <stefanb@linux.ibm.com>
> Subject: [EXTERNAL] [PATCH v6 12/13] crypto: asymmetric_keys - Adjust
> signature size calculation for NIST P521
> ----------------------------------------------------------------------
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> Adjust the calculation of the maximum signature size for support of NIST
> P521. While existing curves may prepend a 0 byte to their coordinates (to
> make the number positive), NIST P521 will not do this since only the first bit in
> the most significant byte is used.
> 
> If the encoding of the x & y coordinates requires at least 128 bytes then an
> additional byte is needed for the encoding of the length. Take this into account
> when calculating the maximum signature size.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/asymmetric_keys/public_key.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/asymmetric_keys/public_key.c
> b/crypto/asymmetric_keys/public_key.c
> index e5f22691febd..16cc0be28929 100644
> --- a/crypto/asymmetric_keys/public_key.c
> +++ b/crypto/asymmetric_keys/public_key.c
> @@ -233,6 +233,7 @@ static int software_key_query(const struct
> kernel_pkey_params *params,
>  	info->key_size = len * 8;
> 
>  	if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
> +		int slen = len;
>  		/*
>  		 * ECDSA key sizes are much smaller than RSA, and thus could
>  		 * operate on (hashed) inputs that are larger than key size.
> @@ -246,8 +247,19 @@ static int software_key_query(const struct
> kernel_pkey_params *params,
>  		 * Verify takes ECDSA-Sig (described in RFC 5480) as input,
>  		 * which is actually 2 'key_size'-bit integers encoded in
>  		 * ASN.1.  Account for the ASN.1 encoding overhead here.
> +		 *
> +		 * NIST P192/256/384 may prepend a '0' to a coordinate to
> +		 * indicate a positive integer. NIST P521 never needs it.
>  		 */
> -		info->max_sig_size = 2 * (len + 3) + 2;
> +		if (strcmp(pkey->pkey_algo, "ecdsa-nist-p521") != 0)
> +			slen += 1;
> +		/* Length of encoding the x & y coordinates */
> +		slen = 2 * (slen + 2);
> +		/*
> +		 * If coordinate encoding takes at least 128 bytes then an
> +		 * additional byte for length encoding is needed.
> +		 */
> +		info->max_sig_size = 1 + (slen >= 128) + 1 + slen;

Is "(slen >= 128)" valid for P192/256/384 also?

Thanks
-Bharat

>  	} else {
>  		info->max_data_size = len;
>  		info->max_sig_size = len;
> --
> 2.43.0

Re: [EXTERNAL] [PATCH v6 12/13] crypto: asymmetric_keys - Adjust signature size calculation for NIST P521

[Lukas Wunner]

On Mon, Mar 18, 2024 at 05:58:23AM +0000, Bharat Bhushan wrote:
> > --- a/crypto/asymmetric_keys/public_key.c
> > +++ b/crypto/asymmetric_keys/public_key.c
> > @@ -233,6 +233,7 @@ static int software_key_query(const struct
> > kernel_pkey_params *params,
> >  	info->key_size = len * 8;
> > 
> >  	if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
> > +		int slen = len;
> >  		/*
> >  		 * ECDSA key sizes are much smaller than RSA, and thus could
> >  		 * operate on (hashed) inputs that are larger than key size.
> > @@ -246,8 +247,19 @@ static int software_key_query(const struct
> > kernel_pkey_params *params,
> >  		 * Verify takes ECDSA-Sig (described in RFC 5480) as input,
> >  		 * which is actually 2 'key_size'-bit integers encoded in
> >  		 * ASN.1.  Account for the ASN.1 encoding overhead here.
> > +		 *
> > +		 * NIST P192/256/384 may prepend a '0' to a coordinate to
> > +		 * indicate a positive integer. NIST P521 never needs it.
> >  		 */
> > -		info->max_sig_size = 2 * (len + 3) + 2;
> > +		if (strcmp(pkey->pkey_algo, "ecdsa-nist-p521") != 0)
> > +			slen += 1;
> > +		/* Length of encoding the x & y coordinates */
> > +		slen = 2 * (slen + 2);
> > +		/*
> > +		 * If coordinate encoding takes at least 128 bytes then an
> > +		 * additional byte for length encoding is needed.
> > +		 */
> > +		info->max_sig_size = 1 + (slen >= 128) + 1 + slen;
> 
> Is "(slen >= 128)" valid for P192/256/384 also?

It is valid but never true for those.

The signature consists of two integers encoded in ASN.1.
So each integer is prepended by 1 byte for the tag and 1 byte for the length.

The two integers are bundled together in a "sequence", which in turn requires
1 byte for the tag and 1 byte for the length.  However, for P521 the length
of the sequence is at least 2*(1+1+66) = 136 bytes, which exceeds 128 bytes
and therefore the length of the sequence occupies 2 bytes instead of 1.

For the shorter key lengths, the sequence fits in less than 128 bytes and
does not require the extra byte for the sequence length.

So the code is fine AFAICS.

Thanks,

Lukas

Re: [PATCH v6 06/13] crypto: ecc - Implement vli_mmod_fast_521 for NIST p521

[Stefan Berger]

On 3/18/24 01:47, Bharat Bhushan wrote:
> 
> 
>> -----Original Message-----
>> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Sent: Wednesday, March 13, 2024 12:06 AM
>> To: keyrings@vger.kernel.org; linux-crypto@vger.kernel.org;
>> herbert@gondor.apana.org.au; davem@davemloft.net
>> Cc: linux-kernel@vger.kernel.org; saulo.alessandre@tse.jus.br;
>> lukas@wunner.de; Bharat Bhushan <bbhushan2@marvell.com>;
>> jarkko@kernel.org; Stefan Berger <stefanb@linux.ibm.com>
>> Subject: [EXTERNAL] [PATCH v6 06/13] crypto: ecc - Implement
>> vli_mmod_fast_521 for NIST p521
>>
>> Prioritize security for external emails: Confirm sender and content safety
>> before clicking links or opening attachments
>>
>> ----------------------------------------------------------------------
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Implement vli_mmod_fast_521 following the description for how to calculate
>> the modulus for NIST P521 in the NIST publication "Recommendations for
>> Discrete Logarithm-Based Cryptography: Elliptic Curve Domain Parameters"
>> section G.1.4.
>>
>> NIST p521 requires 9 64bit digits, so increase the ECC_MAX_DIGITS so that
>> arrays fit the larger numbers.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>>   crypto/ecc.c                  | 25 +++++++++++++++++++++++++
>>   include/crypto/internal/ecc.h |  3 ++-
>>   2 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/crypto/ecc.c b/crypto/ecc.c index 415a2f4e7291..99d41887c005
>> 100644
>> --- a/crypto/ecc.c
>> +++ b/crypto/ecc.c
>> @@ -902,6 +902,28 @@ static void vli_mmod_fast_384(u64 *result, const
>> u64 *product,  #undef AND64H  #undef AND64L
>>
>> +/*
>> + * Computes result = product % curve_prime
>> + * from "Recommendations for Discrete Logarithm-Based Cryptography:
>> + *       Elliptic Curve Domain Parameters" section G.1.4
>> + */
>> +static void vli_mmod_fast_521(u64 *result, const u64 *product,
>> +			      const u64 *curve_prime, u64 *tmp) {
>> +	const unsigned int ndigits = ECC_CURVE_NIST_P521_DIGITS;
>> +	size_t i;
>> +
>> +	/* Initialize result with lowest 521 bits from product */
>> +	vli_set(result, product, ndigits);
>> +	result[8] &= 0x1ff;
>> +
>> +	for (i = 0; i < ndigits; i++)
>> +		tmp[i] = (product[8 + i] >> 9) | (product[9 + i] << 55);
>> +	tmp[8] &= 0x1ff;
> 
> Can we get away from this hardcoding, like 9, 55, 0x1ff etc.
> Or at least add comment about these.
> 
>> +
>> +	vli_mod_add(result, result, tmp, curve_prime, ndigits); }
>> +
>>   /* Computes result = product % curve_prime for different curve_primes.
>>    *
>>    * Note that curve_primes are distinguished just by heuristic check and @@ -
>> 941,6 +963,9 @@ static bool vli_mmod_fast(u64 *result, u64 *product,
>>   	case ECC_CURVE_NIST_P384_DIGITS:
>>   		vli_mmod_fast_384(result, product, curve_prime, tmp);
>>   		break;
>> +	case ECC_CURVE_NIST_P521_DIGITS:
>> +		vli_mmod_fast_521(result, product, curve_prime, tmp);
>> +		break;
>>   	default:
>>   		pr_err_ratelimited("ecc: unsupported digits size!\n");
>>   		return false;
>> diff --git a/include/crypto/internal/ecc.h b/include/crypto/internal/ecc.h index
>> ab722a8986b7..4e2f5f938e91 100644
>> --- a/include/crypto/internal/ecc.h
>> +++ b/include/crypto/internal/ecc.h
>> @@ -33,7 +33,8 @@
>>   #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              (512 / 64) /* due to ecrdsa */
>> +#define ECC_CURVE_NIST_P521_DIGITS  9
> 
> Maybe these can be defined as:
> #define ECC_CURVE_NIST_P521_DIGITS  (DIV_ROUND_UP(521, 64) /* NIST P521 */)

I think for NIST P521 9 can be pre-calculated. It will not change 
anymore in the future.

> 
>> +#define ECC_MAX_DIGITS              DIV_ROUND_UP(521, 64) /* NIST P521 */
> 
> /* NIST_P521 is max digits */
> #define ECC_MAX_DIGITS              ECC_CURVE_ _DIGITS

In this case I think the DIV_ROUND_UP() along with the comment shows 
that it needs to be updated if ever a larger curve comes along.

> 
> Thanks
> -Bharat
> 
>>
>>   #define ECC_DIGITS_TO_BYTES_SHIFT 3
>>
>> --
>> 2.43.0
> 

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Lukas Wunner]

On Tue, Mar 12, 2024 at 02:36:05PM -0400, Stefan Berger wrote:
> This series adds support for the NIST P521 curve to the ecdsa module
> to enable signature verification with it.

v6 of this series is still

Tested-by: Lukas Wunner <lukas@wunner.de>

as it successfully authenticates PCI devices with the
"TPM_ALG_ECDSA_ECC_NIST_P521" SPDM asymmetric algorithm
using my development branch:

https://github.com/l1k/linux/commits/doe

Thanks,

Lukas

Re: [PATCH v6 01/13] crypto: ecc - Use ECC_CURVE_NIST_P192/256/384_DIGITS where possible

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Replace hard-coded numbers with ECC_CURVE_NIST_P192/256/384_DIGITS where
> possible.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  crypto/ecc.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/crypto/ecc.c b/crypto/ecc.c
> index f53fb4d6af99..415a2f4e7291 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -689,7 +689,7 @@ static void vli_mmod_barrett(u64 *result, u64 *product, const u64 *mod,
>  static void vli_mmod_fast_192(u64 *result, const u64 *product,
>  			      const u64 *curve_prime, u64 *tmp)
>  {
> -	const unsigned int ndigits = 3;
> +	const unsigned int ndigits = ECC_CURVE_NIST_P192_DIGITS;
>  	int carry;
>  
>  	vli_set(result, product, ndigits);
> @@ -717,7 +717,7 @@ static void vli_mmod_fast_256(u64 *result, const u64 *product,
>  			      const u64 *curve_prime, u64 *tmp)
>  {
>  	int carry;
> -	const unsigned int ndigits = 4;
> +	const unsigned int ndigits = ECC_CURVE_NIST_P256_DIGITS;
>  
>  	/* t */
>  	vli_set(result, product, ndigits);
> @@ -800,7 +800,7 @@ static void vli_mmod_fast_384(u64 *result, const u64 *product,
>  				const u64 *curve_prime, u64 *tmp)
>  {
>  	int carry;
> -	const unsigned int ndigits = 6;
> +	const unsigned int ndigits = ECC_CURVE_NIST_P384_DIGITS;
>  
>  	/* t */
>  	vli_set(result, product, ndigits);
> @@ -932,13 +932,13 @@ static bool vli_mmod_fast(u64 *result, u64 *product,
>  	}
>  
>  	switch (ndigits) {
> -	case 3:
> +	case ECC_CURVE_NIST_P192_DIGITS:
>  		vli_mmod_fast_192(result, product, curve_prime, tmp);
>  		break;
> -	case 4:
> +	case ECC_CURVE_NIST_P256_DIGITS:
>  		vli_mmod_fast_256(result, product, curve_prime, tmp);
>  		break;
> -	case 6:
> +	case ECC_CURVE_NIST_P384_DIGITS:
>  		vli_mmod_fast_384(result, product, curve_prime, tmp);
>  		break;
>  	default:

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH v6 02/13] crypto: ecdsa - Convert byte arrays with key coordinates to digits

[Jarkko Sakkinen]

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

Re: [PATCH v6 03/13] crypto: ecdsa - Adjust tests on length of key parameters

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> In preparation for support of NIST P521, adjust the basic tests on the
> length of the provided key parameters to only ensure that the length of the
> x plus y coordinates parameter array is not an odd number and that each
> coordinate fits into an array of 'ndigits' digits. Mathematical tests on
> the key's parameters are then done in ecc_is_pubkey_valid_full rejecting
> invalid keys.
>
> The change is necessary since NIST P521 keys do not have keys with
> coordinates that each fully require 'full' digits (= u64), unlike
> NIST P192/256/384 that all require multiple 'full' digits.

This sentence is not really comprehendable English sentence. Can you
just write the rationale in understandable form?

"fully require full digits (= u64)" is something totally alien to me
tbh.

>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/ecdsa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
> index 6653dec17327..64e1e69d53ba 100644
> --- a/crypto/ecdsa.c
> +++ b/crypto/ecdsa.c
> @@ -230,7 +230,7 @@ static int ecdsa_set_pub_key(struct crypto_akcipher *tfm, const void *key, unsig
>  	if (ret < 0)
>  		return ret;
>  
> -	if (keylen < 1 || (((keylen - 1) >> 1) % sizeof(u64)) != 0)
> +	if (keylen < 1 || ((keylen - 1) & 1) != 0)
>  		return -EINVAL;
>  	/* we only accept uncompressed format indicated by '4' */
>  	if (d[0] != 4)


BR, Jarkko

Re: [PATCH v6 03/13] crypto: ecdsa - Adjust tests on length of key parameters

[Lukas Wunner]

On Mon, Mar 18, 2024 at 10:25:26PM +0200, Jarkko Sakkinen wrote:
> On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> > From: Stefan Berger <stefanb@linux.ibm.com>
> >
> > In preparation for support of NIST P521, adjust the basic tests on the
> > length of the provided key parameters to only ensure that the length of the
> > x plus y coordinates parameter array is not an odd number and that each
> > coordinate fits into an array of 'ndigits' digits. Mathematical tests on
> > the key's parameters are then done in ecc_is_pubkey_valid_full rejecting
> > invalid keys.
> >
> > The change is necessary since NIST P521 keys do not have keys with
> > coordinates that each fully require 'full' digits (= u64), unlike
> > NIST P192/256/384 that all require multiple 'full' digits.
> 
> This sentence is not really comprehendable English sentence. Can you
> just write the rationale in understandable form?
> 
> "fully require full digits (= u64)" is something totally alien to me
> tbh.

It is proper English, but requires an understanding of how large integers
are handled by crypto/ecdsa.c:  They're a sequence of u64.  For P192, P256
and P384 all u64 in the sequence are used to their full extent because the
key size is divisable by 64.  That's not the case for P521, where the most
significant u64 is not fully used (only 2 out of 8 bytes are used).

Thanks,

Lukas

Re: [PATCH v6 04/13] crypto: ecdsa - Extend res.x mod n calculation for NIST P521

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> res.x has been calculated by ecc_point_mult_shamir, which uses
> 'mod curve_prime'. The curve_prime 'p' is typically larger than the
> curve_order 'n' and therefore it is possible that p > res.x >= n.

The first sentence is an incomplete sentence. A lot of cross-referencing
from e.g. elixir is required to "decipher" this commit message :-)

I do get that math here is complicated but for that matter each commit
message should be written with care, minimizing the require cross-
referencing.

These commit messages are adding extra layer of salt.

>
> If res.x >= n then res.x mod n can be calculated by iteratively sub-
> tracting n from res.x until n > res.x. For NIST P192/256/384 this can be
> done in a single subtraction. This can also be done in a single
> subtraction for NIST P521.
>
> The mathematical reason why a single subtraction is sufficient is
> due to the values of 'p' and 'n' of the NIST curves where the following
> holds true:
>
>    note: max(res.x) = p - 1
>
>    max(res.x) - n < n
>        p - 1  - n < n
>        p - 1      < 2n  => true for the NIST curves
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Lukas Wunner <lukas@wunner.de>

What was there to test in this anyway? I see only comment change below.

> ---
>  crypto/ecdsa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
> index 64e1e69d53ba..1814f009f971 100644
> --- a/crypto/ecdsa.c
> +++ b/crypto/ecdsa.c
> @@ -122,7 +122,7 @@ static int _ecdsa_verify(struct ecc_ctx *ctx, const u64 *hash, const u64 *r, con
>  
>  	/* res.x = res.x mod n (if res.x > order) */
>  	if (unlikely(vli_cmp(res.x, curve->n, ndigits) == 1))
> -		/* faster alternative for NIST p384, p256 & p192 */
> +		/* faster alternative for NIST p521, p384, p256 & p192 */
>  		vli_sub(res.x, res.x, curve->n, ndigits);
>  
>  	if (!vli_cmp(res.x, r, ndigits))


BR, Jarkko

Re: [PATCH v6 05/13] crypto: ecc - Add nbits field to ecc_curve structure

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Add the number of bits a curve has to the ecc_curve definition and set it
> on all curve definitions.

"because" is missing.

>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/ecc_curve_defs.h    | 4 ++++
>  crypto/ecrdsa_defs.h       | 5 +++++
>  include/crypto/ecc_curve.h | 2 ++
>  3 files changed, 11 insertions(+)
>
> diff --git a/crypto/ecc_curve_defs.h b/crypto/ecc_curve_defs.h
> index 9719934c9428..ab1ef3d94be5 100644
> --- a/crypto/ecc_curve_defs.h
> +++ b/crypto/ecc_curve_defs.h
> @@ -17,6 +17,7 @@ static u64 nist_p192_b[] = { 0xFEB8DEECC146B9B1ull, 0x0FA7E9AB72243049ull,
>  				0x64210519E59C80E7ull };
>  static struct ecc_curve nist_p192 = {
>  	.name = "nist_192",
> +	.nbits = 192,
>  	.g = {
>  		.x = nist_p192_g_x,
>  		.y = nist_p192_g_y,
> @@ -43,6 +44,7 @@ static u64 nist_p256_b[] = { 0x3BCE3C3E27D2604Bull, 0x651D06B0CC53B0F6ull,
>  				0xB3EBBD55769886BCull, 0x5AC635D8AA3A93E7ull };
>  static struct ecc_curve nist_p256 = {
>  	.name = "nist_256",
> +	.nbits = 256,
>  	.g = {
>  		.x = nist_p256_g_x,
>  		.y = nist_p256_g_y,
> @@ -75,6 +77,7 @@ static u64 nist_p384_b[] = { 0x2a85c8edd3ec2aefull, 0xc656398d8a2ed19dull,
>  				0x988e056be3f82d19ull, 0xb3312fa7e23ee7e4ull };
>  static struct ecc_curve nist_p384 = {
>  	.name = "nist_384",
> +	.nbits = 384,
>  	.g = {
>  		.x = nist_p384_g_x,
>  		.y = nist_p384_g_y,
> @@ -95,6 +98,7 @@ static u64 curve25519_a[] = { 0x000000000001DB41, 0x0000000000000000,
>  				0x0000000000000000, 0x0000000000000000 };
>  static const struct ecc_curve ecc_25519 = {
>  	.name = "curve25519",
> +	.nbits = 255,
>  	.g = {
>  		.x = curve25519_g_x,
>  		.ndigits = 4,
> diff --git a/crypto/ecrdsa_defs.h b/crypto/ecrdsa_defs.h
> index 0056335b9d03..1c2c2449e331 100644
> --- a/crypto/ecrdsa_defs.h
> +++ b/crypto/ecrdsa_defs.h
> @@ -47,6 +47,7 @@ static u64 cp256a_b[] = {
>  
>  static struct ecc_curve gost_cp256a = {
>  	.name = "cp256a",
> +	.nbits = 256,
>  	.g = {
>  		.x = cp256a_g_x,
>  		.y = cp256a_g_y,
> @@ -80,6 +81,7 @@ static u64 cp256b_b[] = {
>  
>  static struct ecc_curve gost_cp256b = {
>  	.name = "cp256b",
> +	.nbits = 256,
>  	.g = {
>  		.x = cp256b_g_x,
>  		.y = cp256b_g_y,
> @@ -117,6 +119,7 @@ static u64 cp256c_b[] = {
>  
>  static struct ecc_curve gost_cp256c = {
>  	.name = "cp256c",
> +	.nbits = 256,
>  	.g = {
>  		.x = cp256c_g_x,
>  		.y = cp256c_g_y,
> @@ -166,6 +169,7 @@ static u64 tc512a_b[] = {
>  
>  static struct ecc_curve gost_tc512a = {
>  	.name = "tc512a",
> +	.nbits = 512,
>  	.g = {
>  		.x = tc512a_g_x,
>  		.y = tc512a_g_y,
> @@ -211,6 +215,7 @@ static u64 tc512b_b[] = {
>  
>  static struct ecc_curve gost_tc512b = {
>  	.name = "tc512b",
> +	.nbits = 512,
>  	.g = {
>  		.x = tc512b_g_x,
>  		.y = tc512b_g_y,
> diff --git a/include/crypto/ecc_curve.h b/include/crypto/ecc_curve.h
> index 70964781eb68..63d5754e7614 100644
> --- a/include/crypto/ecc_curve.h
> +++ b/include/crypto/ecc_curve.h
> @@ -23,6 +23,7 @@ struct ecc_point {
>   * struct ecc_curve - definition of elliptic curve
>   *
>   * @name:	Short name of the curve.
> + * @nbits:	The number of bits of a curve.
>   * @g:		Generator point of the curve.
>   * @p:		Prime number, if Barrett's reduction is used for this curve
>   *		pre-calculated value 'mu' is appended to the @p after ndigits.
> @@ -34,6 +35,7 @@ struct ecc_point {
>   */
>  struct ecc_curve {
>  	char *name;
> +	unsigned int nbits;
>  	struct ecc_point g;
>  	u64 *p;
>  	u64 *n;


BR, Jarkko

Re: [PATCH v6 06/13] crypto: ecc - Implement vli_mmod_fast_521 for NIST p521

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Implement vli_mmod_fast_521 following the description for how to calculate
> the modulus for NIST P521 in the NIST publication "Recommendations for
> Discrete Logarithm-Based Cryptography: Elliptic Curve Domain Parameters"
> section G.1.4.
>
> NIST p521 requires 9 64bit digits, so increase the ECC_MAX_DIGITS so that
> arrays fit the larger numbers.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Should provide context on what it will be used (for completeness).


> ---
>  crypto/ecc.c                  | 25 +++++++++++++++++++++++++
>  include/crypto/internal/ecc.h |  3 ++-
>  2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/ecc.c b/crypto/ecc.c
> index 415a2f4e7291..99d41887c005 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -902,6 +902,28 @@ static void vli_mmod_fast_384(u64 *result, const u64 *product,
>  #undef AND64H
>  #undef AND64L
>  
> +/*
> + * Computes result = product % curve_prime
> + * from "Recommendations for Discrete Logarithm-Based Cryptography:
> + *       Elliptic Curve Domain Parameters" section G.1.4
> + */
> +static void vli_mmod_fast_521(u64 *result, const u64 *product,
> +			      const u64 *curve_prime, u64 *tmp)
> +{
> +	const unsigned int ndigits = ECC_CURVE_NIST_P521_DIGITS;
> +	size_t i;
> +
> +	/* Initialize result with lowest 521 bits from product */
> +	vli_set(result, product, ndigits);
> +	result[8] &= 0x1ff;
> +
> +	for (i = 0; i < ndigits; i++)
> +		tmp[i] = (product[8 + i] >> 9) | (product[9 + i] << 55);
> +	tmp[8] &= 0x1ff;
> +
> +	vli_mod_add(result, result, tmp, curve_prime, ndigits);
> +}
> +
>  /* Computes result = product % curve_prime for different curve_primes.
>   *
>   * Note that curve_primes are distinguished just by heuristic check and
> @@ -941,6 +963,9 @@ static bool vli_mmod_fast(u64 *result, u64 *product,
>  	case ECC_CURVE_NIST_P384_DIGITS:
>  		vli_mmod_fast_384(result, product, curve_prime, tmp);
>  		break;
> +	case ECC_CURVE_NIST_P521_DIGITS:
> +		vli_mmod_fast_521(result, product, curve_prime, tmp);
> +		break;
>  	default:
>  		pr_err_ratelimited("ecc: unsupported digits size!\n");
>  		return false;
> diff --git a/include/crypto/internal/ecc.h b/include/crypto/internal/ecc.h
> index ab722a8986b7..4e2f5f938e91 100644
> --- a/include/crypto/internal/ecc.h
> +++ b/include/crypto/internal/ecc.h
> @@ -33,7 +33,8 @@
>  #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              (512 / 64) /* due to ecrdsa */
> +#define ECC_CURVE_NIST_P521_DIGITS  9
> +#define ECC_MAX_DIGITS              DIV_ROUND_UP(521, 64) /* NIST P521 */
>  
>  #define ECC_DIGITS_TO_BYTES_SHIFT 3
>  

Otherwise, lgtm

BR, Jarkko

Re: [PATCH v6 02/13] crypto: ecdsa - Convert byte arrays with key coordinates to digits

[Lukas Wunner]

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

Re: [PATCH v6 04/13] crypto: ecdsa - Extend res.x mod n calculation for NIST P521

[Lukas Wunner]

On Mon, Mar 18, 2024 at 10:33:47PM +0200, Jarkko Sakkinen wrote:
> On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > Tested-by: Lukas Wunner <lukas@wunner.de>
> 
> What was there to test in this anyway? I see only comment change below.

The full series was tested, irrespective of the content of the individual
patches.

Thanks,

Lukas

Re: [PATCH v6 07/13] crypto: ecc - Add special case for NIST P521 in ecc_point_mult

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> In ecc_point_mult use the number of bits of the NIST P521 curve + 2. The
> change is required specifically for NIST P521 to pass mathematical tests
> on the public key.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/ecc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/ecc.c b/crypto/ecc.c
> index 99d41887c005..ead40b5ebb46 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -1320,7 +1320,10 @@ static void ecc_point_mult(struct ecc_point *result,
>  	carry = vli_add(sk[0], scalar, curve->n, ndigits);
>  	vli_add(sk[1], sk[0], curve->n, ndigits);
>  	scalar = sk[!carry];
> -	num_bits = sizeof(u64) * ndigits * 8 + 1;
> +	if (curve->nbits == 521)	/* NIST P521 */
> +		num_bits = curve->nbits + 2;
> +	else
> +		num_bits = sizeof(u64) * ndigits * 8 + 1;
>  
>  	vli_set(rx[1], point->x, ndigits);
>  	vli_set(ry[1], point->y, ndigits);


Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH v6 08/13] crypto: ecc - Add NIST P521 curve parameters

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Add the parameters for the NIST P521 curve and define a new curve ID
> for it. Make the curve available in ecc_get_curve.

This is rare example of "complete story" in this series despite being
short, so no complains :-)

>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/ecc.c            |  2 ++
>  crypto/ecc_curve_defs.h | 45 +++++++++++++++++++++++++++++++++++++++++
>  include/crypto/ecdh.h   |  1 +
>  3 files changed, 48 insertions(+)
>
> diff --git a/crypto/ecc.c b/crypto/ecc.c
> index ead40b5ebb46..4f6fa8617308 100644
> --- a/crypto/ecc.c
> +++ b/crypto/ecc.c
> @@ -60,6 +60,8 @@ const struct ecc_curve *ecc_get_curve(unsigned int curve_id)
>  		return &nist_p256;
>  	case ECC_CURVE_NIST_P384:
>  		return &nist_p384;
> +	case ECC_CURVE_NIST_P521:
> +		return &nist_p521;
>  	default:
>  		return NULL;
>  	}
> diff --git a/crypto/ecc_curve_defs.h b/crypto/ecc_curve_defs.h
> index ab1ef3d94be5..0ecade7d02f5 100644
> --- a/crypto/ecc_curve_defs.h
> +++ b/crypto/ecc_curve_defs.h
> @@ -89,6 +89,51 @@ static struct ecc_curve nist_p384 = {
>  	.b = nist_p384_b
>  };
>  
> +/* NIST P-521 */
> +static u64 nist_p521_g_x[] = { 0xf97e7e31c2e5bd66ull, 0x3348b3c1856a429bull,
> +				0xfe1dc127a2ffa8deull, 0xa14b5e77efe75928ull,
> +				0xf828af606b4d3dbaull, 0x9c648139053fb521ull,
> +				0x9e3ecb662395b442ull, 0x858e06b70404e9cdull,
> +				0xc6ull };
> +static u64 nist_p521_g_y[] = { 0x88be94769fd16650ull, 0x353c7086a272c240ull,
> +				0xc550b9013fad0761ull, 0x97ee72995ef42640ull,
> +				0x17afbd17273e662cull, 0x98f54449579b4468ull,
> +				0x5c8a5fb42c7d1bd9ull, 0x39296a789a3bc004ull,
> +				0x118ull };
> +static u64 nist_p521_p[] = { 0xffffffffffffffffull, 0xffffffffffffffffull,
> +				0xffffffffffffffffull, 0xffffffffffffffffull,
> +				0xffffffffffffffffull, 0xffffffffffffffffull,
> +				0xffffffffffffffffull, 0xffffffffffffffffull,
> +				0x1ffull };
> +static u64 nist_p521_n[] = { 0xbb6fb71e91386409ull, 0x3bb5c9b8899c47aeull,
> +				0x7fcc0148f709a5d0ull, 0x51868783bf2f966bull,
> +				0xfffffffffffffffaull, 0xffffffffffffffffull,
> +				0xffffffffffffffffull, 0xffffffffffffffffull,
> +				0x1ffull };
> +static u64 nist_p521_a[] = { 0xfffffffffffffffcull, 0xffffffffffffffffull,
> +				0xffffffffffffffffull, 0xffffffffffffffffull,
> +				0xffffffffffffffffull, 0xffffffffffffffffull,
> +				0xffffffffffffffffull, 0xffffffffffffffffull,
> +				0x1ffull };
> +static u64 nist_p521_b[] = { 0xef451fd46b503f00ull, 0x3573df883d2c34f1ull,
> +				0x1652c0bd3bb1bf07ull, 0x56193951ec7e937bull,
> +				0xb8b489918ef109e1ull, 0xa2da725b99b315f3ull,
> +				0x929a21a0b68540eeull, 0x953eb9618e1c9a1full,
> +				0x051ull };
> +static struct ecc_curve nist_p521 = {
> +	.name = "nist_521",
> +	.nbits = 521,
> +	.g = {
> +		.x = nist_p521_g_x,
> +		.y = nist_p521_g_y,
> +		.ndigits = 9,
> +	},
> +	.p = nist_p521_p,
> +	.n = nist_p521_n,
> +	.a = nist_p521_a,
> +	.b = nist_p521_b
> +};
> +
>  /* curve25519 */
>  static u64 curve25519_g_x[] = { 0x0000000000000009, 0x0000000000000000,
>  				0x0000000000000000, 0x0000000000000000 };
> diff --git a/include/crypto/ecdh.h b/include/crypto/ecdh.h
> index a9f98078d29c..9784ecdd2fb4 100644
> --- a/include/crypto/ecdh.h
> +++ b/include/crypto/ecdh.h
> @@ -26,6 +26,7 @@
>  #define ECC_CURVE_NIST_P192	0x0001
>  #define ECC_CURVE_NIST_P256	0x0002
>  #define ECC_CURVE_NIST_P384	0x0003
> +#define ECC_CURVE_NIST_P521	0x0004
>  
>  /**
>   * struct ecdh - define an ECDH private key


Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH v6 09/13] crypto: ecdsa - Replace ndigits with nbits where precision is needed

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Replace the usage of ndigits with nbits where more precise space
> calculations are needed, such as in ecdsa_max_size where the length of a
> coordinate is determined.

What does "more precise" mean?

>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/ecdsa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
> index 1814f009f971..4daefb40c37a 100644
> --- a/crypto/ecdsa.c
> +++ b/crypto/ecdsa.c
> @@ -266,7 +266,7 @@ static unsigned int ecdsa_max_size(struct crypto_akcipher *tfm)
>  {
>  	struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm);
>  
> -	return ctx->pub_key.ndigits << ECC_DIGITS_TO_BYTES_SHIFT;
> +	return DIV_ROUND_UP(ctx->curve->nbits, 8);
>  }
>  
>  static int ecdsa_nist_p384_init_tfm(struct crypto_akcipher *tfm)

BR, Jarkko

Re: [PATCH v6 10/13] crypto: ecdsa - Rename keylen to bufsize where necessary

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> In some cases the name keylen does not reflect the purpose of the variable

What are "some cases"?

Just pointing out stuff that should be enumerated properly because this 
requires the reader to complete the commit messages by open coding "some
cases".

> anymore once NIST P521 is used but it is the size of the buffer. There-
> for, rename keylen to bufsize where appropriate.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/ecdsa.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
> index 4daefb40c37a..4e847b59622a 100644
> --- a/crypto/ecdsa.c
> +++ b/crypto/ecdsa.c
> @@ -35,8 +35,8 @@ struct ecdsa_signature_ctx {
>  static int ecdsa_get_signature_rs(u64 *dest, size_t hdrlen, unsigned char tag,
>  				  const void *value, size_t vlen, unsigned int ndigits)
>  {
> -	size_t keylen = ndigits * sizeof(u64);
> -	ssize_t diff = vlen - keylen;
> +	size_t bufsize = ndigits * sizeof(u64);
> +	ssize_t diff = vlen - bufsize;
>  	const char *d = value;
>  	u8 rs[ECC_MAX_BYTES];
>  
> @@ -58,7 +58,7 @@ static int ecdsa_get_signature_rs(u64 *dest, size_t hdrlen, unsigned char tag,
>  		if (diff)
>  			return -EINVAL;
>  	}
> -	if (-diff >= keylen)
> +	if (-diff >= bufsize)
>  		return -EINVAL;
>  
>  	if (diff) {
> @@ -138,7 +138,7 @@ static int ecdsa_verify(struct akcipher_request *req)
>  {
>  	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
>  	struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm);
> -	size_t keylen = ctx->curve->g.ndigits * sizeof(u64);
> +	size_t bufsize = ctx->curve->g.ndigits * sizeof(u64);
>  	struct ecdsa_signature_ctx sig_ctx = {
>  		.curve = ctx->curve,
>  	};
> @@ -165,14 +165,14 @@ static int ecdsa_verify(struct akcipher_request *req)
>  		goto error;
>  
>  	/* if the hash is shorter then we will add leading zeros to fit to ndigits */
> -	diff = keylen - req->dst_len;
> +	diff = bufsize - req->dst_len;
>  	if (diff >= 0) {
>  		if (diff)
>  			memset(rawhash, 0, diff);
>  		memcpy(&rawhash[diff], buffer + req->src_len, req->dst_len);
>  	} else if (diff < 0) {
>  		/* given hash is longer, we take the left-most bytes */
> -		memcpy(&rawhash, buffer + req->src_len, keylen);
> +		memcpy(&rawhash, buffer + req->src_len, bufsize);
>  	}
>  
>  	ecc_swap_digits((u64 *)rawhash, hash, ctx->curve->g.ndigits);

BR, Jarkko

Re: [PATCH v6 11/13] crypto: ecdsa - Register NIST P521 and extend test suite

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Register NIST P521 as an akcipher and extend the testmgr with
> NIST P521-specific test vectors.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/ecdsa.c   |  30 ++++++++++
>  crypto/testmgr.c |   7 +++
>  crypto/testmgr.h | 146 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 183 insertions(+)
>
> diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c
> index 4e847b59622a..894599f1885f 100644
> --- a/crypto/ecdsa.c
> +++ b/crypto/ecdsa.c
> @@ -269,6 +269,28 @@ static unsigned int ecdsa_max_size(struct crypto_akcipher *tfm)
>  	return DIV_ROUND_UP(ctx->curve->nbits, 8);
>  }
>  
> +static int ecdsa_nist_p521_init_tfm(struct crypto_akcipher *tfm)
> +{
> +	struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm);
> +
> +	return ecdsa_ecc_ctx_init(ctx, ECC_CURVE_NIST_P521);
> +}
> +
> +static struct akcipher_alg ecdsa_nist_p521 = {
> +	.verify = ecdsa_verify,
> +	.set_pub_key = ecdsa_set_pub_key,
> +	.max_size = ecdsa_max_size,
> +	.init = ecdsa_nist_p521_init_tfm,
> +	.exit = ecdsa_exit_tfm,
> +	.base = {
> +		.cra_name = "ecdsa-nist-p521",
> +		.cra_driver_name = "ecdsa-nist-p521-generic",
> +		.cra_priority = 100,
> +		.cra_module = THIS_MODULE,
> +		.cra_ctxsize = sizeof(struct ecc_ctx),
> +	},
> +};
> +
>  static int ecdsa_nist_p384_init_tfm(struct crypto_akcipher *tfm)
>  {
>  	struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm);
> @@ -352,8 +374,15 @@ static int __init ecdsa_init(void)
>  	if (ret)
>  		goto nist_p384_error;
>  
> +	ret = crypto_register_akcipher(&ecdsa_nist_p521);
> +	if (ret)
> +		goto nist_p521_error;
> +
>  	return 0;
>  
> +nist_p521_error:
> +	crypto_unregister_akcipher(&ecdsa_nist_p384);
> +
>  nist_p384_error:
>  	crypto_unregister_akcipher(&ecdsa_nist_p256);
>  
> @@ -369,6 +398,7 @@ static void __exit ecdsa_exit(void)
>  		crypto_unregister_akcipher(&ecdsa_nist_p192);
>  	crypto_unregister_akcipher(&ecdsa_nist_p256);
>  	crypto_unregister_akcipher(&ecdsa_nist_p384);
> +	crypto_unregister_akcipher(&ecdsa_nist_p521);
>  }
>  
>  subsys_initcall(ecdsa_init);
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index c26aeda85787..a017b4ad119b 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -5097,6 +5097,13 @@ static const struct alg_test_desc alg_test_descs[] = {
>  		.suite = {
>  			.akcipher = __VECS(ecdsa_nist_p384_tv_template)
>  		}
> +	}, {
> +		.alg = "ecdsa-nist-p521",
> +		.test = alg_test_akcipher,
> +		.fips_allowed = 1,
> +		.suite = {
> +			.akcipher = __VECS(ecdsa_nist_p521_tv_template)
> +		}
>  	}, {
>  		.alg = "ecrdsa",
>  		.test = alg_test_akcipher,
> diff --git a/crypto/testmgr.h b/crypto/testmgr.h
> index 986f331a5fc2..9bde04be8df9 100644
> --- a/crypto/testmgr.h
> +++ b/crypto/testmgr.h
> @@ -991,6 +991,152 @@ static const struct akcipher_testvec ecdsa_nist_p384_tv_template[] = {
>  	},
>  };
>  
> +static const struct akcipher_testvec ecdsa_nist_p521_tv_template[] = {
> +	{
> +	.key = /* secp521r1(sha224) */
> +	"\x04\x01\x4f\x43\x18\xb6\xa9\xc9\x5d\x68\xd3\xa9\x42\xf8\x98\xc0"
> +	"\xd2\xd1\xa9\x50\x3b\xe8\xc4\x40\xe6\x11\x78\x88\x4b\xbd\x76\xa7"
> +	"\x9a\xe0\xdd\x31\xa4\x67\x78\x45\x33\x9e\x8c\xd1\xc7\x44\xac\x61"
> +	"\x68\xc8\x04\xe7\x5c\x79\xb1\xf1\x41\x0c\x71\xc0\x53\xa8\xbc\xfb"
> +	"\xf5\xca\xd4\x01\x40\xfd\xa3\x45\xda\x08\xe0\xb4\xcb\x28\x3b\x0a"
> +	"\x02\x35\x5f\x02\x9f\x3f\xcd\xef\x08\x22\x40\x97\x74\x65\xb7\x76"
> +	"\x85\xc7\xc0\x5c\xfb\x81\xe1\xa5\xde\x0c\x4e\x8b\x12\x31\xb6\x47"
> +	"\xed\x37\x0f\x99\x3f\x26\xba\xa3\x8e\xff\x79\x34\x7c\x3a\xfe\x1f"
> +	"\x3b\x83\x82\x2f\x14",
> +	.key_len = 133,
> +	.params =
> +	"\x30\x10\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x05\x2b\x81\x04"
> +	"\x00\x23",
> +	.param_len = 18,
> +	.m =
> +	"\xa2\x3a\x6a\x8c\x7b\x3c\xf2\x51\xf8\xbe\x5f\x4f\x3b\x15\x05\xc4"
> +	"\xb5\xbc\x19\xe7\x21\x85\xe9\x23\x06\x33\x62\xfb",
> +	.m_size = 28,
> +	.algo = OID_id_ecdsa_with_sha224,
> +	.c =
> +	"\x30\x81\x86\x02\x41\x01\xd6\x43\xe7\xff\x42\xb2\xba\x74\x35\xf6"
> +	"\xdc\x6d\x02\x7b\x22\xac\xe2\xef\x07\x92\xee\x60\x94\x06\xf8\x3f"
> +	"\x59\x0f\x74\xf0\x3f\xd8\x18\xc6\x37\x8a\xcb\xa7\xd8\x7d\x98\x85"
> +	"\x29\x88\xff\x0b\x94\x94\x6c\xa6\x9b\x89\x8b\x1e\xfd\x09\x46\x6b"
> +	"\xc7\xaf\x7a\xb9\x19\x0a\x02\x41\x3a\x26\x0d\x55\xcd\x23\x1e\x7d"
> +	"\xa0\x5e\xf9\x88\xf3\xd2\x32\x90\x57\x0f\xf8\x65\x97\x6b\x09\x4d"
> +	"\x22\x26\x0b\x5f\x49\x32\x6b\x91\x99\x30\x90\x0f\x1c\x8f\x78\xd3"
> +	"\x9f\x0e\x64\xcc\xc4\xe8\x43\xd9\x0e\x1c\xad\x22\xda\x82\x00\x35"
> +	"\xa3\x50\xb1\xa5\x98\x92\x2a\xa5\x52",
> +	.c_size = 137,
> +	.public_key_vec = true,
> +	.siggen_sigver_test = true,
> +	},
> +	{
> +	.key = /* secp521r1(sha256) */
> +	"\x04\x01\x05\x3a\x6b\x3b\x5a\x0f\xa7\xb9\xb7\x32\x53\x4e\xe2\xae"
> +	"\x0a\x52\xc5\xda\xdd\x5a\x79\x1c\x30\x2d\x33\x07\x79\xd5\x70\x14"
> +	"\x61\x0c\xec\x26\x4d\xd8\x35\x57\x04\x1d\x88\x33\x4d\xce\x05\x36"
> +	"\xa5\xaf\x56\x84\xfa\x0b\x9e\xff\x7b\x30\x4b\x92\x1d\x06\xf8\x81"
> +	"\x24\x1e\x51\x00\x09\x21\x51\xf7\x46\x0a\x77\xdb\xb5\x0c\xe7\x9c"
> +	"\xff\x27\x3c\x02\x71\xd7\x85\x36\xf1\xaa\x11\x59\xd8\xb8\xdc\x09"
> +	"\xdc\x6d\x5a\x6f\x63\x07\x6c\xe1\xe5\x4d\x6e\x0f\x6e\xfb\x7c\x05"
> +	"\x8a\xe9\x53\xa8\xcf\xce\x43\x0e\x82\x20\x86\xbc\x88\x9c\xb7\xe3"
> +	"\xe6\x77\x1e\x1f\x8a",
> +	.key_len = 133,
> +	.params =
> +	"\x30\x10\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x05\x2b\x81\x04"
> +	"\x00\x23",
> +	.param_len = 18,
> +	.m =
> +	"\xcc\x97\x73\x0c\x73\xa2\x53\x2b\xfa\xd7\x83\x1d\x0c\x72\x1b\x39"
> +	"\x80\x71\x8d\xdd\xc5\x9b\xff\x55\x32\x98\x25\xa2\x58\x2e\xb7\x73",
> +	.m_size = 32,
> +	.algo = OID_id_ecdsa_with_sha256,
> +	.c =
> +	"\x30\x81\x88\x02\x42\x00\xcd\xa5\x5f\x57\x52\x27\x78\x3a\xb5\x06"
> +	"\x0f\xfd\x83\xfc\x0e\xd9\xce\x50\x9f\x7d\x1f\xca\x8b\xa8\x2d\x56"
> +	"\x3c\xf6\xf0\xd8\xe1\xb7\x5d\x95\x35\x6f\x02\x0e\xaf\xe1\x4c\xae"
> +	"\xce\x54\x76\x9a\xc2\x8f\xb8\x38\x1f\x46\x0b\x04\x64\x34\x79\xde"
> +	"\x7e\xd7\x59\x10\xe9\xd9\xd5\x02\x42\x01\xcf\x50\x85\x38\xf9\x15"
> +	"\x83\x18\x04\x6b\x35\xae\x65\xb5\x99\x12\x0a\xa9\x79\x24\xb9\x37"
> +	"\x35\xdd\xa0\xe0\x87\x2c\x44\x4b\x5a\xee\xaf\xfa\x10\xdd\x9b\xfb"
> +	"\x36\x1a\x31\x03\x42\x02\x5f\x50\xf0\xa2\x0d\x1c\x57\x56\x8f\x12"
> +	"\xb7\x1d\x91\x55\x38\xb6\xf6\x34\x65\xc7\xbd",
> +	.c_size = 139,
> +	.public_key_vec = true,
> +	.siggen_sigver_test = true,
> +	},
> +	{
> +	.key = /* secp521r1(sha384) */
> +	"\x04\x00\x2e\xd6\x21\x04\x75\xc3\xdc\x7d\xff\x0e\xf3\x70\x25\x2b"
> +	"\xad\x72\xfc\x5a\x91\xf1\xd5\x9c\x64\xf3\x1f\x47\x11\x10\x62\x33"
> +	"\xfd\x2e\xe8\x32\xca\x9e\x6f\x0a\x4c\x5b\x35\x9a\x46\xc5\xe7\xd4"
> +	"\x38\xda\xb2\xf0\xf4\x87\xf3\x86\xf4\xea\x70\xad\x1e\xd4\x78\x8c"
> +	"\x36\x18\x17\x00\xa2\xa0\x34\x1b\x2e\x6a\xdf\x06\xd6\x99\x2d\x47"
> +	"\x50\x92\x1a\x8a\x72\x9c\x23\x44\xfa\xa7\xa9\xed\xa6\xef\x26\x14"
> +	"\xb3\x9d\xfe\x5e\xa3\x8c\xd8\x29\xf8\xdf\xad\xa6\xab\xfc\xdd\x46"
> +	"\x22\x6e\xd7\x35\xc7\x23\xb7\x13\xae\xb6\x34\xff\xd7\x80\xe5\x39"
> +	"\xb3\x3b\x5b\x1b\x94",
> +	.key_len = 133,
> +	.params =
> +	"\x30\x10\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x05\x2b\x81\x04"
> +	"\x00\x23",
> +	.param_len = 18,
> +	.m =
> +	"\x36\x98\xd6\x82\xfa\xad\xed\x3c\xb9\x40\xb6\x4d\x9e\xb7\x04\x26"
> +	"\xad\x72\x34\x44\xd2\x81\xb4\x9b\xbe\x01\x04\x7a\xd8\x50\xf8\x59"
> +	"\xba\xad\x23\x85\x6b\x59\xbe\xfb\xf6\x86\xd4\x67\xa8\x43\x28\x76",
> +	.m_size = 48,
> +	.algo = OID_id_ecdsa_with_sha384,
> +	.c =
> +	"\x30\x81\x88\x02\x42\x00\x93\x96\x76\x3c\x27\xea\xaa\x9c\x26\xec"
> +	"\x51\xdc\xe8\x35\x5e\xae\x16\xf2\x4b\x64\x98\xf7\xec\xda\xc7\x7e"
> +	"\x42\x71\x86\x57\x2d\xf1\x7d\xe4\xdf\x9b\x7d\x9e\x47\xca\x33\x32"
> +	"\x76\x06\xd0\xf9\xc0\xe4\xe6\x84\x59\xfd\x1a\xc4\x40\xdd\x43\xb8"
> +	"\x6a\xdd\xfb\xe6\x63\x4e\x28\x02\x42\x00\xff\xc3\x6a\x87\x6e\xb5"
> +	"\x13\x1f\x20\x55\xce\x37\x97\xc9\x05\x51\xe5\xe4\x3c\xbc\x93\x65"
> +	"\x57\x1c\x30\xda\xa7\xcd\x26\x28\x76\x3b\x52\xdf\xc4\xc0\xdb\x54"
> +	"\xdb\x8a\x0d\x6a\xc3\xf3\x7a\xd1\xfa\xe7\xa7\xe5\x5a\x94\x56\xcf"
> +	"\x8f\xb4\x22\xc6\x4f\xab\x2b\x62\xc1\x42\xb1",
> +	.c_size = 139,
> +	.public_key_vec = true,
> +	.siggen_sigver_test = true,
> +	},
> +	{
> +	.key = /* secp521r1(sha512) */
> +	"\x04\x00\xc7\x65\xee\x0b\x86\x7d\x8f\x02\xf1\x74\x5b\xb0\x4c\x3f"
> +	"\xa6\x35\x60\x9f\x55\x23\x11\xcc\xdf\xb8\x42\x99\xee\x6c\x96\x6a"
> +	"\x27\xa2\x56\xb2\x2b\x03\xad\x0f\xe7\x97\xde\x09\x5d\xb4\xc5\x5f"
> +	"\xbd\x87\x37\xbf\x5a\x16\x35\x56\x08\xfd\x6f\x06\x1a\x1c\x84\xee"
> +	"\xc3\x64\xb3\x00\x9e\xbd\x6e\x60\x76\xee\x69\xfd\x3a\xb8\xcd\x7e"
> +	"\x91\x68\x53\x57\x44\x13\x2e\x77\x09\x2a\xbe\x48\xbd\x91\xd8\xf6"
> +	"\x21\x16\x53\x99\xd5\xf0\x40\xad\xa6\xf8\x58\x26\xb6\x9a\xf8\x77"
> +	"\xfe\x3a\x05\x1a\xdb\xa9\x0f\xc0\x6c\x76\x30\x8c\xd8\xde\x44\xae"
> +	"\xd0\x17\xdf\x49\x6a",
> +	.key_len = 133,
> +	.params =
> +	"\x30\x10\x06\x07\x2a\x86\x48\xce\x3d\x02\x01\x06\x05\x2b\x81\x04"
> +	"\x00\x23",
> +	.param_len = 18,
> +	.m =
> +	"\x5c\xa6\xbc\x79\xb8\xa0\x1e\x11\x83\xf7\xe9\x05\xdf\xba\xf7\x69"
> +	"\x97\x22\x32\xe4\x94\x7c\x65\xbd\x74\xc6\x9a\x8b\xbd\x0d\xdc\xed"
> +	"\xf5\x9c\xeb\xe1\xc5\x68\x40\xf2\xc7\x04\xde\x9e\x0d\x76\xc5\xa3"
> +	"\xf9\x3c\x6c\x98\x08\x31\xbd\x39\xe8\x42\x7f\x80\x39\x6f\xfe\x68",
> +	.m_size = 64,
> +	.algo = OID_id_ecdsa_with_sha512,
> +	.c =
> +	"\x30\x81\x88\x02\x42\x01\x5c\x71\x86\x96\xac\x21\x33\x7e\x4e\xaa"
> +	"\x86\xec\xa8\x05\x03\x52\x56\x63\x0e\x02\xcc\x94\xa9\x05\xb9\xfb"
> +	"\x62\x1e\x42\x03\x6c\x74\x8a\x1f\x12\x3e\xb7\x7e\x51\xff\x7f\x27"
> +	"\x93\xe8\x6c\x49\x7d\x28\xfc\x80\xa6\x13\xfc\xb6\x90\xf7\xbb\x28"
> +	"\xb5\x04\xb0\xb6\x33\x1c\x7e\x02\x42\x01\x70\x43\x52\x1d\xe3\xc6"
> +	"\xbd\x5a\x40\x95\x35\x89\x4f\x41\x5f\x9e\x19\x88\x05\x3e\x43\x39"
> +	"\x01\xbd\xb7\x7a\x76\x37\x51\x47\x49\x98\x12\x71\xd0\xe9\xca\xa7"
> +	"\xc0\xcb\xaa\x00\x55\xbb\x6a\xb4\x73\x00\xd2\x72\x74\x13\x63\x39"
> +	"\xa6\xe5\x25\x46\x1e\x77\x44\x78\xe0\xd1\x04",
> +	.c_size = 139,
> +	.public_key_vec = true,
> +	.siggen_sigver_test = true,
> +	},
> +};
> +
>  /*
>   * EC-RDSA test vectors are generated by gost-engine.
>   */

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

Re: [PATCH v6 12/13] crypto: asymmetric_keys - Adjust signature size calculation for NIST P521

[Jarkko Sakkinen]

On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Adjust the calculation of the maximum signature size for support of
> NIST P521. While existing curves may prepend a 0 byte to their coordinates
> (to make the number positive), NIST P521 will not do this since only the
> first bit in the most significant byte is used.
>
> If the encoding of the x & y coordinates requires at least 128 bytes then
> an additional byte is needed for the encoding of the length. Take this into
> account when calculating the maximum signature size.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> Tested-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/asymmetric_keys/public_key.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
> index e5f22691febd..16cc0be28929 100644
> --- a/crypto/asymmetric_keys/public_key.c
> +++ b/crypto/asymmetric_keys/public_key.c
> @@ -233,6 +233,7 @@ static int software_key_query(const struct kernel_pkey_params *params,
>  	info->key_size = len * 8;
>  
>  	if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
> +		int slen = len;
>  		/*
>  		 * ECDSA key sizes are much smaller than RSA, and thus could
>  		 * operate on (hashed) inputs that are larger than key size.
> @@ -246,8 +247,19 @@ static int software_key_query(const struct kernel_pkey_params *params,
>  		 * Verify takes ECDSA-Sig (described in RFC 5480) as input,
>  		 * which is actually 2 'key_size'-bit integers encoded in
>  		 * ASN.1.  Account for the ASN.1 encoding overhead here.
> +		 *
> +		 * NIST P192/256/384 may prepend a '0' to a coordinate to
> +		 * indicate a positive integer. NIST P521 never needs it.
>  		 */
> -		info->max_sig_size = 2 * (len + 3) + 2;
> +		if (strcmp(pkey->pkey_algo, "ecdsa-nist-p521") != 0)
> +			slen += 1;

Just wondering the logic of picking between these:

1. "strncmp"
2. "strcmp"

Now the "ecdsa" is matched with strncmp and "ecdsa-nist-p521" is
compared with strcmp.

So is there a good reason to use different function in these
cases?

I'd guess both could be using strcmp since comparing against
constant...

> +		/* Length of encoding the x & y coordinates */
> +		slen = 2 * (slen + 2);
> +		/*
> +		 * If coordinate encoding takes at least 128 bytes then an
> +		 * additional byte for length encoding is needed.
> +		 */
> +		info->max_sig_size = 1 + (slen >= 128) + 1 + slen;
>  	} else {
>  		info->max_data_size = len;
>  		info->max_sig_size = len;


BR, Jarkko

Re: [PATCH v6 04/13] crypto: ecdsa - Extend res.x mod n calculation for NIST P521

[Jarkko Sakkinen]

On Mon Mar 18, 2024 at 10:39 PM EET, Lukas Wunner wrote:
> On Mon, Mar 18, 2024 at 10:33:47PM +0200, Jarkko Sakkinen wrote:
> > On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > > Tested-by: Lukas Wunner <lukas@wunner.de>
> > 
> > What was there to test in this anyway? I see only comment change below.
>
> The full series was tested, irrespective of the content of the individual
> patches.

Tested-by's should be per patch, and in this patch tested-by has no
meaning at all.

In order to determine which patches tested-by is applicable it can
be derived on what was actually tested.

This looks as tested-by was used in place of acked/reviewed-by, which
is not how it should be used.

BR, Jarkko

Re: [PATCH v6 02/13] crypto: ecdsa - Convert byte arrays with key coordinates to digits

[Jarkko Sakkinen]

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

Re: [PATCH v6 03/13] crypto: ecdsa - Adjust tests on length of key parameters

[Jarkko Sakkinen]

On Mon Mar 18, 2024 at 10:32 PM EET, Lukas Wunner wrote:
> On Mon, Mar 18, 2024 at 10:25:26PM +0200, Jarkko Sakkinen wrote:
> > On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> > > From: Stefan Berger <stefanb@linux.ibm.com>
> > >
> > > In preparation for support of NIST P521, adjust the basic tests on the
> > > length of the provided key parameters to only ensure that the length of the
> > > x plus y coordinates parameter array is not an odd number and that each
> > > coordinate fits into an array of 'ndigits' digits. Mathematical tests on
> > > the key's parameters are then done in ecc_is_pubkey_valid_full rejecting
> > > invalid keys.
> > >
> > > The change is necessary since NIST P521 keys do not have keys with
> > > coordinates that each fully require 'full' digits (= u64), unlike
> > > NIST P192/256/384 that all require multiple 'full' digits.
> > 
> > This sentence is not really comprehendable English sentence. Can you
> > just write the rationale in understandable form?
> > 
> > "fully require full digits (= u64)" is something totally alien to me
> > tbh.
>
> It is proper English, but requires an understanding of how large integers
> are handled by crypto/ecdsa.c:  They're a sequence of u64.  For P192, P256
> and P384 all u64 in the sequence are used to their full extent because the
> key size is divisable by 64.  That's not the case for P521, where the most
> significant u64 is not fully used (only 2 out of 8 bytes are used).

This would be a great extension to the current commit message.

My point here is that:

1. I obviously acknowledge that not all math etc. related to a crypto
   standard can be explained in a commit message.
2. That said, they should be more verbose than usualy commit messages
   to be as easy to follow as possible, given the complexity of topic.

Here the topic is fairly complex but most of commit messages are written
without much focus on the background story. In this type of patch set
even having some redundancy in the commit messages is favorable so that
they are as easy to understand as possible.

Actually just as code changes they are quite simple but why they are
made is the complex topic, which means that commit messages are even
more important. This motivation comes from e.g. when these need to be
backtracked at some point when bisecting a bug and whatnot.

> Thanks,
>
> Lukas


BR, Jarkko

Re: [PATCH v6 12/13] crypto: asymmetric_keys - Adjust signature size calculation for NIST P521

[Stefan Berger]

On 3/18/24 17:12, Jarkko Sakkinen wrote:
> On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Adjust the calculation of the maximum signature size for support of
>> NIST P521. While existing curves may prepend a 0 byte to their coordinates
>> (to make the number positive), NIST P521 will not do this since only the
>> first bit in the most significant byte is used.
>>
>> If the encoding of the x & y coordinates requires at least 128 bytes then
>> an additional byte is needed for the encoding of the length. Take this into
>> account when calculating the maximum signature size.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> Reviewed-by: Lukas Wunner <lukas@wunner.de>
>> Tested-by: Lukas Wunner <lukas@wunner.de>
>> ---
>>   crypto/asymmetric_keys/public_key.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
>> index e5f22691febd..16cc0be28929 100644
>> --- a/crypto/asymmetric_keys/public_key.c
>> +++ b/crypto/asymmetric_keys/public_key.c
>> @@ -233,6 +233,7 @@ static int software_key_query(const struct kernel_pkey_params *params,
>>   	info->key_size = len * 8;
>>   
>>   	if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
>> +		int slen = len;
>>   		/*
>>   		 * ECDSA key sizes are much smaller than RSA, and thus could
>>   		 * operate on (hashed) inputs that are larger than key size.
>> @@ -246,8 +247,19 @@ static int software_key_query(const struct kernel_pkey_params *params,
>>   		 * Verify takes ECDSA-Sig (described in RFC 5480) as input,
>>   		 * which is actually 2 'key_size'-bit integers encoded in
>>   		 * ASN.1.  Account for the ASN.1 encoding overhead here.
>> +		 *
>> +		 * NIST P192/256/384 may prepend a '0' to a coordinate to
>> +		 * indicate a positive integer. NIST P521 never needs it.
>>   		 */
>> -		info->max_sig_size = 2 * (len + 3) + 2;
>> +		if (strcmp(pkey->pkey_algo, "ecdsa-nist-p521") != 0)
>> +			slen += 1;
> 
> Just wondering the logic of picking between these:
> 
> 1. "strncmp"
> 2. "strcmp"
> 

strncmp: prefix-matching
strcmp: full string matching

> Now the "ecdsa" is matched with strncmp and "ecdsa-nist-p521" is
> compared with strcmp.

That's prefix matching vs. full string match.

.. and indeed 'ecdsa' is a prefix of 'ecdsa-nist-p521'.

> 
> So is there a good reason to use different function in these
> cases?

Yes, there is.

> 
> I'd guess both could be using strcmp since comparing against
> constant...

No, prefix versus full string matching requires different function calls.

> 
>> +		/* Length of encoding the x & y coordinates */
>> +		slen = 2 * (slen + 2);
>> +		/*
>> +		 * If coordinate encoding takes at least 128 bytes then an
>> +		 * additional byte for length encoding is needed.
>> +		 */
>> +		info->max_sig_size = 1 + (slen >= 128) + 1 + slen;
>>   	} else {
>>   		info->max_data_size = len;
>>   		info->max_sig_size = len;
> 
> 
> BR, Jarkko
> 

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Stefan Berger]

On 3/18/24 14:48, Lukas Wunner wrote:
> On Tue, Mar 12, 2024 at 02:36:05PM -0400, Stefan Berger wrote:
>> This series adds support for the NIST P521 curve to the ecdsa module
>> to enable signature verification with it.
> 
> v6 of this series is still
> 
> Tested-by: Lukas Wunner <lukas@wunner.de>

Thanks.

> 
> as it successfully authenticates PCI devices with the
> "TPM_ALG_ECDSA_ECC_NIST_P521" SPDM asymmetric algorithm
> using my development branch:
> 
> https://github.com/l1k/linux/commits/doe
> 
> Thanks,
> 
> Lukas
> 

Re: [PATCH v6 08/13] crypto: ecc - Add NIST P521 curve parameters

[Stefan Berger]

On 3/18/24 17:05, Jarkko Sakkinen wrote:
> On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Add the parameters for the NIST P521 curve and define a new curve ID
>> for it. Make the curve available in ecc_get_curve.
> 
> This is rare example of "complete story" in this series despite being
> short, so no complains :-)

Wew :-) Thanks for the reviews.

> 
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> Tested-by: Lukas Wunner <lukas@wunner.de>
>> ---
>>   crypto/ecc.c            |  2 ++
>>   crypto/ecc_curve_defs.h | 45 +++++++++++++++++++++++++++++++++++++++++
>>   include/crypto/ecdh.h   |  1 +
>>   3 files changed, 48 insertions(+)
>>
>> diff --git a/crypto/ecc.c b/crypto/ecc.c
>> index ead40b5ebb46..4f6fa8617308 100644
>> --- a/crypto/ecc.c
>> +++ b/crypto/ecc.c
>> @@ -60,6 +60,8 @@ const struct ecc_curve *ecc_get_curve(unsigned int curve_id)
>>   		return &nist_p256;
>>   	case ECC_CURVE_NIST_P384:
>>   		return &nist_p384;
>> +	case ECC_CURVE_NIST_P521:
>> +		return &nist_p521;
>>   	default:
>>   		return NULL;
>>   	}
>> diff --git a/crypto/ecc_curve_defs.h b/crypto/ecc_curve_defs.h
>> index ab1ef3d94be5..0ecade7d02f5 100644
>> --- a/crypto/ecc_curve_defs.h
>> +++ b/crypto/ecc_curve_defs.h
>> @@ -89,6 +89,51 @@ static struct ecc_curve nist_p384 = {
>>   	.b = nist_p384_b
>>   };
>>   
>> +/* NIST P-521 */
>> +static u64 nist_p521_g_x[] = { 0xf97e7e31c2e5bd66ull, 0x3348b3c1856a429bull,
>> +				0xfe1dc127a2ffa8deull, 0xa14b5e77efe75928ull,
>> +				0xf828af606b4d3dbaull, 0x9c648139053fb521ull,
>> +				0x9e3ecb662395b442ull, 0x858e06b70404e9cdull,
>> +				0xc6ull };
>> +static u64 nist_p521_g_y[] = { 0x88be94769fd16650ull, 0x353c7086a272c240ull,
>> +				0xc550b9013fad0761ull, 0x97ee72995ef42640ull,
>> +				0x17afbd17273e662cull, 0x98f54449579b4468ull,
>> +				0x5c8a5fb42c7d1bd9ull, 0x39296a789a3bc004ull,
>> +				0x118ull };
>> +static u64 nist_p521_p[] = { 0xffffffffffffffffull, 0xffffffffffffffffull,
>> +				0xffffffffffffffffull, 0xffffffffffffffffull,
>> +				0xffffffffffffffffull, 0xffffffffffffffffull,
>> +				0xffffffffffffffffull, 0xffffffffffffffffull,
>> +				0x1ffull };
>> +static u64 nist_p521_n[] = { 0xbb6fb71e91386409ull, 0x3bb5c9b8899c47aeull,
>> +				0x7fcc0148f709a5d0ull, 0x51868783bf2f966bull,
>> +				0xfffffffffffffffaull, 0xffffffffffffffffull,
>> +				0xffffffffffffffffull, 0xffffffffffffffffull,
>> +				0x1ffull };
>> +static u64 nist_p521_a[] = { 0xfffffffffffffffcull, 0xffffffffffffffffull,
>> +				0xffffffffffffffffull, 0xffffffffffffffffull,
>> +				0xffffffffffffffffull, 0xffffffffffffffffull,
>> +				0xffffffffffffffffull, 0xffffffffffffffffull,
>> +				0x1ffull };
>> +static u64 nist_p521_b[] = { 0xef451fd46b503f00ull, 0x3573df883d2c34f1ull,
>> +				0x1652c0bd3bb1bf07ull, 0x56193951ec7e937bull,
>> +				0xb8b489918ef109e1ull, 0xa2da725b99b315f3ull,
>> +				0x929a21a0b68540eeull, 0x953eb9618e1c9a1full,
>> +				0x051ull };
>> +static struct ecc_curve nist_p521 = {
>> +	.name = "nist_521",
>> +	.nbits = 521,
>> +	.g = {
>> +		.x = nist_p521_g_x,
>> +		.y = nist_p521_g_y,
>> +		.ndigits = 9,
>> +	},
>> +	.p = nist_p521_p,
>> +	.n = nist_p521_n,
>> +	.a = nist_p521_a,
>> +	.b = nist_p521_b
>> +};
>> +
>>   /* curve25519 */
>>   static u64 curve25519_g_x[] = { 0x0000000000000009, 0x0000000000000000,
>>   				0x0000000000000000, 0x0000000000000000 };
>> diff --git a/include/crypto/ecdh.h b/include/crypto/ecdh.h
>> index a9f98078d29c..9784ecdd2fb4 100644
>> --- a/include/crypto/ecdh.h
>> +++ b/include/crypto/ecdh.h
>> @@ -26,6 +26,7 @@
>>   #define ECC_CURVE_NIST_P192	0x0001
>>   #define ECC_CURVE_NIST_P256	0x0002
>>   #define ECC_CURVE_NIST_P384	0x0003
>> +#define ECC_CURVE_NIST_P521	0x0004
>>   
>>   /**
>>    * struct ecdh - define an ECDH private key
> 
> 
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> BR, Jarkko
> 

RE: [PATCH v6 12/13] crypto: asymmetric_keys - Adjust signature size calculation for NIST P521

[Bharat Bhushan]

> -----Original Message-----
> From: Lukas Wunner <lukas@wunner.de>
> Sent: Monday, March 18, 2024 12:36 PM
> To: Bharat Bhushan <bbhushan2@marvell.com>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>; keyrings@vger.kernel.org;
> linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au;
> davem@davemloft.net; linux-kernel@vger.kernel.org;
> saulo.alessandre@tse.jus.br; jarkko@kernel.org; Stefan Berger
> <stefanb@linux.ibm.com>
> Subject: Re: [EXTERNAL] [PATCH v6 12/13] crypto: asymmetric_keys - Adjust
> signature size calculation for NIST P521
> 
> On Mon, Mar 18, 2024 at 05:58:23AM +0000, Bharat Bhushan wrote:
> > > --- a/crypto/asymmetric_keys/public_key.c
> > > +++ b/crypto/asymmetric_keys/public_key.c
> > > @@ -233,6 +233,7 @@ static int software_key_query(const struct
> > > kernel_pkey_params *params,
> > >  	info->key_size = len * 8;
> > >
> > >  	if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
> > > +		int slen = len;
> > >  		/*
> > >  		 * ECDSA key sizes are much smaller than RSA, and thus could
> > >  		 * operate on (hashed) inputs that are larger than key size.
> > > @@ -246,8 +247,19 @@ static int software_key_query(const struct
> > > kernel_pkey_params *params,
> > >  		 * Verify takes ECDSA-Sig (described in RFC 5480) as input,
> > >  		 * which is actually 2 'key_size'-bit integers encoded in
> > >  		 * ASN.1.  Account for the ASN.1 encoding overhead here.
> > > +		 *
> > > +		 * NIST P192/256/384 may prepend a '0' to a coordinate to
> > > +		 * indicate a positive integer. NIST P521 never needs it.
> > >  		 */
> > > -		info->max_sig_size = 2 * (len + 3) + 2;
> > > +		if (strcmp(pkey->pkey_algo, "ecdsa-nist-p521") != 0)
> > > +			slen += 1;
> > > +		/* Length of encoding the x & y coordinates */
> > > +		slen = 2 * (slen + 2);
> > > +		/*
> > > +		 * If coordinate encoding takes at least 128 bytes then an
> > > +		 * additional byte for length encoding is needed.
> > > +		 */
> > > +		info->max_sig_size = 1 + (slen >= 128) + 1 + slen;
> >
> > Is "(slen >= 128)" valid for P192/256/384 also?
> 
> It is valid but never true for those.

Okay, just want to check if that was valid for P192/256/384 and this patch is fixing same as well.

Otherwise looks good to me as well.

Thanks
-Bharat
 
> 
> The signature consists of two integers encoded in ASN.1.
> So each integer is prepended by 1 byte for the tag and 1 byte for the length.
> 
> The two integers are bundled together in a "sequence", which in turn requires
> 1 byte for the tag and 1 byte for the length.  However, for P521 the length of
> the sequence is at least 2*(1+1+66) = 136 bytes, which exceeds 128 bytes and
> therefore the length of the sequence occupies 2 bytes instead of 1.
> 
> For the shorter key lengths, the sequence fits in less than 128 bytes and does
> not require the extra byte for the sequence length.
> 
> So the code is fine AFAICS.
> 
> Thanks,
> 
> Lukas

Re: [PATCH v6 06/13] crypto: ecc - Implement vli_mmod_fast_521 for NIST p521

[Bharat Bhushan]

> -----Original Message-----
> From: Stefan Berger <stefanb@linux.ibm.com>
> Sent: Tuesday, March 19, 2024 12:09 AM
> To: Bharat Bhushan <bbhushan2@marvell.com>; Stefan Berger
> <stefanb@linux.vnet.ibm.com>; keyrings@vger.kernel.org; linux-
> crypto@vger.kernel.org; herbert@gondor.apana.org.au;
> davem@davemloft.net
> Cc: linux-kernel@vger.kernel.org; saulo.alessandre@tse.jus.br;
> lukas@wunner.de; jarkko@kernel.org
> Subject: [EXTERNAL] Re: [PATCH v6 06/13] crypto: ecc - Implement
> vli_mmod_fast_521 for NIST p521
> 
> ----------------------------------------------------------------------
> 
> 
> On 3/18/24 01:47, Bharat Bhushan wrote:
> >
> >
> >> -----Original Message-----
> >> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
> >> Sent: Wednesday, March 13, 2024 12:06 AM
> >> To: keyrings@vger.kernel.org; linux-crypto@vger.kernel.org;
> >> herbert@gondor.apana.org.au; davem@davemloft.net
> >> Cc: linux-kernel@vger.kernel.org; saulo.alessandre@tse.jus.br;
> >> lukas@wunner.de; Bharat Bhushan <bbhushan2@marvell.com>;
> >> jarkko@kernel.org; Stefan Berger <stefanb@linux.ibm.com>
> >> Subject: [EXTERNAL] [PATCH v6 06/13] crypto: ecc - Implement
> >> vli_mmod_fast_521 for NIST p521
> >>
> >> ---------------------------------------------------------------------
> >> -
> >> From: Stefan Berger <stefanb@linux.ibm.com>
> >>
> >> Implement vli_mmod_fast_521 following the description for how to
> >> calculate the modulus for NIST P521 in the NIST publication
> >> "Recommendations for Discrete Logarithm-Based Cryptography: Elliptic
> Curve Domain Parameters"
> >> section G.1.4.
> >>
> >> NIST p521 requires 9 64bit digits, so increase the ECC_MAX_DIGITS so
> >> that arrays fit the larger numbers.
> >>
> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> >> ---
> >>   crypto/ecc.c                  | 25 +++++++++++++++++++++++++
> >>   include/crypto/internal/ecc.h |  3 ++-
> >>   2 files changed, 27 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/crypto/ecc.c b/crypto/ecc.c index
> >> 415a2f4e7291..99d41887c005
> >> 100644
> >> --- a/crypto/ecc.c
> >> +++ b/crypto/ecc.c
> >> @@ -902,6 +902,28 @@ static void vli_mmod_fast_384(u64 *result, const
> >> u64 *product,  #undef AND64H  #undef AND64L
> >>
> >> +/*
> >> + * Computes result = product % curve_prime
> >> + * from "Recommendations for Discrete Logarithm-Based Cryptography:
> >> + *       Elliptic Curve Domain Parameters" section G.1.4
> >> + */
> >> +static void vli_mmod_fast_521(u64 *result, const u64 *product,
> >> +			      const u64 *curve_prime, u64 *tmp) {
> >> +	const unsigned int ndigits = ECC_CURVE_NIST_P521_DIGITS;
> >> +	size_t i;
> >> +
> >> +	/* Initialize result with lowest 521 bits from product */
> >> +	vli_set(result, product, ndigits);
> >> +	result[8] &= 0x1ff;
> >> +
> >> +	for (i = 0; i < ndigits; i++)
> >> +		tmp[i] = (product[8 + i] >> 9) | (product[9 + i] << 55);
> >> +	tmp[8] &= 0x1ff;
> >
> > Can we get away from this hardcoding, like 9, 55, 0x1ff etc.
> > Or at least add comment about these.
> >
> >> +
> >> +	vli_mod_add(result, result, tmp, curve_prime, ndigits); }
> >> +
> >>   /* Computes result = product % curve_prime for different curve_primes.
> >>    *
> >>    * Note that curve_primes are distinguished just by heuristic check
> >> and @@ -
> >> 941,6 +963,9 @@ static bool vli_mmod_fast(u64 *result, u64 *product,
> >>   	case ECC_CURVE_NIST_P384_DIGITS:
> >>   		vli_mmod_fast_384(result, product, curve_prime, tmp);
> >>   		break;
> >> +	case ECC_CURVE_NIST_P521_DIGITS:
> >> +		vli_mmod_fast_521(result, product, curve_prime, tmp);
> >> +		break;
> >>   	default:
> >>   		pr_err_ratelimited("ecc: unsupported digits size!\n");
> >>   		return false;
> >> diff --git a/include/crypto/internal/ecc.h
> >> b/include/crypto/internal/ecc.h index
> >> ab722a8986b7..4e2f5f938e91 100644
> >> --- a/include/crypto/internal/ecc.h
> >> +++ b/include/crypto/internal/ecc.h
> >> @@ -33,7 +33,8 @@
> >>   #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              (512 / 64) /* due to ecrdsa */
> >> +#define ECC_CURVE_NIST_P521_DIGITS  9
> >
> > Maybe these can be defined as:
> > #define ECC_CURVE_NIST_P521_DIGITS  (DIV_ROUND_UP(521, 64) /* NIST
> > P521 */)
> 
> I think for NIST P521 9 can be pre-calculated. It will not change anymore in the
> future.

pre-calculation for others is perfect division by 64 but not for P521. So that creates a little confusion why it is 9 and not 8.
So in my view we can either use same logic used for ECC_MAX_DIGITS or a comment that it is rounded up. Anyways not a major concern.

Thanks
-Bharat

> 
> >
> >> +#define ECC_MAX_DIGITS              DIV_ROUND_UP(521, 64) /* NIST P521
> */
> >
> > /* NIST_P521 is max digits */
> > #define ECC_MAX_DIGITS              ECC_CURVE_ _DIGITS
> 
> In this case I think the DIV_ROUND_UP() along with the comment shows that
> it needs to be updated if ever a larger curve comes along.
> 
> >
> > Thanks
> > -Bharat
> >
> >>
> >>   #define ECC_DIGITS_TO_BYTES_SHIFT 3
> >>
> >> --
> >> 2.43.0
> >

Re: [PATCH v6 12/13] crypto: asymmetric_keys - Adjust signature size calculation for NIST P521

[Jarkko Sakkinen]

On Tue Mar 19, 2024 at 12:42 AM EET, Stefan Berger wrote:
>
>
> On 3/18/24 17:12, Jarkko Sakkinen wrote:
> > On Tue Mar 12, 2024 at 8:36 PM EET, Stefan Berger wrote:
> >> From: Stefan Berger <stefanb@linux.ibm.com>
> >>
> >> Adjust the calculation of the maximum signature size for support of
> >> NIST P521. While existing curves may prepend a 0 byte to their coordinates
> >> (to make the number positive), NIST P521 will not do this since only the
> >> first bit in the most significant byte is used.
> >>
> >> If the encoding of the x & y coordinates requires at least 128 bytes then
> >> an additional byte is needed for the encoding of the length. Take this into
> >> account when calculating the maximum signature size.
> >>
> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> >> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> >> Tested-by: Lukas Wunner <lukas@wunner.de>
> >> ---
> >>   crypto/asymmetric_keys/public_key.c | 14 +++++++++++++-
> >>   1 file changed, 13 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
> >> index e5f22691febd..16cc0be28929 100644
> >> --- a/crypto/asymmetric_keys/public_key.c
> >> +++ b/crypto/asymmetric_keys/public_key.c
> >> @@ -233,6 +233,7 @@ static int software_key_query(const struct kernel_pkey_params *params,
> >>   	info->key_size = len * 8;
> >>   
> >>   	if (strncmp(pkey->pkey_algo, "ecdsa", 5) == 0) {
> >> +		int slen = len;
> >>   		/*
> >>   		 * ECDSA key sizes are much smaller than RSA, and thus could
> >>   		 * operate on (hashed) inputs that are larger than key size.
> >> @@ -246,8 +247,19 @@ static int software_key_query(const struct kernel_pkey_params *params,
> >>   		 * Verify takes ECDSA-Sig (described in RFC 5480) as input,
> >>   		 * which is actually 2 'key_size'-bit integers encoded in
> >>   		 * ASN.1.  Account for the ASN.1 encoding overhead here.
> >> +		 *
> >> +		 * NIST P192/256/384 may prepend a '0' to a coordinate to
> >> +		 * indicate a positive integer. NIST P521 never needs it.
> >>   		 */
> >> -		info->max_sig_size = 2 * (len + 3) + 2;
> >> +		if (strcmp(pkey->pkey_algo, "ecdsa-nist-p521") != 0)
> >> +			slen += 1;
> > 
> > Just wondering the logic of picking between these:
> > 
> > 1. "strncmp"
> > 2. "strcmp"
> > 
>
> strncmp: prefix-matching
> strcmp: full string matching

Right, in first case is necessary because strcmp() would return "-1" for
the substring.

BR, Jarkko

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Jarkko Sakkinen]

On Tue Mar 19, 2024 at 12:42 AM EET, Stefan Berger wrote:
>
>
> On 3/18/24 14:48, Lukas Wunner wrote:
> > On Tue, Mar 12, 2024 at 02:36:05PM -0400, Stefan Berger wrote:
> >> This series adds support for the NIST P521 curve to the ecdsa module
> >> to enable signature verification with it.
> > 
> > v6 of this series is still
> > 
> > Tested-by: Lukas Wunner <lukas@wunner.de>
>
> Thanks.

This has been discussed before in LKML but generally tested-by for
series does not have semantical meaning.

Please apply only for patches that were tested.

BR, Jarkko

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Jarkko Sakkinen]

On Tue Mar 19, 2024 at 8:22 PM EET, Jarkko Sakkinen wrote:
> On Tue Mar 19, 2024 at 12:42 AM EET, Stefan Berger wrote:
> >
> >
> > On 3/18/24 14:48, Lukas Wunner wrote:
> > > On Tue, Mar 12, 2024 at 02:36:05PM -0400, Stefan Berger wrote:
> > >> This series adds support for the NIST P521 curve to the ecdsa module
> > >> to enable signature verification with it.
> > > 
> > > v6 of this series is still
> > > 
> > > Tested-by: Lukas Wunner <lukas@wunner.de>
> >
> > Thanks.
>
> This has been discussed before in LKML but generally tested-by for
> series does not have semantical meaning.
>
> Please apply only for patches that were tested.

How to implement this in practice or place tested-by correctly?

I'd place the tested-by to the patch which contains the uapi trigger
to which for testing was done.

By having this granularity that also does help fixing bugs later on
so it is really not just "plain bureacracy".

BR, Jarkko

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Stefan Berger]

On 3/19/24 14:22, Jarkko Sakkinen wrote:
> On Tue Mar 19, 2024 at 12:42 AM EET, Stefan Berger wrote:
>>
>>
>> On 3/18/24 14:48, Lukas Wunner wrote:
>>> On Tue, Mar 12, 2024 at 02:36:05PM -0400, Stefan Berger wrote:
>>>> This series adds support for the NIST P521 curve to the ecdsa module
>>>> to enable signature verification with it.
>>>
>>> v6 of this series is still
>>>
>>> Tested-by: Lukas Wunner <lukas@wunner.de>
>>
>> Thanks.
> 
> This has been discussed before in LKML but generally tested-by for
> series does not have semantical meaning.
> 
> Please apply only for patches that were tested.

Ok, I will remove the Tested-by tag.

However, patch 4/13, that only changes a comment, can also be tested in 
so far as to check whether the code is correct as-is for the tests that 
'I' ran and no further modifications are needed for NIST P521. In this 
case it would mean that a single subtraction of 'n' from res.x seems 
sufficient and existing code is good as described by the modified comment.

> 
> BR, Jarkko
> 

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Jarkko Sakkinen]

On Tue Mar 19, 2024 at 8:55 PM EET, Stefan Berger wrote:
>
>
> On 3/19/24 14:22, Jarkko Sakkinen wrote:
> > On Tue Mar 19, 2024 at 12:42 AM EET, Stefan Berger wrote:
> >>
> >>
> >> On 3/18/24 14:48, Lukas Wunner wrote:
> >>> On Tue, Mar 12, 2024 at 02:36:05PM -0400, Stefan Berger wrote:
> >>>> This series adds support for the NIST P521 curve to the ecdsa module
> >>>> to enable signature verification with it.
> >>>
> >>> v6 of this series is still
> >>>
> >>> Tested-by: Lukas Wunner <lukas@wunner.de>
> >>
> >> Thanks.
> > 
> > This has been discussed before in LKML but generally tested-by for
> > series does not have semantical meaning.
> > 
> > Please apply only for patches that were tested.
>
> Ok, I will remove the Tested-by tag.
>
> However, patch 4/13, that only changes a comment, can also be tested in 
> so far as to check whether the code is correct as-is for the tests that 
> 'I' ran and no further modifications are needed for NIST P521. In this 
> case it would mean that a single subtraction of 'n' from res.x seems 
> sufficient and existing code is good as described by the modified comment.

So, since all patches are required to test anything at all, I think that
putting tested-by to 13/13 would be the most appropriate, right?

I without enabling this x509 parser, there is nothing to test, I'd
presume.

It doesn't have to be more complicated than this.

BR, Jarkko

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Lukas Wunner]

On Tue, Mar 19, 2024 at 08:22:51PM +0200, Jarkko Sakkinen wrote:
> On Tue Mar 19, 2024 at 12:42 AM EET, Stefan Berger wrote:
> > On 3/18/24 14:48, Lukas Wunner wrote:
> > > On Tue, Mar 12, 2024 at 02:36:05PM -0400, Stefan Berger wrote:
> > >> This series adds support for the NIST P521 curve to the ecdsa module
> > >> to enable signature verification with it.
> > > 
> > > v6 of this series is still
> > > 
> > > Tested-by: Lukas Wunner <lukas@wunner.de>
> >
> > Thanks.
> 
> This has been discussed before in LKML but generally tested-by for
> series does not have semantical meaning.

I believe that notion is outdated.

It seems to be becoming the norm that maintainers apply patches with
"b4 am --apply-cover-trailers", which automatically picks up Acked-by,
Reviewed-by, Tested-by and other tags sent in-reply-to the cover letter
and adds them to all patches in the series.

Consequently, providing such tags in-reply-to the cover letter is not
unusual and nothing to object to.

If Herbert applies patches with "b4 am --apply-cover-trailers" or
"b4 shazam --apply-cover-trailers" (I don't know if he does),
it is completely irrelevant whether Stefan strips my Tested-by from
individual patches:  It will automatically be re-added when the
series gets applied.

I have only tested the collection of *all* patches in this series and
can thus only vouch for correct functioning of the *entire* series,
hence providing the Tested-by in-reply-to the cover letter is the only
thing that makes sense to me.

Either way, I don't think arguing over which patch to apply a Tested-by
to is a productive use of everyone's time.

Thanks,

Lukas

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Konstantin Ryabitsev]

On Wed, Mar 20, 2024 at 06:40:33AM +0100, Lukas Wunner wrote:
> If Herbert applies patches with "b4 am --apply-cover-trailers" or
> "b4 shazam --apply-cover-trailers" (I don't know if he does),
> it is completely irrelevant whether Stefan strips my Tested-by from
> individual patches:  It will automatically be re-added when the
> series gets applied.

Applying trailers sent to the cover letter is now the default behaviour in
0.13, so this flag is no longer required (it does nothing).

-K

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Jarkko Sakkinen]

On Wed Mar 20, 2024 at 4:41 PM EET, Konstantin Ryabitsev wrote:
> On Wed, Mar 20, 2024 at 06:40:33AM +0100, Lukas Wunner wrote:
> > If Herbert applies patches with "b4 am --apply-cover-trailers" or
> > "b4 shazam --apply-cover-trailers" (I don't know if he does),
> > it is completely irrelevant whether Stefan strips my Tested-by from
> > individual patches:  It will automatically be re-added when the
> > series gets applied.
>
> Applying trailers sent to the cover letter is now the default behaviour in
> 0.13, so this flag is no longer required (it does nothing).
>
> -K

The whole policy of how to put tested-by in my experience is subsystem
dependent.

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

Official documentation only speaks about patches so perhaps it should
then be refined for the series.

I'm hearing about this option in b4 for the first time in my life.

BR, Jarkko

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Jarkko Sakkinen]

On Thu Mar 21, 2024 at 6:17 PM EET, Jarkko Sakkinen wrote:
> On Wed Mar 20, 2024 at 4:41 PM EET, Konstantin Ryabitsev wrote:
> > On Wed, Mar 20, 2024 at 06:40:33AM +0100, Lukas Wunner wrote:
> > > If Herbert applies patches with "b4 am --apply-cover-trailers" or
> > > "b4 shazam --apply-cover-trailers" (I don't know if he does),
> > > it is completely irrelevant whether Stefan strips my Tested-by from
> > > individual patches:  It will automatically be re-added when the
> > > series gets applied.
> >
> > Applying trailers sent to the cover letter is now the default behaviour in
> > 0.13, so this flag is no longer required (it does nothing).
> >
> > -K
>
> The whole policy of how to put tested-by in my experience is subsystem
> dependent.
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> Official documentation only speaks about patches so perhaps it should
> then be refined for the series.
>
> I'm hearing about this option in b4 for the first time in my life.

It is also pretty relevant to know when you read the commit log e.g.
when bisecting what was *actually* tested.

If you put tested-by to whole series it probably means that you've
tested the uapi and are getting the expected results. Thus in this
case it would 13/13 that is *actually* tested.

Putting tested-by to every possible patch only degrades the quality
of the commit log.

I don't see how this is "irrelevant".

BR, Jarkko

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Stefan Berger]

On 3/21/24 12:19, Jarkko Sakkinen wrote:
> On Thu Mar 21, 2024 at 6:17 PM EET, Jarkko Sakkinen wrote:
>> On Wed Mar 20, 2024 at 4:41 PM EET, Konstantin Ryabitsev wrote:
>>> On Wed, Mar 20, 2024 at 06:40:33AM +0100, Lukas Wunner wrote:
>>>> If Herbert applies patches with "b4 am --apply-cover-trailers" or
>>>> "b4 shazam --apply-cover-trailers" (I don't know if he does),
>>>> it is completely irrelevant whether Stefan strips my Tested-by from
>>>> individual patches:  It will automatically be re-added when the
>>>> series gets applied.
>>>
>>> Applying trailers sent to the cover letter is now the default behaviour in
>>> 0.13, so this flag is no longer required (it does nothing).
>>>
>>> -K
>>
>> The whole policy of how to put tested-by in my experience is subsystem
>> dependent.
>>
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>>
>> Official documentation only speaks about patches so perhaps it should
>> then be refined for the series.
>>
>> I'm hearing about this option in b4 for the first time in my life.
> 
> It is also pretty relevant to know when you read the commit log e.g.
> when bisecting what was *actually* tested.
> 
> If you put tested-by to whole series it probably means that you've
> tested the uapi and are getting the expected results. Thus in this
> case it would 13/13 that is *actually* tested.

Btw, we have 2 entry points into the code and those are uapi and testmgr.

So if I was to exercise the uapi with a NIST P521 key then are you 
saying that none of the other code was exercised and therefore wasn't 
tested? How would YOU suggest to test individual patches then?

What the docs at the link above say is this:

A Tested-by: tag indicates that the patch has been successfully tested 
(in some environment) by the person named. This tag informs maintainers 
that some testing has been performed, provides a means to locate testers 
for future patches, and ensures credit for the testers.

Note: 'some testing' 'in some environment'. We probably can reasonably 
assume that not only 13/13 is necessary but also several of the other 
patches are necessary to support this new curve and were exercised with 
either UAPI and probably also testmgr.

> 
> Putting tested-by to every possible patch only degrades the quality
> of the commit log.

I would still be interested how one would test individual patches in a 
series so they are worthy of a Tested-by tag.

> 
> I don't see how this is "irrelevant".
> 
> BR, Jarkko

Re: [PATCH v6 00/13] Add support for NIST P521 to ecdsa

[Jarkko Sakkinen]

On Thu Mar 21, 2024 at 6:36 PM EET, Stefan Berger wrote:
> > 
> > Putting tested-by to every possible patch only degrades the quality
> > of the commit log.
>
> I would still be interested how one would test individual patches in a 
> series so they are worthy of a Tested-by tag.

I've at least said this twice in this thread.

I.e. in a feature you most likely test the uapi so 13/13.

In a bug fix you test kernel with and without the patch. Generally you
test stuff that you can observe.

You can also test non-uapi behaviour with e.g. kprobes or measure
e.g. performance, depending on patch.

BR, Jarkko