ecdsa KEYCTL_PKEY_QUERY fixes

The callers of crypto_sig_sign() assume that the signature size is always equivalent to the key size.

This happens to be true for RSA, which is currently the only algorithm implementing the ->sign() callback. But it is false e.g. for X9.62 encoded ECDSA signatures because they have variable length.

Prepare for addition of a ->sign() callback to such algorithms by letting the callback return the signature size (or a negative integer on error). When testing the ->sign() callback in test_sig_one(), use crypto_sig_maxsize() instead of crypto_sig_keysize() to verify that the test vector's signature does not exceed an algorithm's maximum signature size.

There has been a relatively recent effort to upstream ECDSA signature generation support which may benefit from this change:

https://lore.kernel.org/linux-crypto/[email protected]/

However the main motivation for this commit is to reduce the number of crypto_sig_keysize() callers: This function is about to be changed to return the size in bits instead of bytes and that will require amending most callers to divide the return value by 8.

Signed-off-by Lukas Wunner <[email protected]>

Cc Ignat Korchagin <[email protected]>

---
  crypto/asymmetric_keys/public_key.c | 9 ++-------
  crypto/rsassa-pkcs1.c               | 2 +-
  crypto/testmgr.c                    | 7 ++++---
  include/crypto/sig.h                | 5 +++--
  4 files changed, 10 insertions(+), 13 deletions(-)

crypto/asymmetric_keys/public_key.c +2 -7

index bbd07a9022e6..bf165d321440 100644
@@ -267,7 +267,6 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
  	struct crypto_sig *sig;
  	char *key, *ptr;
  	bool issig;
-	int ksz;
  	int ret;
 
  	pr_devel("==>%s()\n", __func__);

@@ -302,8 +301,6 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
  			ret = crypto_sig_set_pubkey(sig, key, pkey->keylen);
  		if (ret)
  			goto error_free_tfm;
-
-		ksz = crypto_sig_keysize(sig);
  	} else {
  		tfm = crypto_alloc_akcipher(alg_name, 0, 0);
  		if (IS_ERR(tfm)) {

@@ -317,8 +314,6 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
  			ret = crypto_akcipher_set_pub_key(tfm, key, pkey->keylen);
  		if (ret)
  			goto error_free_tfm;
-
-		ksz = crypto_akcipher_maxsize(tfm);
  	}
 
  	ret = -EINVAL;

@@ -347,8 +342,8 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
  		BUG();
  	}
 
-	if (ret == 0)
-		ret = ksz;
+	if (!issig && ret == 0)
+		ret = crypto_akcipher_maxsize(tfm);
 
  error_free_tfm:
  	if (issig)

crypto/rsassa-pkcs1.c +1 -1

index f68ffd338f48..d01ac75635e0 100644
@@ -210,7 +210,7 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm,
  		memset(dst, 0, pad_len);
  	}
 
-	return 0;
+	return ctx->key_size;
  }
 
  static int rsassa_pkcs1_verify(struct crypto_sig *tfm,

crypto/testmgr.c +4 -3

index 1f5f48ab18c7..76c013bcebe5 100644
@@ -4339,7 +4339,7 @@ static int test_sig_one(struct crypto_sig *tfm, const struct sig_testvec *vecs)
  	if (vecs->public_key_vec)
  		return 0;
 
-	sig_size = crypto_sig_keysize(tfm);
+	sig_size = crypto_sig_maxsize(tfm);
  	if (sig_size < vecs->c_size) {
  		pr_err("alg: sig: invalid maxsize %u\n", sig_size);
  		return -EINVAL;

@@ -4351,13 +4351,14 @@ static int test_sig_one(struct crypto_sig *tfm, const struct sig_testvec *vecs)
 
  	/* Run asymmetric signature generation */
  	err = crypto_sig_sign(tfm, vecs->m, vecs->m_size, sig, sig_size);
-	if (err) {
+	if (err < 0) {
  		pr_err("alg: sig: sign test failed: err %d\n", err);
  		return err;
  	}
 
  	/* Verify that generated signature equals cooked signature */
-	if (memcmp(sig, vecs->c, vecs->c_size) ||
+	if (err != vecs->c_size ||
+	    memcmp(sig, vecs->c, vecs->c_size) ||
  	    memchr_inv(sig + vecs->c_size, 0, sig_size - vecs->c_size)) {
  		pr_err("alg: sig: sign test failed: invalid output\n");
  		hexdump(sig, sig_size);

include/crypto/sig.h +2 -1

index cff41ad93824..11024708c069 100644
@@ -23,7 +23,8 @@ struct crypto_sig {
   * struct sig_alg - generic public key signature algorithm
   *
   * @sign:	Function performs a sign operation as defined by public key
- *		algorithm. Optional.
+ *		algorithm. On success, the signature size is returned.
+ *		Optional.
   * @verify:	Function performs a complete verify operation as defined by
   *		public key algorithm, returning verification status. Optional.
   * @set_pub_key: Function invokes the algorithm specific set public key

KEYCTL_PKEY_QUERY system calls for ecdsa keys return the key size as max_enc_size and max_dec_size, even though such keys cannot be used for encryption/decryption. They're exclusively for signature generation or verification.

Only rsa keys with pkcs1 encoding can also be used for encryption or decryption.

Return 0 instead for ecdsa keys (as well as ecrdsa keys).

Signed-off-by Lukas Wunner <[email protected]>

---
  crypto/asymmetric_keys/public_key.c | 9 +++++++--
  1 file changed, 7 insertions(+), 2 deletions(-)

crypto/asymmetric_keys/public_key.c +7 -0

index bf165d321440..dd44a966947f 100644
@@ -188,6 +188,8 @@ static int software_key_query(const struct kernel_pkey_params *params,
  	ptr = pkey_pack_u32(ptr, pkey->paramlen);
  	memcpy(ptr, pkey->params, pkey->paramlen);
 
+	memset(info, 0, sizeof(*info));
+
  	if (issig) {
  		sig = crypto_alloc_sig(alg_name, 0, 0);
  		if (IS_ERR(sig)) {

@@ -211,6 +213,9 @@ static int software_key_query(const struct kernel_pkey_params *params,
  			info->supported_ops |= KEYCTL_SUPPORTS_SIGN;
 
  		if (strcmp(params->encoding, "pkcs1") == 0) {
+			info->max_enc_size = len;
+			info->max_dec_size = len;
+
  			info->supported_ops |= KEYCTL_SUPPORTS_ENCRYPT;
  			if (pkey->key_is_private)
  				info->supported_ops |= KEYCTL_SUPPORTS_DECRYPT;

@@ -232,6 +237,8 @@ static int software_key_query(const struct kernel_pkey_params *params,
  		len = crypto_akcipher_maxsize(tfm);
  		info->max_sig_size = len;
  		info->max_data_size = len;
+		info->max_enc_size = len;
+		info->max_dec_size = len;
 
  		info->supported_ops = KEYCTL_SUPPORTS_ENCRYPT;
  		if (pkey->key_is_private)

When user space issues a KEYCTL_PKEY_QUERY system call for a NIST P521 key, the key_size is incorrectly reported as 528 bits instead of 521.

Quoted 1 line

That's because the key size obtained through crypto_sig_keysize() is in

bytes and software_key_query() multiplies by 8 to yield the size in bits. The underlying assumption is that the key size is always a multiple of 8. With the recent addition of NIST P521, that's no longer the case.

Fix by returning the key_size in bits from crypto_sig_keysize() and adjusting the calculations in software_key_query().

The ->key_size() callbacks of sig_alg algorithms now return the size in bits, whereas the ->digest_size() and ->max_size() callbacks return the size in bytes. This matches with the units in struct keyctl_pkey_query.

Fixes a7d45ba77d3d ("crypto: ecdsa - Register NIST P521 and extend test suite")

Signed-off-by Lukas Wunner <[email protected]>> ---

  crypto/asymmetric_keys/public_key.c | 8 ++++----
  crypto/ecdsa-p1363.c                | 4 ++--
  crypto/ecdsa-x962.c                 | 4 ++--
  crypto/ecdsa.c                      | 2 +-
  crypto/ecrdsa.c                     | 2 +-
  crypto/rsassa-pkcs1.c               | 2 +-
  crypto/sig.c                        | 9 +++++++--
  include/crypto/sig.h                | 2 +-
  8 files changed, 19 insertions(+), 14 deletions(-)

crypto/asymmetric_keys/public_key.c +4 -4

index dd44a966947f..6cafb2b8aa22 100644
@@ -205,6 +205,7 @@ static int software_key_query(const struct kernel_pkey_params *params,
  			goto error_free_tfm;
 
  		len = crypto_sig_keysize(sig);
+		info->key_size = len;
  		info->max_sig_size = crypto_sig_maxsize(sig);
  		info->max_data_size = crypto_sig_digestsize(sig);
 

@@ -213,8 +214,8 @@ static int software_key_query(const struct kernel_pkey_params *params,
  			info->supported_ops |= KEYCTL_SUPPORTS_SIGN;
 
  		if (strcmp(params->encoding, "pkcs1") == 0) {
-			info->max_enc_size = len;
-			info->max_dec_size = len;
+			info->max_enc_size = len / 8;
+			info->max_dec_size = len / 8;
 
  			info->supported_ops |= KEYCTL_SUPPORTS_ENCRYPT;
  			if (pkey->key_is_private)

@@ -235,6 +236,7 @@ static int software_key_query(const struct kernel_pkey_params *params,
  			goto error_free_tfm;
 
  		len = crypto_akcipher_maxsize(tfm);
+		info->key_size = len * 8;
  		info->max_sig_size = len;
  		info->max_data_size = len;
  		info->max_enc_size = len;

@@ -245,8 +247,6 @@ static int software_key_query(const struct kernel_pkey_params *params,
  			info->supported_ops |= KEYCTL_SUPPORTS_DECRYPT;
  	}
 
-	info->key_size = len * 8;
-
  	ret = 0;
 
  error_free_tfm:

crypto/ecdsa-p1363.c +2 -2

index eaae7214d69b..c4f458df18ed 100644
@@ -21,7 +21,7 @@ static int ecdsa_p1363_verify(struct crypto_sig *tfm,
  			      const void *digest, unsigned int dlen)
  {
  	struct ecdsa_p1363_ctx *ctx = crypto_sig_ctx(tfm);
-	unsigned int keylen = crypto_sig_keysize(ctx->child);
+	unsigned int keylen = DIV_ROUND_UP(crypto_sig_keysize(ctx->child), 8);
  	unsigned int ndigits = DIV_ROUND_UP(keylen, sizeof(u64));
  	struct ecdsa_raw_sig sig;
 

@@ -45,7 +45,7 @@ static unsigned int ecdsa_p1363_max_size(struct crypto_sig *tfm)
  {
  	struct ecdsa_p1363_ctx *ctx = crypto_sig_ctx(tfm);
 
-	return 2 * crypto_sig_keysize(ctx->child);
+	return 2 * DIV_ROUND_UP(crypto_sig_keysize(ctx->child), 8);
  }
 
  static unsigned int ecdsa_p1363_digest_size(struct crypto_sig *tfm)

crypto/ecdsa-x962.c +2 -2

index 6a77c13e192b..0327e1441374 100644
@@ -82,7 +82,7 @@ static int ecdsa_x962_verify(struct crypto_sig *tfm,
  	int err;
 
  	sig_ctx.ndigits = DIV_ROUND_UP(crypto_sig_keysize(ctx->child),
-				       sizeof(u64));
+				       sizeof(u64) * 8);
 
  	err = asn1_ber_decoder(&ecdsasignature_decoder, &sig_ctx, src, slen);
  	if (err < 0)

@@ -103,7 +103,7 @@ static unsigned int ecdsa_x962_max_size(struct crypto_sig *tfm)
  {
  	struct ecdsa_x962_ctx *ctx = crypto_sig_ctx(tfm);
  	struct sig_alg *alg = crypto_sig_alg(ctx->child);
-	int slen = crypto_sig_keysize(ctx->child);
+	int slen = DIV_ROUND_UP(crypto_sig_keysize(ctx->child), 8);
 
  	/*
  	 * Verify takes ECDSA-Sig-Value (described in RFC 5480) as input,

crypto/ecdsa.c +1 -1

index 117526d15dde..a70b60a90a3c 100644
@@ -167,7 +167,7 @@ static unsigned int ecdsa_key_size(struct crypto_sig *tfm)
  {
  	struct ecc_ctx *ctx = crypto_sig_ctx(tfm);
 
-	return DIV_ROUND_UP(ctx->curve->nbits, 8);
+	return ctx->curve->nbits;
  }
 
  static unsigned int ecdsa_digest_size(struct crypto_sig *tfm)

crypto/ecrdsa.c +1 -1

index b3dd8a3ddeb7..53c9fd9f807f 100644
@@ -249,7 +249,7 @@ static unsigned int ecrdsa_key_size(struct crypto_sig *tfm)
  	 * Verify doesn't need any output, so it's just informational
  	 * for keyctl to determine the key bit size.
  	 */
-	return ctx->pub_key.ndigits * sizeof(u64);
+	return ctx->pub_key.ndigits * sizeof(u64) * 8;
  }
 
  static unsigned int ecrdsa_max_size(struct crypto_sig *tfm)

crypto/rsassa-pkcs1.c +1 -1

index d01ac75635e0..299b2512cc95 100644
@@ -301,7 +301,7 @@ static unsigned int rsassa_pkcs1_key_size(struct crypto_sig *tfm)
  {
  	struct rsassa_pkcs1_ctx *ctx = crypto_sig_ctx(tfm);
 
-	return ctx->key_size;
+	return ctx->key_size * 8;
  }
 
  static int rsassa_pkcs1_set_pub_key(struct crypto_sig *tfm,

crypto/sig.c +7 -2

index dfc7cae90802..7399e67c6f12 100644
@@ -102,6 +102,11 @@ static int sig_default_set_key(struct crypto_sig *tfm,
  	return -ENOSYS;
  }
 
+static unsigned int sig_default_size(struct crypto_sig *tfm)
+{
+	return DIV_ROUND_UP(crypto_sig_keysize(tfm), 8);
+}
+
  static int sig_prepare_alg(struct sig_alg *alg)
  {
  	struct crypto_alg *base = &alg->base;

@@ -117,9 +122,9 @@ static int sig_prepare_alg(struct sig_alg *alg)
  	if (!alg->key_size)
  		return -EINVAL;
  	if (!alg->max_size)
-		alg->max_size = alg->key_size;
+		alg->max_size = sig_default_size;
  	if (!alg->digest_size)
-		alg->digest_size = alg->key_size;
+		alg->digest_size = sig_default_size;
 
  	base->cra_type = &crypto_sig_type;
  	base->cra_flags &= ~CRYPTO_ALG_TYPE_MASK;

include/crypto/sig.h +0 -0

index 11024708c069..fa6dafafab3f 100644

Quoted 2 lines · Lukas Wunner

When user space issues a KEYCTL_PKEY_QUERY system call for a NIST P521 key, the key_size is incorrectly reported as 528 bits instead of 521.

Is there a way to query this with keyctl pkey_query?

Quoted 4 lines · Stefan Berger

Quoted 2 lines · Lukas Wunner

When user space issues a KEYCTL_PKEY_QUERY system call for a NIST P521 key, the key_size is incorrectly reported as 528 bits instead of 521.

Quoted 1 line

Is there a way to query this with keyctl pkey_query?

Yes, these are the commands I've used for testing:

id=keyctl padd asymmetric "" %:_uid.0 < end_responder.cert.der keyctl pkey_query $id 0 enc=x962 hash=sha256

This is the certificate I've used:

https://github.com/DMTF/libspdm/raw/refs/heads/main/unit_test/sample_key/ecp521/end_responder.cert.der

Quoted 1 line

Before:

   key_size=528
   max_data_size=64
   max_sig_size=139
   max_enc_size=66
   max_dec_size=66
   encrypt=n
   decrypt=n
   sign=n
   verify=y

After:

   key_size=521
   max_data_size=64
   max_sig_size=139
   max_enc_size=0
   max_dec_size=0
   encrypt=n
   decrypt=n
   sign=n
   verify=y

Thanks,

Lukas

crypto/ecdsa-p1363.c static int ecdsa_p1363_verify(struct crypto_sig *tfm, · line 24

Quoted 11 lines · Lukas Wunner

crypto/ecdsa-p1363.c +1 -1

index eaae7214d69b..c4f458df18ed 100644
@@ -21,7 +21,7 @@ static int ecdsa_p1363_verify(struct crypto_sig *tfm,
 			      const void *digest, unsigned int dlen)
 {
 	struct ecdsa_p1363_ctx *ctx = crypto_sig_ctx(tfm);
-	unsigned int keylen = crypto_sig_keysize(ctx->child);
+	unsigned int keylen = DIV_ROUND_UP(crypto_sig_keysize(ctx->child), 8);

This may overflow unnecessarily, please rewrite these as:

	X / 8 + !!(X & 7)