From 5bb3d56f351833e5ad80e5976691957f70f7d8cf Mon Sep 17 00:00:00 2001 From: Vitaly Chikunov Date: Thu, 11 Apr 2019 18:51:15 +0300 Subject: crypto: akcipher - new verify API for public key algorithms Previous akcipher .verify() just `decrypts' (using RSA encrypt which is using public key) signature to uncover message hash, which was then compared in upper level public_key_verify_signature() with the expected hash value, which itself was never passed into verify(). This approach was incompatible with EC-DSA family of algorithms, because, to verify a signature EC-DSA algorithm also needs a hash value as input; then it's used (together with a signature divided into halves `r||s') to produce a witness value, which is then compared with `r' to determine if the signature is correct. Thus, for EC-DSA, nor requirements of .verify() itself, nor its output expectations in public_key_verify_signature() wasn't sufficient. Make improved .verify() call which gets hash value as input and produce complete signature check without any output besides status. Now for the top level verification only crypto_akcipher_verify() needs to be called and its return value inspected. Make sure that `digest' is in kmalloc'd memory (in place of `output`) in {public,tpm}_key_verify_signature() as insisted by Herbert Xu, and will be changed in the following commit. Cc: David Howells Cc: keyrings@vger.kernel.org Signed-off-by: Vitaly Chikunov Reviewed-by: Denis Kenzior Signed-off-by: Herbert Xu --- crypto/asymmetric_keys/public_key.c | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) (limited to 'crypto/asymmetric_keys/public_key.c') diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index f5d85b47..0c069fe8 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -227,10 +227,9 @@ int public_key_verify_signature(const struct public_key *pkey, struct crypto_wait cwait; struct crypto_akcipher *tfm; struct akcipher_request *req; - struct scatterlist sig_sg, digest_sg; + struct scatterlist src_sg[2]; char alg_name[CRYPTO_MAX_ALG_NAME]; - void *output; - unsigned int outlen; + void *digest; int ret; pr_devel("==>%s()\n", __func__); @@ -264,35 +263,22 @@ int public_key_verify_signature(const struct public_key *pkey, goto error_free_req; ret = -ENOMEM; - outlen = crypto_akcipher_maxsize(tfm); - output = kmalloc(outlen, GFP_KERNEL); - if (!output) + digest = kmemdup(sig->digest, sig->digest_size, GFP_KERNEL); + if (!digest) goto error_free_req; - sg_init_one(&sig_sg, sig->s, sig->s_size); - sg_init_one(&digest_sg, output, outlen); - akcipher_request_set_crypt(req, &sig_sg, &digest_sg, sig->s_size, - outlen); + sg_init_table(src_sg, 2); + sg_set_buf(&src_sg[0], sig->s, sig->s_size); + sg_set_buf(&src_sg[1], digest, sig->digest_size); + akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size, + sig->digest_size); crypto_init_wait(&cwait); akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP, crypto_req_done, &cwait); - - /* Perform the verification calculation. This doesn't actually do the - * verification, but rather calculates the hash expected by the - * signature and returns that to us. - */ ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait); - if (ret) - goto out_free_output; - - /* Do the actual verification step. */ - if (req->dst_len != sig->digest_size || - memcmp(sig->digest, output, sig->digest_size) != 0) - ret = -EKEYREJECTED; -out_free_output: - kfree(output); + kfree(digest); error_free_req: akcipher_request_free(req); error_free_tfm: -- cgit v1.2.3 From 08d6cc22bc61409cb870a2eee6b716f9e3e0b705 Mon Sep 17 00:00:00 2001 From: Vitaly Chikunov Date: Thu, 11 Apr 2019 18:51:16 +0300 Subject: KEYS: do not kmemdup digest in {public,tpm}_key_verify_signature Treat (struct public_key_signature)'s digest same as its signature (s). Since digest should be already in the kmalloc'd memory do not kmemdup digest value before calling {public,tpm}_key_verify_signature. Patch is split from the previous as suggested by Herbert Xu. Suggested-by: David Howells Cc: David Howells Cc: keyrings@vger.kernel.org Signed-off-by: Vitaly Chikunov Reviewed-by: Denis Kenzior Signed-off-by: Herbert Xu --- crypto/asymmetric_keys/asym_tpm.c | 10 +--------- crypto/asymmetric_keys/public_key.c | 9 +-------- 2 files changed, 2 insertions(+), 17 deletions(-) (limited to 'crypto/asymmetric_keys/public_key.c') diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c index 4e5b6fb5..402fc34c 100644 --- a/crypto/asymmetric_keys/asym_tpm.c +++ b/crypto/asymmetric_keys/asym_tpm.c @@ -748,7 +748,6 @@ static int tpm_key_verify_signature(const struct key *key, char alg_name[CRYPTO_MAX_ALG_NAME]; uint8_t der_pub_key[PUB_KEY_BUF_SIZE]; uint32_t der_pub_key_len; - void *digest; int ret; pr_devel("==>%s()\n", __func__); @@ -780,14 +779,9 @@ static int tpm_key_verify_signature(const struct key *key, if (!req) goto error_free_tfm; - ret = -ENOMEM; - digest = kmemdup(sig->digest, sig->digest_size, GFP_KERNEL); - if (!digest) - goto error_free_req; - sg_init_table(src_sg, 2); sg_set_buf(&src_sg[0], sig->s, sig->s_size); - sg_set_buf(&src_sg[1], digest, sig->digest_size); + sg_set_buf(&src_sg[1], sig->digest, sig->digest_size); akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size, sig->digest_size); crypto_init_wait(&cwait); @@ -796,8 +790,6 @@ static int tpm_key_verify_signature(const struct key *key, crypto_req_done, &cwait); ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait); - kfree(digest); -error_free_req: akcipher_request_free(req); error_free_tfm: crypto_free_akcipher(tfm); diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 0c069fe8..33093b7b 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -229,7 +229,6 @@ int public_key_verify_signature(const struct public_key *pkey, struct akcipher_request *req; struct scatterlist src_sg[2]; char alg_name[CRYPTO_MAX_ALG_NAME]; - void *digest; int ret; pr_devel("==>%s()\n", __func__); @@ -262,14 +261,9 @@ int public_key_verify_signature(const struct public_key *pkey, if (ret) goto error_free_req; - ret = -ENOMEM; - digest = kmemdup(sig->digest, sig->digest_size, GFP_KERNEL); - if (!digest) - goto error_free_req; - sg_init_table(src_sg, 2); sg_set_buf(&src_sg[0], sig->s, sig->s_size); - sg_set_buf(&src_sg[1], digest, sig->digest_size); + sg_set_buf(&src_sg[1], sig->digest, sig->digest_size); akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size, sig->digest_size); crypto_init_wait(&cwait); @@ -278,7 +272,6 @@ int public_key_verify_signature(const struct public_key *pkey, crypto_req_done, &cwait); ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait); - kfree(digest); error_free_req: akcipher_request_free(req); error_free_tfm: -- cgit v1.2.3 From 1bef742086ae8082ebb7ee069a0436043c016c1c Mon Sep 17 00:00:00 2001 From: Vitaly Chikunov Date: Thu, 11 Apr 2019 18:51:17 +0300 Subject: X.509: parse public key parameters from x509 for akcipher Some public key algorithms (like EC-DSA) keep in parameters field important data such as digest and curve OIDs (possibly more for different EC-DSA variants). Thus, just setting a public key (as for RSA) is not enough. Append parameters into the key stream for akcipher_set_{pub,priv}_key. Appended data is: (u32) algo OID, (u32) parameters length, parameters data. This does not affect current akcipher API nor RSA ciphers (they could ignore it). Idea of appending parameters to the key stream is by Herbert Xu. Cc: David Howells Cc: Denis Kenzior Cc: keyrings@vger.kernel.org Signed-off-by: Vitaly Chikunov Reviewed-by: Denis Kenzior Signed-off-by: Herbert Xu --- crypto/asymmetric_keys/asym_tpm.c | 9 +++- crypto/asymmetric_keys/public_key.c | 72 ++++++++++++++++++++++++------- crypto/asymmetric_keys/x509.asn1 | 2 +- crypto/asymmetric_keys/x509_cert_parser.c | 31 +++++++++++++ crypto/testmgr.c | 24 +++++++++-- crypto/testmgr.h | 5 +++ 6 files changed, 122 insertions(+), 21 deletions(-) (limited to 'crypto/asymmetric_keys/public_key.c') diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c index 402fc34c..76d2ce3a 100644 --- a/crypto/asymmetric_keys/asym_tpm.c +++ b/crypto/asymmetric_keys/asym_tpm.c @@ -276,6 +276,10 @@ static int tpm_sign(struct tpm_buf *tb, return datalen; } + +/* Room to fit two u32 zeros for algo id and parameters length. */ +#define SETKEY_PARAMS_SIZE (sizeof(u32) * 2) + /* * Maximum buffer size for the BER/DER encoded public key. The public key * is of the form SEQUENCE { INTEGER n, INTEGER e } where n is a maximum 2048 @@ -286,8 +290,9 @@ static int tpm_sign(struct tpm_buf *tb, * - 257 bytes of n * - max 2 bytes for INTEGER e type/length * - 3 bytes of e + * - 4+4 of zeros for set_pub_key parameters (SETKEY_PARAMS_SIZE) */ -#define PUB_KEY_BUF_SIZE (4 + 4 + 257 + 2 + 3) +#define PUB_KEY_BUF_SIZE (4 + 4 + 257 + 2 + 3 + SETKEY_PARAMS_SIZE) /* * Provide a part of a description of the key for /proc/keys. @@ -364,6 +369,8 @@ static uint32_t derive_pub_key(const void *pub_key, uint32_t len, uint8_t *buf) cur = encode_tag_length(cur, 0x02, sizeof(e)); memcpy(cur, e, sizeof(e)); cur += sizeof(e); + /* Zero parameters to satisfy set_pub_key ABI. */ + memset(cur, 0, SETKEY_PARAMS_SIZE); return cur - buf; } diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 33093b7b..77e0ae78 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -45,6 +45,7 @@ void public_key_free(struct public_key *key) { if (key) { kfree(key->key); + kfree(key->params); kfree(key); } } @@ -94,6 +95,12 @@ int software_key_determine_akcipher(const char *encoding, return -ENOPKG; } +static u8 *pkey_pack_u32(u8 *dst, u32 val) +{ + memcpy(dst, &val, sizeof(val)); + return dst + sizeof(val); +} + /* * Query information about a key. */ @@ -103,6 +110,7 @@ static int software_key_query(const struct kernel_pkey_params *params, struct crypto_akcipher *tfm; struct public_key *pkey = params->key->payload.data[asym_crypto]; char alg_name[CRYPTO_MAX_ALG_NAME]; + u8 *key, *ptr; int ret, len; ret = software_key_determine_akcipher(params->encoding, @@ -115,14 +123,22 @@ static int software_key_query(const struct kernel_pkey_params *params, if (IS_ERR(tfm)) return PTR_ERR(tfm); + key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, + GFP_KERNEL); + if (!key) + goto error_free_tfm; + memcpy(key, pkey->key, pkey->keylen); + ptr = key + pkey->keylen; + ptr = pkey_pack_u32(ptr, pkey->algo); + ptr = pkey_pack_u32(ptr, pkey->paramlen); + memcpy(ptr, pkey->params, pkey->paramlen); + if (pkey->key_is_private) - ret = crypto_akcipher_set_priv_key(tfm, - pkey->key, pkey->keylen); + ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen); else - ret = crypto_akcipher_set_pub_key(tfm, - pkey->key, pkey->keylen); + ret = crypto_akcipher_set_pub_key(tfm, key, pkey->keylen); if (ret < 0) - goto error_free_tfm; + goto error_free_key; len = crypto_akcipher_maxsize(tfm); info->key_size = len * 8; @@ -137,6 +153,8 @@ static int software_key_query(const struct kernel_pkey_params *params, KEYCTL_SUPPORTS_SIGN); ret = 0; +error_free_key: + kfree(key); error_free_tfm: crypto_free_akcipher(tfm); pr_devel("<==%s() = %d\n", __func__, ret); @@ -155,6 +173,7 @@ static int software_key_eds_op(struct kernel_pkey_params *params, struct crypto_wait cwait; struct scatterlist in_sg, out_sg; char alg_name[CRYPTO_MAX_ALG_NAME]; + char *key, *ptr; int ret; pr_devel("==>%s()\n", __func__); @@ -173,14 +192,23 @@ static int software_key_eds_op(struct kernel_pkey_params *params, if (!req) goto error_free_tfm; + key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, + GFP_KERNEL); + if (!key) + goto error_free_req; + + memcpy(key, pkey->key, pkey->keylen); + ptr = key + pkey->keylen; + ptr = pkey_pack_u32(ptr, pkey->algo); + ptr = pkey_pack_u32(ptr, pkey->paramlen); + memcpy(ptr, pkey->params, pkey->paramlen); + if (pkey->key_is_private) - ret = crypto_akcipher_set_priv_key(tfm, - pkey->key, pkey->keylen); + ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen); else - ret = crypto_akcipher_set_pub_key(tfm, - pkey->key, pkey->keylen); + ret = crypto_akcipher_set_pub_key(tfm, key, pkey->keylen); if (ret) - goto error_free_req; + goto error_free_key; sg_init_one(&in_sg, in, params->in_len); sg_init_one(&out_sg, out, params->out_len); @@ -210,6 +238,8 @@ static int software_key_eds_op(struct kernel_pkey_params *params, if (ret == 0) ret = req->dst_len; +error_free_key: + kfree(key); error_free_req: akcipher_request_free(req); error_free_tfm: @@ -229,6 +259,7 @@ int public_key_verify_signature(const struct public_key *pkey, struct akcipher_request *req; struct scatterlist src_sg[2]; char alg_name[CRYPTO_MAX_ALG_NAME]; + char *key, *ptr; int ret; pr_devel("==>%s()\n", __func__); @@ -252,14 +283,23 @@ int public_key_verify_signature(const struct public_key *pkey, if (!req) goto error_free_tfm; + key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, + GFP_KERNEL); + if (!key) + goto error_free_req; + + memcpy(key, pkey->key, pkey->keylen); + ptr = key + pkey->keylen; + ptr = pkey_pack_u32(ptr, pkey->algo); + ptr = pkey_pack_u32(ptr, pkey->paramlen); + memcpy(ptr, pkey->params, pkey->paramlen); + if (pkey->key_is_private) - ret = crypto_akcipher_set_priv_key(tfm, - pkey->key, pkey->keylen); + ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen); else - ret = crypto_akcipher_set_pub_key(tfm, - pkey->key, pkey->keylen); + ret = crypto_akcipher_set_pub_key(tfm, key, pkey->keylen); if (ret) - goto error_free_req; + goto error_free_key; sg_init_table(src_sg, 2); sg_set_buf(&src_sg[0], sig->s, sig->s_size); @@ -272,6 +312,8 @@ int public_key_verify_signature(const struct public_key *pkey, crypto_req_done, &cwait); ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait); +error_free_key: + kfree(key); error_free_req: akcipher_request_free(req); error_free_tfm: diff --git a/crypto/asymmetric_keys/x509.asn1 b/crypto/asymmetric_keys/x509.asn1 index aae0cde4..5c9f4e4a 100644 --- a/crypto/asymmetric_keys/x509.asn1 +++ b/crypto/asymmetric_keys/x509.asn1 @@ -22,7 +22,7 @@ CertificateSerialNumber ::= INTEGER AlgorithmIdentifier ::= SEQUENCE { algorithm OBJECT IDENTIFIER ({ x509_note_OID }), - parameters ANY OPTIONAL + parameters ANY OPTIONAL ({ x509_note_params }) } Name ::= SEQUENCE OF RelativeDistinguishedName diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 991f4d73..b2cdf2db 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -26,6 +26,9 @@ struct x509_parse_context { const void *cert_start; /* Start of cert content */ const void *key; /* Key data */ size_t key_size; /* Size of key data */ + const void *params; /* Key parameters */ + size_t params_size; /* Size of key parameters */ + enum OID key_algo; /* Public key algorithm */ enum OID last_oid; /* Last OID encountered */ enum OID algo_oid; /* Algorithm OID */ unsigned char nr_mpi; /* Number of MPIs stored */ @@ -109,6 +112,13 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen) cert->pub->keylen = ctx->key_size; + cert->pub->params = kmemdup(ctx->params, ctx->params_size, GFP_KERNEL); + if (!cert->pub->params) + goto error_decode; + + cert->pub->paramlen = ctx->params_size; + cert->pub->algo = ctx->key_algo; + /* Grab the signature bits */ ret = x509_get_sig_params(cert); if (ret < 0) @@ -400,6 +410,27 @@ int x509_note_subject(void *context, size_t hdrlen, return x509_fabricate_name(ctx, hdrlen, tag, &ctx->cert->subject, vlen); } +/* + * Extract the parameters for the public key + */ +int x509_note_params(void *context, size_t hdrlen, + unsigned char tag, + const void *value, size_t vlen) +{ + struct x509_parse_context *ctx = context; + + /* + * AlgorithmIdentifier is used three times in the x509, we should skip + * first and ignore third, using second one which is after subject and + * before subjectPublicKey. + */ + if (!ctx->cert->raw_subject || ctx->key) + return 0; + ctx->params = value - hdrlen; + ctx->params_size = vlen + hdrlen; + return 0; +} + /* * Extract the data for the public key algorithm */ diff --git a/crypto/testmgr.c b/crypto/testmgr.c index 21b27996..bc382b0c 100644 --- a/crypto/testmgr.c +++ b/crypto/testmgr.c @@ -2585,6 +2585,12 @@ static int alg_test_kpp(const struct alg_test_desc *desc, const char *driver, return err; } +static u8 *test_pack_u32(u8 *dst, u32 val) +{ + memcpy(dst, &val, sizeof(val)); + return dst + sizeof(val); +} + static int test_akcipher_one(struct crypto_akcipher *tfm, const struct akcipher_testvec *vecs) { @@ -2599,6 +2605,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, const char *m, *c; unsigned int m_size, c_size; const char *op; + u8 *key, *ptr; if (testmgr_alloc_buf(xbuf)) return err; @@ -2609,12 +2616,20 @@ static int test_akcipher_one(struct crypto_akcipher *tfm, crypto_init_wait(&wait); + key = kmalloc(vecs->key_len + sizeof(u32) * 2 + vecs->param_len, + GFP_KERNEL); + if (!key) + goto free_xbuf; + memcpy(key, vecs->key, vecs->key_len); + ptr = key + vecs->key_len; + ptr = test_pack_u32(ptr, vecs->algo); + ptr = test_pack_u32(ptr, vecs->param_len); + memcpy(ptr, vecs->params, vecs->param_len); + if (vecs->public_key_vec) - err = crypto_akcipher_set_pub_key(tfm, vecs->key, - vecs->key_len); + err = crypto_akcipher_set_pub_key(tfm, key, vecs->key_len); else - err = crypto_akcipher_set_priv_key(tfm, vecs->key, - vecs->key_len); + err = crypto_akcipher_set_priv_key(tfm, key, vecs->key_len); if (err) goto free_req; @@ -2744,6 +2759,7 @@ free_all: kfree(outbuf_enc); free_req: akcipher_request_free(req); + kfree(key); free_xbuf: testmgr_free_buf(xbuf); return err; diff --git a/crypto/testmgr.h b/crypto/testmgr.h index f267633c..75d8f8c3 100644 --- a/crypto/testmgr.h +++ b/crypto/testmgr.h @@ -25,6 +25,8 @@ #ifndef _CRYPTO_TESTMGR_H #define _CRYPTO_TESTMGR_H +#include + #define MAX_IVLEN 32 /* @@ -135,13 +137,16 @@ struct drbg_testvec { struct akcipher_testvec { const unsigned char *key; + const unsigned char *params; const unsigned char *m; const unsigned char *c; unsigned int key_len; + unsigned int param_len; unsigned int m_size; unsigned int c_size; bool public_key_vec; bool siggen_sigver_test; + enum OID algo; }; struct kpp_testvec { -- cgit v1.2.3