| Message ID | 20210201151910.1465705-4-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for x509 certs with NIST p256 and p192 keys | expand |
Hi Stefan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on cryptodev/master]
[also build test ERROR on crypto/master security/next-testing v5.11-rc7 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Stefan-Berger/Add-support-for-x509-certs-with-NIST-p256-and-p192-keys/20210201-232803
base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: x86_64-randconfig-a011-20200911 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/6e1523b0e77785c263bcb639b87a862ae59731a4
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Stefan-Berger/Add-support-for-x509-certs-with-NIST-p256-and-p192-keys/20210201-232803
git checkout 6e1523b0e77785c263bcb639b87a862ae59731a4
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
ld: crypto/asymmetric_keys/public_key.o: in function `software_key_determine_akcipher':
>> crypto/asymmetric_keys/public_key.c:97: undefined reference to `parse_OID'
vim +97 crypto/asymmetric_keys/public_key.c
61
62 /*
63 * Determine the crypto algorithm name.
64 */
65 static
66 int software_key_determine_akcipher(const char *encoding,
67 const char *hash_algo,
68 const struct public_key *pkey,
69 char alg_name[CRYPTO_MAX_ALG_NAME])
70 {
71 int n;
72
73 if (strcmp(encoding, "pkcs1") == 0) {
74 /* The data wangled by the RSA algorithm is typically padded
75 * and encoded in some manner, such as EMSA-PKCS1-1_5 [RFC3447
76 * sec 8.2].
77 */
78 if (!hash_algo)
79 n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
80 "pkcs1pad(%s)",
81 pkey->pkey_algo);
82 else
83 n = snprintf(alg_name, CRYPTO_MAX_ALG_NAME,
84 "pkcs1pad(%s,%s)",
85 pkey->pkey_algo, hash_algo);
86 return n >= CRYPTO_MAX_ALG_NAME ? -EINVAL : 0;
87 }
88
89 if (strcmp(encoding, "raw") == 0) {
90 strcpy(alg_name, pkey->pkey_algo);
91 return 0;
92 }
93
94 if (strcmp(encoding, "x962") == 0) {
95 enum OID oid;
96
> 97 if (parse_OID(pkey->params, pkey->paramlen, &oid) != 0)
98 return -EBADMSG;
99
100 switch (oid) {
101 case OID_id_prime192v1:
102 strcpy(alg_name, "ecdsa-nist-p192");
103 return 0;
104 case OID_id_prime256v1:
105 strcpy(alg_name, "ecdsa-nist-p256");
106 return 0;
107 default:
108 return -EINVAL;
109 }
110 }
111
112 return -ENOPKG;
113 }
114
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 2/11/21 3:03 AM, kernel test robot wrote: > Hi Stefan, > > Thank you for the patch! Yet something to improve: > >>> crypto/asymmetric_keys/public_key.c:97: undefined reference to `parse_OID' So the issue is that only ASYMMETRIC_PUBLIC_KEY_SUBTYPE is selected in this config and the selection of OID_REGISTRY is missing. I am not sure whether ASYMMETRIC_PUBLIC_KEY_SUBTYPE should/could select OID_REGISTRY or whether that would be wrong... ? Stefan
On 2/11/21 12:30 PM, Stefan Berger wrote: > On 2/11/21 3:03 AM, kernel test robot wrote: >> Hi Stefan, >> >> Thank you for the patch! Yet something to improve: >> >>>> crypto/asymmetric_keys/public_key.c:97: undefined reference to >>>> `parse_OID' > > > So the issue is that only ASYMMETRIC_PUBLIC_KEY_SUBTYPE is selected > in this config and the selection of OID_REGISTRY is missing. I am not > sure whether ASYMMETRIC_PUBLIC_KEY_SUBTYPE should/could select > OID_REGISTRY or whether that would be wrong... ? David, if the above is not desired then the following change would let us get rid of the offending parse_OID(). The below change is only for NIST p192 in this experiment but shows that we need to add additional strcmp() cases in x509_check_for_self_signed() since cert->sig->pkey_algo is set to "ecdsa". I am not sure whether we should derive from the signature which curve was used to create the signature so that cert->sig->pkey_algo could be more specific and the simple existing strcmp() would pass. So two possible ways to go forward. Which way should we go? Stefan diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 0aff4e584b11..71d83bb345c4 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -505,6 +505,8 @@ int x509_extract_key_data(void *context, size_t hdrlen, ctx->cert->pub->pkey_algo = "sm2"; break; case OID_id_prime192v1: + ctx->cert->pub->pkey_algo = "ecdsa-nist-p192"; + break; case OID_id_prime256v1: ctx->cert->pub->pkey_algo = "ecdsa"; break; diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c index ae450eb8be14..3ebeed195b61 100644 --- a/crypto/asymmetric_keys/x509_public_key.c +++ b/crypto/asymmetric_keys/x509_public_key.c @@ -129,7 +129,10 @@ int x509_check_for_self_signed(struct x509_certificate *cert) } ret = -EKEYREJECTED; - if (strcmp(cert->pub->pkey_algo, cert->sig->pkey_algo) != 0) +printk(KERN_INFO "%s: %s ==? %s\n", __func__, cert->pub->pkey_algo, cert->sig->pkey_algo); + if (strcmp(cert->pub->pkey_algo, cert->sig->pkey_algo) != 0 && + strncmp(cert->pub->pkey_algo, "ecdsa-nist-p", 12) != 0 && + strcmp(cert->sig->pkey_algo, "ecdsa") != 0) goto out; ret = public_key_verify_signature(cert->pub, cert->sig); > > > Stefan >
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 8892908ad58c..7dae61b79d5a 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -14,6 +14,7 @@ #include <linux/slab.h> #include <linux/seq_file.h> #include <linux/scatterlist.h> +#include <linux/asn1.h> #include <keys/asymmetric-subtype.h> #include <crypto/public_key.h> #include <crypto/akcipher.h> @@ -90,6 +91,24 @@ int software_key_determine_akcipher(const char *encoding, return 0; } + if (strcmp(encoding, "x962") == 0) { + enum OID oid; + + if (parse_OID(pkey->params, pkey->paramlen, &oid) != 0) + return -EBADMSG; + + switch (oid) { + case OID_id_prime192v1: + strcpy(alg_name, "ecdsa-nist-p192"); + return 0; + case OID_id_prime256v1: + strcpy(alg_name, "ecdsa-nist-p256"); + return 0; + default: + return -EINVAL; + } + } + return -ENOPKG; } diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 1621ceaf5c95..0aff4e584b11 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -227,6 +227,26 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, ctx->cert->sig->hash_algo = "sha224"; goto rsa_pkcs1; + case OID_id_ecdsa_with_sha1: + ctx->cert->sig->hash_algo = "sha1"; + goto ecdsa; + + case OID_id_ecdsa_with_sha224: + ctx->cert->sig->hash_algo = "sha224"; + goto ecdsa; + + case OID_id_ecdsa_with_sha256: + ctx->cert->sig->hash_algo = "sha256"; + goto ecdsa; + + case OID_id_ecdsa_with_sha384: + ctx->cert->sig->hash_algo = "sha384"; + goto ecdsa; + + case OID_id_ecdsa_with_sha512: + ctx->cert->sig->hash_algo = "sha512"; + goto ecdsa; + case OID_gost2012Signature256: ctx->cert->sig->hash_algo = "streebog256"; goto ecrdsa; @@ -255,6 +275,11 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, ctx->cert->sig->encoding = "raw"; ctx->algo_oid = ctx->last_oid; return 0; +ecdsa: + ctx->cert->sig->pkey_algo = "ecdsa"; + ctx->cert->sig->encoding = "x962"; + ctx->algo_oid = ctx->last_oid; + return 0; } /* @@ -276,7 +301,8 @@ int x509_note_signature(void *context, size_t hdrlen, if (strcmp(ctx->cert->sig->pkey_algo, "rsa") == 0 || strcmp(ctx->cert->sig->pkey_algo, "ecrdsa") == 0 || - strcmp(ctx->cert->sig->pkey_algo, "sm2") == 0) { + strcmp(ctx->cert->sig->pkey_algo, "sm2") == 0 || + strcmp(ctx->cert->sig->pkey_algo, "ecdsa") == 0) { /* Discard the BIT STRING metadata */ if (vlen < 1 || *(const u8 *)value != 0) return -EBADMSG; @@ -478,6 +504,10 @@ int x509_extract_key_data(void *context, size_t hdrlen, case OID_sm2: ctx->cert->pub->pkey_algo = "sm2"; break; + case OID_id_prime192v1: + case OID_id_prime256v1: + ctx->cert->pub->pkey_algo = "ecdsa"; + break; default: return -ENOPKG; } diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h index f3b2c097c886..ff3cad9f8c1f 100644 --- a/include/linux/oid_registry.h +++ b/include/linux/oid_registry.h @@ -21,6 +21,8 @@ enum OID { OID_id_dsa, /* 1.2.840.10040.4.1 */ OID_id_ecdsa_with_sha1, /* 1.2.840.10045.4.1 */ OID_id_ecPublicKey, /* 1.2.840.10045.2.1 */ + OID_id_prime192v1, /* 1.2.840.10045.3.1.1 */ + OID_id_prime256v1, /* 1.2.840.10045.3.1.7 */ OID_id_ecdsa_with_sha224, /* 1.2.840.10045.4.3.1 */ OID_id_ecdsa_with_sha256, /* 1.2.840.10045.4.3.2 */ OID_id_ecdsa_with_sha384, /* 1.2.840.10045.4.3.3 */
This patch adds support for parsing of x509 certificates that contain ECDSA keys, such as NIST P256, that have been signed by a CA using any of the current SHA hash algorithms. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Cc: David Howells <dhowells@redhat.com> Cc: keyrings@vger.kernel.org --- crypto/asymmetric_keys/public_key.c | 19 ++++++++++++++ crypto/asymmetric_keys/x509_cert_parser.c | 32 ++++++++++++++++++++++- include/linux/oid_registry.h | 2 ++ 3 files changed, 52 insertions(+), 1 deletion(-)