From 4ce1025a8ac37d255f569147116dd776f9267cce Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Thu, 27 Aug 2020 10:07:09 +0200 Subject: [PATCH] PEM: Make PEM_write_bio_PrivateKey_traditional() handle provider-native keys PEM_write_bio_PrivateKey_traditional() didn't handle provider-native keys very well. Originally, it would simply use the corresponding encoder, which is likely to output modern PEM (not "traditional"). PEM_write_bio_PrivateKey_traditional() is now changed to try and get a legacy copy of the input EVP_PKEY, and use that copy for traditional output, if it has such support. Internally, evp_pkey_copy_downgraded() is added, to be used when evp_pkey_downgrade() is too intrusive for what it's needed for. Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/12738) --- crypto/evp/p_lib.c | 222 +++++++++++------- crypto/pem/pem_pkey.c | 20 +- .../man3/evp_pkey_export_to_provider.pod | 10 +- include/crypto/evp.h | 2 + 4 files changed, 163 insertions(+), 91 deletions(-) diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c index fec4e2d43b..0f5378c4fe 100644 --- a/crypto/evp/p_lib.c +++ b/crypto/evp/p_lib.c @@ -1369,6 +1369,19 @@ size_t EVP_PKEY_get1_tls_encodedpoint(EVP_PKEY *pkey, unsigned char **ppt) /*- All methods below can also be used in FIPS_MODULE */ +static int evp_pkey_reset_unlocked(EVP_PKEY *pk) +{ + if (pk == NULL) + return 0; + + memset(pk, 0, sizeof(*pk)); + pk->type = EVP_PKEY_NONE; + pk->save_type = EVP_PKEY_NONE; + pk->references = 1; + pk->save_parameters = 1; + return 1; +} + EVP_PKEY *EVP_PKEY_new(void) { EVP_PKEY *ret = OPENSSL_zalloc(sizeof(*ret)); @@ -1377,10 +1390,10 @@ EVP_PKEY *EVP_PKEY_new(void) EVPerr(EVP_F_EVP_PKEY_NEW, ERR_R_MALLOC_FAILURE); return NULL; } - ret->type = EVP_PKEY_NONE; - ret->save_type = EVP_PKEY_NONE; - ret->references = 1; - ret->save_parameters = 1; + + if (!evp_pkey_reset_unlocked(ret)) + goto err; + ret->lock = CRYPTO_THREAD_lock_new(); if (ret->lock == NULL) { EVPerr(EVP_F_EVP_PKEY_NEW, ERR_R_MALLOC_FAILURE); @@ -1802,109 +1815,142 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OPENSSL_CTX *libctx, } #ifndef FIPS_MODULE -int evp_pkey_downgrade(EVP_PKEY *pk) +int evp_pkey_copy_downgraded(EVP_PKEY **dest, const EVP_PKEY *src) { - EVP_KEYMGMT *keymgmt = pk->keymgmt; - void *keydata = pk->keydata; - int type = pk->type; - const char *keytype = NULL; + if (!ossl_assert(dest != NULL)) + return 0; - /* If this isn't a provider side key, we're done */ - if (keymgmt == NULL) - return 1; + if (evp_pkey_is_assigned(src) && evp_pkey_is_provided(src)) { + EVP_KEYMGMT *keymgmt = src->keymgmt; + void *keydata = src->keydata; + int type = src->type; + const char *keytype = NULL; - keytype = evp_first_name(EVP_KEYMGMT_provider(keymgmt), keymgmt->name_id); + keytype = evp_first_name(EVP_KEYMGMT_provider(keymgmt), + keymgmt->name_id); - /* - * If the type is EVP_PKEY_NONE, then we have a problem somewhere else - * in our code. If it's not one of the well known EVP_PKEY_xxx values, - * it should at least be EVP_PKEY_KEYMGMT at this point. - * TODO(3.0) remove this check when we're confident that the rest of the - * code treats this correctly. - */ - if (!ossl_assert(type != EVP_PKEY_NONE)) { - ERR_raise_data(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR, - "keymgmt key type = %s but legacy type = EVP_PKEY_NONE", - keytype); - return 0; - } + /* + * If the type is EVP_PKEY_NONE, then we have a problem somewhere + * else in our code. If it's not one of the well known EVP_PKEY_xxx + * values, it should at least be EVP_PKEY_KEYMGMT at this point. + * TODO(3.0) remove this check when we're confident that the rest + * of the code treats this correctly. + */ + if (!ossl_assert(type != EVP_PKEY_NONE)) { + ERR_raise_data(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR, + "keymgmt key type = %s but legacy type = EVP_PKEY_NONE", + keytype); + return 0; + } - /* Prefer the legacy key type name for error reporting */ - if (type != EVP_PKEY_KEYMGMT) - keytype = OBJ_nid2sn(type); + /* Prefer the legacy key type name for error reporting */ + if (type != EVP_PKEY_KEYMGMT) + keytype = OBJ_nid2sn(type); - /* - * To be able to downgrade, we steal the provider side "origin" keymgmt - * and keydata. We've already grabbed the pointers, so all we need to - * do is clear those pointers in |pk| and then call evp_pkey_free_it(). - * That way, we can restore |pk| if we need to. - */ - pk->keymgmt = NULL; - pk->keydata = NULL; - evp_pkey_free_it(pk); - if (EVP_PKEY_set_type(pk, type)) { - /* If the key is typed but empty, we're done */ - if (keydata == NULL) { - /* We're dropping the EVP_KEYMGMT */ - EVP_KEYMGMT_free(keymgmt); - return 1; - } + /* Make sure we have a clean slate to copy into */ + if (*dest == NULL) + *dest = EVP_PKEY_new(); + else + evp_pkey_free_it(*dest); - if (pk->ameth->import_from == NULL) { - ERR_raise_data(ERR_LIB_EVP, EVP_R_NO_IMPORT_FUNCTION, - "key type = %s", keytype); - } else { - /* - * We perform the export in the same libctx as the keymgmt that we - * are using. - */ - OPENSSL_CTX *libctx = ossl_provider_library_context(keymgmt->prov); - EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_from_pkey(libctx, pk, NULL); - if (pctx == NULL) - ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE); + if (EVP_PKEY_set_type(*dest, type)) { + /* If the key is typed but empty, we're done */ + if (keydata == NULL) + return 1; - if (pctx != NULL - && evp_keymgmt_export(keymgmt, keydata, - OSSL_KEYMGMT_SELECT_ALL, - pk->ameth->import_from, pctx)) { + if ((*dest)->ameth->import_from == NULL) { + ERR_raise_data(ERR_LIB_EVP, EVP_R_NO_IMPORT_FUNCTION, + "key type = %s", keytype); + } else { /* - * Save the provider side data in the operation cache, so they'll - * find it again. evp_pkey_free_it() cleared the cache, so it's - * safe to assume slot zero is free. - * Note that evp_keymgmt_util_cache_keydata() increments keymgmt's - * reference count. + * We perform the export in the same libctx as the keymgmt + * that we are using. */ - evp_keymgmt_util_cache_keydata(pk, 0, keymgmt, keydata); - EVP_PKEY_CTX_free(pctx); + OPENSSL_CTX *libctx = + ossl_provider_library_context(keymgmt->prov); + EVP_PKEY_CTX *pctx = + EVP_PKEY_CTX_new_from_pkey(libctx, *dest, NULL); - /* Synchronize the dirty count */ - pk->dirty_cnt_copy = pk->ameth->dirty_cnt(pk); + if (pctx == NULL) + ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE); - /* evp_keymgmt_export() increased the refcount... */ - EVP_KEYMGMT_free(keymgmt); - return 1; + if (pctx != NULL + && evp_keymgmt_export(keymgmt, keydata, + OSSL_KEYMGMT_SELECT_ALL, + (*dest)->ameth->import_from, + pctx)) { + /* Synchronize the dirty count */ + (*dest)->dirty_cnt_copy = (*dest)->ameth->dirty_cnt(*dest); + + EVP_PKEY_CTX_free(pctx); + return 1; + } + EVP_PKEY_CTX_free(pctx); } - EVP_PKEY_CTX_free(pctx); - } - ERR_raise_data(ERR_LIB_EVP, EVP_R_KEYMGMT_EXPORT_FAILURE, - "key type = %s", keytype); + ERR_raise_data(ERR_LIB_EVP, EVP_R_KEYMGMT_EXPORT_FAILURE, + "key type = %s", keytype); + } } + return 0; +} + +int evp_pkey_downgrade(EVP_PKEY *pk) +{ + EVP_PKEY tmp_copy; /* Stack allocated! */ + + /* If this isn't an assigned provider side key, we're done */ + if (!evp_pkey_is_assigned(pk) || !evp_pkey_is_provided(pk)) + return 1; + /* - * Something went wrong. This could for example happen if the keymgmt - * turns out to be an HSM implementation that refuses to let go of some - * of the key data, typically the private bits. In this case, we restore - * the provider side internal "origin" and leave it at that. + * To be able to downgrade, we steal the contents of |pk|, then reset + * it, and finally try to make it a downgraded copy. If any of that + * fails, we restore the copied contents into |pk|. */ - if (!ossl_assert(evp_keymgmt_util_assign_pkey(pk, keymgmt, keydata))) { - /* This should not be impossible */ - ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR); - return 0; + tmp_copy = *pk; + + if (evp_pkey_reset_unlocked(pk) + && evp_pkey_copy_downgraded(&pk, &tmp_copy)) { + /* Restore the common attributes, then empty |tmp_copy| */ + pk->references = tmp_copy.references; + pk->lock = tmp_copy.lock; + pk->attributes = tmp_copy.attributes; + pk->save_parameters = tmp_copy.save_parameters; + pk->ex_data = tmp_copy.ex_data; + + /* Ensure that stuff we've copied won't be freed */ + tmp_copy.lock = NULL; + tmp_copy.attributes = NULL; + memset(&tmp_copy.ex_data, 0, sizeof(tmp_copy.ex_data)); + + /* + * Save the provider side data in the operation cache, so they'll + * find it again. |pk| is new, so it's safe to assume slot zero + * is free. + * Note that evp_keymgmt_util_cache_keydata() increments keymgmt's + * reference count, so we need to decrement it, or there will be a + * leak. + */ + evp_keymgmt_util_cache_keydata(pk, 0, tmp_copy.keymgmt, + tmp_copy.keydata); + EVP_KEYMGMT_free(tmp_copy.keymgmt); + + /* + * Clear keymgmt and keydata from |tmp_copy|, or they'll get + * inadvertently freed. + */ + tmp_copy.keymgmt = NULL; + tmp_copy.keydata = NULL; + + evp_pkey_free_it(&tmp_copy); + + return 1; } - /* evp_keymgmt_util_assign_pkey() increased the refcount... */ - EVP_KEYMGMT_free(keymgmt); - return 0; /* No downgrade, but at least the key is restored */ + + *pk = tmp_copy; + return 0; } #endif /* FIPS_MODULE */ diff --git a/crypto/pem/pem_pkey.c b/crypto/pem/pem_pkey.c index 8b9bfe101e..462010d2ac 100644 --- a/crypto/pem/pem_pkey.c +++ b/crypto/pem/pem_pkey.c @@ -165,20 +165,36 @@ PEM_write_cb_fnsig(PrivateKey, EVP_PKEY, BIO, write_bio) return PEM_write_bio_PrivateKey_traditional(out, x, enc, kstr, klen, cb, u); } +/* + * Note: there is no way to tell a provided pkey encoder to use "traditional" + * encoding. Therefore, if the pkey is provided, we try to take a copy + * TODO: when #legacy keys are gone, this function will not be possible any + * more and should be removed. + */ int PEM_write_bio_PrivateKey_traditional(BIO *bp, const EVP_PKEY *x, const EVP_CIPHER *enc, const unsigned char *kstr, int klen, pem_password_cb *cb, void *u) { char pem_str[80]; + EVP_PKEY *copy = NULL; + int ret; + + if (evp_pkey_is_assigned(x) + && evp_pkey_is_provided(x) + && evp_pkey_copy_downgraded(©, x)) + x = copy; if (x->ameth == NULL || x->ameth->old_priv_encode == NULL) { ERR_raise(ERR_LIB_PEM, PEM_R_UNSUPPORTED_PUBLIC_KEY_TYPE); return 0; } BIO_snprintf(pem_str, 80, "%s PRIVATE KEY", x->ameth->pem_str); - return PEM_ASN1_write_bio((i2d_of_void *)i2d_PrivateKey, - pem_str, bp, x, enc, kstr, klen, cb, u); + ret = PEM_ASN1_write_bio((i2d_of_void *)i2d_PrivateKey, + pem_str, bp, x, enc, kstr, klen, cb, u); + + EVP_PKEY_free(copy); + return ret; } EVP_PKEY *PEM_read_bio_Parameters_ex(BIO *bp, EVP_PKEY **x, diff --git a/doc/internal/man3/evp_pkey_export_to_provider.pod b/doc/internal/man3/evp_pkey_export_to_provider.pod index 1c80365ca6..b34cf86619 100644 --- a/doc/internal/man3/evp_pkey_export_to_provider.pod +++ b/doc/internal/man3/evp_pkey_export_to_provider.pod @@ -2,7 +2,7 @@ =head1 NAME -evp_pkey_export_to_provider, evp_pkey_downgrade +evp_pkey_export_to_provider, evp_pkey_copy_downgraded, evp_pkey_downgrade - internal EVP_PKEY support functions for providers =head1 SYNOPSIS @@ -13,6 +13,7 @@ evp_pkey_export_to_provider, evp_pkey_downgrade void *evp_pkey_export_to_provider(EVP_PKEY *pk, OPENSSL_CTX *libctx, EVP_KEYMGMT **keymgmt, const char *propquery); + int evp_pkey_copy_downgraded(EVP_PKEY **dest, const EVP_PKEY *src); int evp_pkey_downgrade(EVP_PKEY *pk); =head1 DESCRIPTION @@ -29,6 +30,13 @@ default context), the name of the legacy type of I, and the I If I isn't NULL but I<*keymgmt> is, and the "origin" was successfully exported, then I<*keymgmt> is assigned the implicitly fetched B. +evp_pkey_copy_downgraded() makes a copy of I in legacy form into I<*dest>, +if there's a corresponding legacy implementation. This should be used if the +use of a downgraded key is temporary. +For example, L uses this to try its +best to get "traditional" PEM output even if the input B has a +provider-native internal key. + evp_pkey_downgrade() converts an B with a provider side "origin" key to one with a legacy "origin", if there's a corresponding legacy implementation. This clears the operation cache, except for the provider side "origin" key. diff --git a/include/crypto/evp.h b/include/crypto/evp.h index c488834511..9d9b0a7298 100644 --- a/include/crypto/evp.h +++ b/include/crypto/evp.h @@ -591,6 +591,7 @@ struct evp_pkey_st { # endif /* == Common attributes == */ + /* If these are modified, so must evp_pkey_downgrade() */ CRYPTO_REF_COUNT references; CRYPTO_RWLOCK *lock; STACK_OF(X509_ATTRIBUTE) *attributes; /* [ 0 ] */ @@ -672,6 +673,7 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OPENSSL_CTX *libctx, EVP_KEYMGMT **keymgmt, const char *propquery); #ifndef FIPS_MODULE +int evp_pkey_copy_downgraded(EVP_PKEY **dest, const EVP_PKEY *src); int evp_pkey_downgrade(EVP_PKEY *pk); void evp_pkey_free_legacy(EVP_PKEY *x); #endif -- 2.34.1