From 98642df4ba886818900ab7e6b23703544e6addd4 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 10 Nov 2022 10:46:32 -0500 Subject: [PATCH 1/3] Propagate selection all the way on key export EVP_PKEY_eq() is used to check, among other things, if a certificate public key corresponds to a private key. When the private key belongs to a provider that does not allow to export private keys this currently fails as the internal functions used to import/export keys ignored the selection given (which specifies that only the public key needs to be considered) and instead tries to export everything. This patch allows to propagate the selection all the way down including adding it in the cache so that a following operation actually looking for other selection parameters does not mistakenly pick up an export containing only partial information. Signed-off-by: Simo Sorce Reviewed-by: Dmitry Belyavskiy Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/19648) diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c index b06730dc7a..2d0238ee27 100644 --- a/crypto/evp/keymgmt_lib.c +++ b/crypto/evp/keymgmt_lib.c @@ -93,7 +93,8 @@ int evp_keymgmt_util_export(const EVP_PKEY *pk, int selection, export_cb, export_cbarg); } -void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt) +void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt, + int selection) { struct evp_keymgmt_util_try_import_data_st import_data; OP_CACHE_ELEM *op; @@ -127,7 +128,7 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt) */ if (pk->dirty_cnt == pk->dirty_cnt_copy) { /* If this key is already exported to |keymgmt|, no more to do */ - op = evp_keymgmt_util_find_operation_cache(pk, keymgmt); + op = evp_keymgmt_util_find_operation_cache(pk, keymgmt, selection); if (op != NULL && op->keymgmt != NULL) { void *ret = op->keydata; @@ -157,13 +158,13 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt) /* Setup for the export callback */ import_data.keydata = NULL; /* evp_keymgmt_util_try_import will create it */ import_data.keymgmt = keymgmt; - import_data.selection = OSSL_KEYMGMT_SELECT_ALL; + import_data.selection = selection; /* * The export function calls the callback (evp_keymgmt_util_try_import), * which does the import for us. If successful, we're done. */ - if (!evp_keymgmt_util_export(pk, OSSL_KEYMGMT_SELECT_ALL, + if (!evp_keymgmt_util_export(pk, selection, &evp_keymgmt_util_try_import, &import_data)) /* If there was an error, bail out */ return NULL; @@ -173,7 +174,7 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt) return NULL; } /* Check to make sure some other thread didn't get there first */ - op = evp_keymgmt_util_find_operation_cache(pk, keymgmt); + op = evp_keymgmt_util_find_operation_cache(pk, keymgmt, selection); if (op != NULL && op->keydata != NULL) { void *ret = op->keydata; @@ -196,7 +197,8 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt) evp_keymgmt_util_clear_operation_cache(pk, 0); /* Add the new export to the operation cache */ - if (!evp_keymgmt_util_cache_keydata(pk, keymgmt, import_data.keydata)) { + if (!evp_keymgmt_util_cache_keydata(pk, keymgmt, import_data.keydata, + selection)) { CRYPTO_THREAD_unlock(pk->lock); evp_keymgmt_freedata(keymgmt, import_data.keydata); return NULL; @@ -232,7 +234,8 @@ int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk, int locking) } OP_CACHE_ELEM *evp_keymgmt_util_find_operation_cache(EVP_PKEY *pk, - EVP_KEYMGMT *keymgmt) + EVP_KEYMGMT *keymgmt, + int selection) { int i, end = sk_OP_CACHE_ELEM_num(pk->operation_cache); OP_CACHE_ELEM *p; @@ -243,14 +246,14 @@ OP_CACHE_ELEM *evp_keymgmt_util_find_operation_cache(EVP_PKEY *pk, */ for (i = 0; i < end; i++) { p = sk_OP_CACHE_ELEM_value(pk->operation_cache, i); - if (keymgmt == p->keymgmt) + if (keymgmt == p->keymgmt && (p->selection & selection) == selection) return p; } return NULL; } -int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, - EVP_KEYMGMT *keymgmt, void *keydata) +int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt, + void *keydata, int selection) { OP_CACHE_ELEM *p = NULL; @@ -266,6 +269,7 @@ int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, return 0; p->keydata = keydata; p->keymgmt = keymgmt; + p->selection = selection; if (!EVP_KEYMGMT_up_ref(keymgmt)) { OPENSSL_free(p); @@ -391,7 +395,8 @@ int evp_keymgmt_util_match(EVP_PKEY *pk1, EVP_PKEY *pk2, int selection) ok = 1; if (keydata1 != NULL) { tmp_keydata = - evp_keymgmt_util_export_to_provider(pk1, keymgmt2); + evp_keymgmt_util_export_to_provider(pk1, keymgmt2, + selection); ok = (tmp_keydata != NULL); } if (ok) { @@ -411,7 +416,8 @@ int evp_keymgmt_util_match(EVP_PKEY *pk1, EVP_PKEY *pk2, int selection) ok = 1; if (keydata2 != NULL) { tmp_keydata = - evp_keymgmt_util_export_to_provider(pk2, keymgmt1); + evp_keymgmt_util_export_to_provider(pk2, keymgmt1, + selection); ok = (tmp_keydata != NULL); } if (ok) { diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c index 70d17ec37e..905e9c9ce4 100644 --- a/crypto/evp/p_lib.c +++ b/crypto/evp/p_lib.c @@ -1822,6 +1822,7 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx, { EVP_KEYMGMT *allocated_keymgmt = NULL; EVP_KEYMGMT *tmp_keymgmt = NULL; + int selection = OSSL_KEYMGMT_SELECT_ALL; void *keydata = NULL; int check; @@ -1883,7 +1884,8 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx, if (pk->ameth->dirty_cnt(pk) == pk->dirty_cnt_copy) { if (!CRYPTO_THREAD_read_lock(pk->lock)) goto end; - op = evp_keymgmt_util_find_operation_cache(pk, tmp_keymgmt); + op = evp_keymgmt_util_find_operation_cache(pk, tmp_keymgmt, + selection); /* * If |tmp_keymgmt| is present in the operation cache, it means @@ -1938,7 +1940,7 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx, EVP_KEYMGMT_free(tmp_keymgmt); /* refcnt-- */ /* Check to make sure some other thread didn't get there first */ - op = evp_keymgmt_util_find_operation_cache(pk, tmp_keymgmt); + op = evp_keymgmt_util_find_operation_cache(pk, tmp_keymgmt, selection); if (op != NULL && op->keymgmt != NULL) { void *tmp_keydata = op->keydata; @@ -1949,7 +1951,8 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx, } /* Add the new export to the operation cache */ - if (!evp_keymgmt_util_cache_keydata(pk, tmp_keymgmt, keydata)) { + if (!evp_keymgmt_util_cache_keydata(pk, tmp_keymgmt, keydata, + selection)) { CRYPTO_THREAD_unlock(pk->lock); evp_keymgmt_freedata(tmp_keymgmt, keydata); keydata = NULL; @@ -1964,7 +1967,7 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx, } #endif /* FIPS_MODULE */ - keydata = evp_keymgmt_util_export_to_provider(pk, tmp_keymgmt); + keydata = evp_keymgmt_util_export_to_provider(pk, tmp_keymgmt, selection); end: /* diff --git a/include/crypto/evp.h b/include/crypto/evp.h index f601b72807..dbbdcccbda 100644 --- a/include/crypto/evp.h +++ b/include/crypto/evp.h @@ -589,6 +589,7 @@ int evp_cipher_asn1_to_param_ex(EVP_CIPHER_CTX *c, ASN1_TYPE *type, typedef struct { EVP_KEYMGMT *keymgmt; void *keydata; + int selection; } OP_CACHE_ELEM; DEFINE_STACK_OF(OP_CACHE_ELEM) @@ -778,12 +779,14 @@ EVP_PKEY *evp_keymgmt_util_make_pkey(EVP_KEYMGMT *keymgmt, void *keydata); int evp_keymgmt_util_export(const EVP_PKEY *pk, int selection, OSSL_CALLBACK *export_cb, void *export_cbarg); -void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt); +void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt, + int selection); OP_CACHE_ELEM *evp_keymgmt_util_find_operation_cache(EVP_PKEY *pk, - EVP_KEYMGMT *keymgmt); + EVP_KEYMGMT *keymgmt, + int selection); int evp_keymgmt_util_clear_operation_cache(EVP_PKEY *pk, int locking); -int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, - EVP_KEYMGMT *keymgmt, void *keydata); +int evp_keymgmt_util_cache_keydata(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt, + void *keydata, int selection); void evp_keymgmt_util_cache_keyinfo(EVP_PKEY *pk); void *evp_keymgmt_util_fromdata(EVP_PKEY *target, EVP_KEYMGMT *keymgmt, int selection, const OSSL_PARAM params[]); -- 2.38.1