From b4c4a2c68817ea0b2df8012673fa4e0712681704 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Tue, 27 Apr 2021 16:01:13 +0200 Subject: [PATCH] Implement pem_read_key directly through OSSL_DECODER Using OSSL_STORE is too heavy and breaks things. There were also needed various fixes mainly for missing proper handling of the SM2 keys in the OSSL_DECODER. Fixes #14788 Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/15045) --- crypto/ec/ec_asn1.c | 7 ++ crypto/ec/ec_key.c | 3 + crypto/pem/pem_pkey.c | 84 ++++++------------- crypto/x509/x_pubkey.c | 4 +- providers/fips-sources.checksums | 6 +- providers/fips.checksum | 2 +- providers/implementations/keymgmt/ec_kmgmt.c | 8 +- providers/implementations/keymgmt/rsa_kmgmt.c | 4 +- test/evp_extra_test2.c | 9 +- test/sslapitest.c | 30 ++++--- 10 files changed, 74 insertions(+), 83 deletions(-) diff --git a/crypto/ec/ec_asn1.c b/crypto/ec/ec_asn1.c index ed30d1b3a9..0e37b21ac3 100644 --- a/crypto/ec/ec_asn1.c +++ b/crypto/ec/ec_asn1.c @@ -965,6 +965,9 @@ EC_KEY *d2i_ECPrivateKey(EC_KEY **a, const unsigned char **in, long len) goto err; } + if (EC_GROUP_get_curve_name(ret->group) == NID_sm2) + EC_KEY_set_flags(ret, EC_FLAG_SM2_RANGE); + EC_POINT_clear_free(ret->pub_key); ret->pub_key = EC_POINT_new(ret->group); if (ret->pub_key == NULL) { @@ -1109,6 +1112,10 @@ EC_KEY *d2i_ECParameters(EC_KEY **a, const unsigned char **in, long len) ret->dirty_cnt++; return NULL; } + + if (EC_GROUP_get_curve_name(ret->group) == NID_sm2) + EC_KEY_set_flags(ret, EC_FLAG_SM2_RANGE); + ret->dirty_cnt++; if (a) diff --git a/crypto/ec/ec_key.c b/crypto/ec/ec_key.c index f06715fa6b..ea2bad3e26 100644 --- a/crypto/ec/ec_key.c +++ b/crypto/ec/ec_key.c @@ -678,6 +678,9 @@ int EC_KEY_set_group(EC_KEY *key, const EC_GROUP *group) return 0; EC_GROUP_free(key->group); key->group = EC_GROUP_dup(group); + if (key->group != NULL && EC_GROUP_get_curve_name(key->group) == NID_sm2) + EC_KEY_set_flags(key, EC_FLAG_SM2_RANGE); + key->dirty_cnt++; return (key->group == NULL) ? 0 : 1; } diff --git a/crypto/pem/pem_pkey.c b/crypto/pem/pem_pkey.c index 3faca8d0ec..cbb07a1356 100644 --- a/crypto/pem/pem_pkey.c +++ b/crypto/pem/pem_pkey.c @@ -7,7 +7,7 @@ * https://www.openssl.org/source/license.html */ -/* We need to use some STORE deprecated APIs */ +/* We need to use some deprecated APIs */ #define OPENSSL_SUPPRESS_DEPRECATED #include @@ -20,9 +20,8 @@ #include #include #include -#include +#include #include -#include "crypto/store.h" #include "crypto/asn1.h" #include "crypto/evp.h" #include "pem_local.h" @@ -32,71 +31,42 @@ int ossl_pem_check_suffix(const char *pem_str, const char *suffix); static EVP_PKEY *pem_read_bio_key(BIO *bp, EVP_PKEY **x, pem_password_cb *cb, void *u, OSSL_LIB_CTX *libctx, const char *propq, - int expected_store_info_type, - int try_secure) + int selection) { - EVP_PKEY *ret = NULL; - OSSL_STORE_CTX *ctx = NULL; - OSSL_STORE_INFO *info = NULL; - const UI_METHOD *ui_method = NULL; - UI_METHOD *allocated_ui_method = NULL; - - if (expected_store_info_type != OSSL_STORE_INFO_PKEY - && expected_store_info_type != OSSL_STORE_INFO_PUBKEY - && expected_store_info_type != OSSL_STORE_INFO_PARAMS) { - ERR_raise(ERR_LIB_PEM, ERR_R_PASSED_INVALID_ARGUMENT); + EVP_PKEY *pkey = NULL; + OSSL_DECODER_CTX *dctx = NULL; + + dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "PEM", NULL, NULL, + selection, libctx, propq); + + if (dctx == NULL) return NULL; - } if (cb == NULL) cb = PEM_def_callback; - ui_method = allocated_ui_method = UI_UTIL_wrap_read_pem_callback(cb, 0); - if (ui_method == NULL) - return NULL; - if ((ctx = OSSL_STORE_attach(bp, "file", libctx, propq, ui_method, u, - NULL, NULL, NULL)) == NULL) + if (!OSSL_DECODER_CTX_set_pem_password_cb(dctx, cb, u)) goto err; -#ifndef OPENSSL_NO_SECURE_HEAP -# ifndef OPENSSL_NO_DEPRECATED_3_0 - if (try_secure) { - int on = 1; - if (!OSSL_STORE_ctrl(ctx, OSSL_STORE_C_USE_SECMEM, &on)) + + while (!OSSL_DECODER_from_bio(dctx, bp) || pkey == NULL) + if (BIO_eof(bp) != 0) goto err; - } -# endif -#endif - if (!OSSL_STORE_expect(ctx, expected_store_info_type)) + if (!evp_keymgmt_util_has(pkey, selection)) { + EVP_PKEY_free(pkey); + pkey = NULL; + ERR_raise(ERR_LIB_PEM, PEM_R_UNSUPPORTED_KEY_COMPONENTS); goto err; - - while (!OSSL_STORE_eof(ctx) - && (info = OSSL_STORE_load(ctx)) != NULL) { - if (OSSL_STORE_INFO_get_type(info) == expected_store_info_type) { - switch (expected_store_info_type) { - case OSSL_STORE_INFO_PKEY: - ret = OSSL_STORE_INFO_get1_PKEY(info); - break; - case OSSL_STORE_INFO_PUBKEY: - ret = OSSL_STORE_INFO_get1_PUBKEY(info); - break; - case OSSL_STORE_INFO_PARAMS: - ret = OSSL_STORE_INFO_get1_PARAMS(info); - break; - } - } - OSSL_STORE_INFO_free(info); - info = NULL; } - if (ret != NULL && x != NULL) - *x = ret; + if (x != NULL) { + EVP_PKEY_free(*x); + *x = pkey; + } err: - OSSL_STORE_close(ctx); - UI_destroy_method(allocated_ui_method); - OSSL_STORE_INFO_free(info); - return ret; + OSSL_DECODER_CTX_free(dctx); + return pkey; } EVP_PKEY *PEM_read_bio_PUBKEY_ex(BIO *bp, EVP_PKEY **x, @@ -104,7 +74,7 @@ EVP_PKEY *PEM_read_bio_PUBKEY_ex(BIO *bp, EVP_PKEY **x, OSSL_LIB_CTX *libctx, const char *propq) { return pem_read_bio_key(bp, x, cb, u, libctx, propq, - OSSL_STORE_INFO_PUBKEY, 0); + EVP_PKEY_PUBLIC_KEY); } EVP_PKEY *PEM_read_bio_PUBKEY(BIO *bp, EVP_PKEY **x, pem_password_cb *cb, @@ -142,7 +112,7 @@ EVP_PKEY *PEM_read_bio_PrivateKey_ex(BIO *bp, EVP_PKEY **x, OSSL_LIB_CTX *libctx, const char *propq) { return pem_read_bio_key(bp, x, cb, u, libctx, propq, - OSSL_STORE_INFO_PKEY, 1); + EVP_PKEY_KEYPAIR); } EVP_PKEY *PEM_read_bio_PrivateKey(BIO *bp, EVP_PKEY **x, pem_password_cb *cb, @@ -207,7 +177,7 @@ EVP_PKEY *PEM_read_bio_Parameters_ex(BIO *bp, EVP_PKEY **x, OSSL_LIB_CTX *libctx, const char *propq) { return pem_read_bio_key(bp, x, NULL, NULL, libctx, propq, - OSSL_STORE_INFO_PARAMS, 0); + EVP_PKEY_KEY_PARAMETERS); } EVP_PKEY *PEM_read_bio_Parameters(BIO *bp, EVP_PKEY **x) diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c index 9b846a8bc2..966a1a534b 100644 --- a/crypto/x509/x_pubkey.c +++ b/crypto/x509/x_pubkey.c @@ -678,12 +678,14 @@ EC_KEY *d2i_EC_PUBKEY(EC_KEY **a, const unsigned char **pp, long length) EVP_PKEY *pkey; EC_KEY *key = NULL; const unsigned char *q; + int type; q = *pp; pkey = d2i_PUBKEY_legacy(NULL, &q, length); if (pkey == NULL) return NULL; - if (EVP_PKEY_id(pkey) == EVP_PKEY_EC) + type = EVP_PKEY_id(pkey); + if (type == EVP_PKEY_EC || type == EVP_PKEY_SM2) key = EVP_PKEY_get1_EC_KEY(pkey); EVP_PKEY_free(pkey); if (key == NULL) diff --git a/providers/fips-sources.checksums b/providers/fips-sources.checksums index 57c66af718..872759e0c7 100644 --- a/providers/fips-sources.checksums +++ b/providers/fips-sources.checksums @@ -147,7 +147,7 @@ d4969259e4fa5b71d8abbf5e736e658bd1daad6e46d272a9b88e190e2de96b61 crypto/ec/curv 86e2becf9b3870979e2abefa1bd318e1a31820d275e2b50e03b17fc287abb20a crypto/ec/ec_check.c 845a5e6ad6921aed63a18084d6b64a1907e4cb093639153ba32138e0b29ff0e5 crypto/ec/ec_curve.c 8cfd0dcfb5acbf6105691a2d5e2826dba1ff3906707bc9dd6ff9bffcc306468f crypto/ec/ec_cvt.c -2103bb62699b1a0ca4e3f75bd1697d856a9afd7f0051d49e433cf69d62d53e2a crypto/ec/ec_key.c +5485a66d4251bc2f044e4d91f6a6b5068957c3b685237bf96a4b45e9c737420c crypto/ec/ec_key.c 7b34605e017eb81037344538f917c32d3ab85c744a819617e012bab73c27dd68 crypto/ec/ec_kmeth.c 90f070e5a7ea950e6fe88ed81c72161c58a4896efb4608076061e1fe12908908 crypto/ec/ec_lib.c 58aa89c186c9bb6a5075a1d961723fe1fc97c6e290756ae682fe494c4f2435a0 crypto/ec/ec_mult.c @@ -323,7 +323,7 @@ d447cd774869da68a2cc0bbb19c547ee6ed4858c7aee1f3d5bba7796f97823a9 providers/comm eec462d685dd3b4764b076a3c18ecd9dd254350a0b78ddc2f8a60587829e1ce3 providers/common/provider_util.c ce6731be4da709c753bd2c04e88d51d567c955c651e7575bb1410968e6c7620e providers/common/securitycheck.c 50a0e01e877ae818cf874f4515a130db0e869d4e9e8ce882bff1255695aba789 providers/common/securitycheck_fips.c -5c31ba4eedb31e2509288be50280e0df58faa86fe4b5e99a1167a53fd6f3bd0f providers/fips/fipsprov.c +ff2d14b053ecad3a2bc42e2b4a54fe2bbb62fd6068d090dde4d68ae0e14a1a1d providers/fips/fipsprov.c c69e60c29711d55cd5672dab9ff051f3c093d54e63a0ec575baa899e6bbf9c2b providers/fips/self_test.c fb56f801613642f6b497803890b528a643024e3cdb5bd5dd619a2981afb2f3b0 providers/fips/self_test_kats.c 08b287621158afb67e61e52fc34efbb9f9fe22ee6709c7ed6c937d5feb2b7fd8 providers/implementations/asymciphers/rsa_enc.c @@ -375,7 +375,7 @@ a5b4ddffa137a52f6a0a0c0c28c618d9bff00af2ec49e51885fc7af116e04869 providers/impl 1a6b7e37229e81eae3981ab2e0b7669eb24aaa6487738c4b44a970da212560b6 providers/implementations/keymgmt/ecx_kmgmt.c 053a2be39a87f50b877ebdbbf799cf5faf8b2de33b04311d819d212ee1ea329b providers/implementations/keymgmt/kdf_legacy_kmgmt.c 21b259d6a9eb5e319106012179e04963fb9659ed85af37f5c9c8752ec2385dae providers/implementations/keymgmt/mac_legacy_kmgmt.c -c48eb00f0de1c28baaa3cf7c0e85d4d2a20592783aa545f8934da487c05a3e87 providers/implementations/keymgmt/rsa_kmgmt.c +adb3672738af90c3f5829c77abe95af2862b13a7cb1679aac4edc9c704cbdef7 providers/implementations/keymgmt/rsa_kmgmt.c 25d20ceb61cadb495ec890ae2c49c5c1c840b39ac77f20058ee87249cab341ef providers/implementations/macs/cmac_prov.c f51b074d55028d3e24656da348d21ca79f6680fdb30383d936251f1b3467caab providers/implementations/macs/gmac_prov.c 35505704fda658c0911f95974913c1f2dd75c8f91c5d2ec597c70c52624bdfdf providers/implementations/macs/hmac_prov.c diff --git a/providers/fips.checksum b/providers/fips.checksum index 83fe30d81c..3054d8e19f 100644 --- a/providers/fips.checksum +++ b/providers/fips.checksum @@ -1 +1 @@ -3ea8c9568047f0cf5ca79b8de0b7d4daa76044baa6bfe25a22a7bbfe13186f7c providers/fips-sources.checksums +b3dca5cc989c42b9e46c0e0b1738ff17b51ce825f0b87ae13b8f609a0840978f providers/fips-sources.checksums diff --git a/providers/implementations/keymgmt/ec_kmgmt.c b/providers/implementations/keymgmt/ec_kmgmt.c index f563d920c4..2673619ef4 100644 --- a/providers/implementations/keymgmt/ec_kmgmt.c +++ b/providers/implementations/keymgmt/ec_kmgmt.c @@ -1288,14 +1288,8 @@ static void *sm2_gen(void *genctx, OSSL_CALLBACK *osslcb, void *cbarg) ret = ec_gen_assign_group(ec, gctx->gen_group); /* Whether you want it or not, you get a keypair, not just one half */ - if ((gctx->selection & OSSL_KEYMGMT_SELECT_KEYPAIR) != 0) { - /* - * For SM2, we need a new flag to indicate the 'generate' function - * to use a new range - */ - EC_KEY_set_flags(ec, EC_FLAG_SM2_RANGE); + if ((gctx->selection & OSSL_KEYMGMT_SELECT_KEYPAIR) != 0) ret = ret && EC_KEY_generate_key(ec); - } if (ret) return ec; diff --git a/providers/implementations/keymgmt/rsa_kmgmt.c b/providers/implementations/keymgmt/rsa_kmgmt.c index a075c54487..34871629ba 100644 --- a/providers/implementations/keymgmt/rsa_kmgmt.c +++ b/providers/implementations/keymgmt/rsa_kmgmt.c @@ -122,9 +122,7 @@ static int rsa_has(const void *keydata, int selection) if ((selection & RSA_POSSIBLE_SELECTIONS) == 0) return 1; /* the selection is not missing */ - if ((selection & OSSL_KEYMGMT_SELECT_OTHER_PARAMETERS) != 0) - /* This will change with OAEP */ - ok = ok && (RSA_test_flags(rsa, RSA_FLAG_TYPE_RSASSAPSS) != 0); + /* OSSL_KEYMGMT_SELECT_OTHER_PARAMETERS are always available even if empty */ if ((selection & OSSL_KEYMGMT_SELECT_KEYPAIR) != 0) ok = ok && (RSA_get0_e(rsa) != NULL); if ((selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) diff --git a/test/evp_extra_test2.c b/test/evp_extra_test2.c index d9d26711ba..cad1934c5b 100644 --- a/test/evp_extra_test2.c +++ b/test/evp_extra_test2.c @@ -371,10 +371,17 @@ static int test_d2i_PrivateKey_ex(void) { provider = OSSL_PROVIDER_load(NULL, "default"); key_bio = BIO_new_mem_buf((&keydata[0])->kder, (&keydata)[0]->size); - ok = TEST_ptr(pkey = PEM_read_bio_PrivateKey(key_bio, NULL, NULL, NULL)); + if (!TEST_ptr_null(pkey = PEM_read_bio_PrivateKey(key_bio, NULL, NULL, NULL))) + goto err; + + ERR_clear_error(); + if (!TEST_int_ge(BIO_seek(key_bio, 0), 0)) + goto err; + ok = TEST_ptr(pkey = d2i_PrivateKey_bio(key_bio, NULL)); TEST_int_eq(ERR_peek_error(), 0); test_openssl_errors(); + err: EVP_PKEY_free(pkey); BIO_free(key_bio); OSSL_PROVIDER_unload(provider); diff --git a/test/sslapitest.c b/test/sslapitest.c index d4c8bf4d38..ad83491573 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -8102,12 +8102,21 @@ static int test_sigalgs_available(int idx) if (!TEST_ptr(cctx) || !TEST_ptr(sctx)) goto end; - if (!TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(), - TLS_client_method(), - TLS1_VERSION, - 0, - &sctx, &cctx, cert, privkey))) - goto end; + if (idx != 5) { + if (!TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(), + TLS_client_method(), + TLS1_VERSION, + 0, + &sctx, &cctx, cert, privkey))) + goto end; + } else { + if (!TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(), + TLS_client_method(), + TLS1_VERSION, + 0, + &sctx, &cctx, cert2, privkey2))) + goto end; + } /* Ensure we only use TLSv1.2 ciphersuites based on SHA256 */ if (idx < 4) { @@ -8135,16 +8144,17 @@ static int test_sigalgs_available(int idx) goto end; } - if (!TEST_int_eq(SSL_CTX_use_certificate_file(sctx, cert2, - SSL_FILETYPE_PEM), 1) + if (idx != 5 + && (!TEST_int_eq(SSL_CTX_use_certificate_file(sctx, cert2, + SSL_FILETYPE_PEM), 1) || !TEST_int_eq(SSL_CTX_use_PrivateKey_file(sctx, privkey2, SSL_FILETYPE_PEM), 1) - || !TEST_int_eq(SSL_CTX_check_private_key(sctx), 1)) + || !TEST_int_eq(SSL_CTX_check_private_key(sctx), 1))) goto end; if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, - NULL, NULL))) + NULL, NULL))) goto end; if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))) -- 2.34.1