diff options
author | Richard Levitte <levitte@openssl.org> | 2020-12-10 18:33:16 +0100 |
---|---|---|
committer | Richard Levitte <levitte@openssl.org> | 2020-12-17 12:02:08 +0100 |
commit | 054cde175664f3e7c8fe5f753c0a5cb5be75dccc (patch) | |
tree | 330481b6242430ead6739c60b3e8cf080777d9c4 /crypto | |
parent | 4159ebca3cb3d9586d6709c7a0166a4af5676f91 (diff) |
DECODER EVP_PKEY: Don't store all the EVP_KEYMGMTs
OSSL_DECODER_CTX_new_by_EVP_PKEY() would keep copies of all the
EVP_KEYMGMTs it finds.
This turns out to be fragile in certain circumstances, so we switch to
fetch the appropriate EVP_KEYMGMT when it's time to construct an
EVP_PKEY from the decoded data instead. This has the added benefit
that we now actually use the property query string that was given by
the caller for these fetches.
Fixes #13503
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13661)
Diffstat (limited to 'crypto')
-rw-r--r-- | crypto/encode_decode/decoder_pkey.c | 234 |
1 files changed, 100 insertions, 134 deletions
diff --git a/crypto/encode_decode/decoder_pkey.c b/crypto/encode_decode/decoder_pkey.c index 016d6047bd..c515cb6d44 100644 --- a/crypto/encode_decode/decoder_pkey.c +++ b/crypto/encode_decode/decoder_pkey.c @@ -55,9 +55,11 @@ int OSSL_DECODER_CTX_set_passphrase_cb(OSSL_DECODER_CTX *ctx, DEFINE_STACK_OF(EVP_KEYMGMT) struct decoder_EVP_PKEY_data_st { + OSSL_LIB_CTX *libctx; + char *propq; + char *object_type; /* recorded object data type, may be NULL */ void **object; /* Where the result should end up */ - STACK_OF(EVP_KEYMGMT) *keymgmts; /* The EVP_KEYMGMTs we handle */ }; static int decoder_construct_EVP_PKEY(OSSL_DECODER_INSTANCE *decoder_inst, @@ -67,7 +69,7 @@ static int decoder_construct_EVP_PKEY(OSSL_DECODER_INSTANCE *decoder_inst, struct decoder_EVP_PKEY_data_st *data = construct_data; OSSL_DECODER *decoder = OSSL_DECODER_INSTANCE_get_decoder(decoder_inst); void *decoderctx = OSSL_DECODER_INSTANCE_get_decoder_ctx(decoder_inst); - size_t i, end_i; + EVP_KEYMGMT *keymgmt = NULL; /* * |object_ref| points to a provider reference to an object, its exact * contents entirely opaque to us, but may be passed to any provider @@ -101,75 +103,54 @@ static int decoder_construct_EVP_PKEY(OSSL_DECODER_INSTANCE *decoder_inst, object_ref = p->data; object_ref_sz = p->data_size; - /* We may have reached one of the goals, let's find out! */ - end_i = sk_EVP_KEYMGMT_num(data->keymgmts); - for (i = 0; end_i; i++) { - EVP_KEYMGMT *keymgmt = sk_EVP_KEYMGMT_value(data->keymgmts, i); + keymgmt = EVP_KEYMGMT_fetch(data->libctx, data->object_type, data->propq); + + if (keymgmt != NULL) { + EVP_PKEY *pkey = NULL; + void *keydata = NULL; + const OSSL_PROVIDER *keymgmt_prov = EVP_KEYMGMT_provider(keymgmt); + const OSSL_PROVIDER *decoder_prov = OSSL_DECODER_provider(decoder); /* - * There are two ways to find a matching KEYMGMT: - * - * 1. If the object data type (recorded in |data->object_type|) - * is defined, by checking it using EVP_KEYMGMT_is_a(). - * 2. If the object data type is NOT defined, by comparing the - * EVP_KEYMGMT and OSSL_DECODER method numbers. Since - * EVP_KEYMGMT and OSSL_DECODE operate with the same - * namemap, we know that the method numbers must match. + * If the EVP_KEYMGMT and the OSSL_DECODER are from the + * same provider, we assume that the KEYMGMT has a key loading + * function that can handle the provider reference we hold. * - * This allows individual decoders to specify variants of keys, - * such as a DER to RSA decoder finding a RSA-PSS key, without - * having to decode the exact same DER blob into the exact same - * internal structure twice. This is, of course, entirely at the - * discretion of the decoder implementations. + * Otherwise, we export from the decoder and import the + * result in the keymgmt. */ - if (data->object_type != NULL - ? EVP_KEYMGMT_is_a(keymgmt, data->object_type) - : EVP_KEYMGMT_number(keymgmt) == OSSL_DECODER_number(decoder)) { - EVP_PKEY *pkey = NULL; - void *keydata = NULL; - const OSSL_PROVIDER *keymgmt_prov = - EVP_KEYMGMT_provider(keymgmt); - const OSSL_PROVIDER *decoder_prov = - OSSL_DECODER_provider(decoder); + if (keymgmt_prov == decoder_prov) { + keydata = evp_keymgmt_load(keymgmt, object_ref, object_ref_sz); + } else { + struct evp_keymgmt_util_try_import_data_st import_data; + + import_data.keymgmt = keymgmt; + import_data.keydata = NULL; + import_data.selection = OSSL_KEYMGMT_SELECT_ALL; /* - * If the EVP_KEYMGMT and the OSSL_DECODER are from the - * same provider, we assume that the KEYMGMT has a key loading - * function that can handle the provider reference we hold. - * - * Otherwise, we export from the decoder and import the - * result in the keymgmt. + * No need to check for errors here, the value of + * |import_data.keydata| is as much an indicator. */ - if (keymgmt_prov == decoder_prov) { - keydata = evp_keymgmt_load(keymgmt, object_ref, object_ref_sz); - } else { - struct evp_keymgmt_util_try_import_data_st import_data; - - import_data.keymgmt = keymgmt; - import_data.keydata = NULL; - import_data.selection = OSSL_KEYMGMT_SELECT_ALL; - - /* - * No need to check for errors here, the value of - * |import_data.keydata| is as much an indicator. - */ - (void)decoder->export_object(decoderctx, - object_ref, object_ref_sz, - &evp_keymgmt_util_try_import, - &import_data); - keydata = import_data.keydata; - import_data.keydata = NULL; - } - - if (keydata != NULL - && (pkey = - evp_keymgmt_util_make_pkey(keymgmt, keydata)) == NULL) - evp_keymgmt_freedata(keymgmt, keydata); - - *data->object = pkey; - - break; + (void)decoder->export_object(decoderctx, + object_ref, object_ref_sz, + &evp_keymgmt_util_try_import, + &import_data); + keydata = import_data.keydata; + import_data.keydata = NULL; } + + if (keydata != NULL + && (pkey = evp_keymgmt_util_make_pkey(keymgmt, keydata)) == NULL) + evp_keymgmt_freedata(keymgmt, keydata); + + *data->object = pkey; + + /* + * evp_keymgmt_util_make_pkey() increments the reference count when + * assigning the EVP_PKEY, so we can free the keymgmt here. + */ + EVP_KEYMGMT_free(keymgmt); } /* * We successfully looked through, |*ctx->object| determines if we @@ -183,63 +164,37 @@ static void decoder_clean_EVP_PKEY_construct_arg(void *construct_data) struct decoder_EVP_PKEY_data_st *data = construct_data; if (data != NULL) { - sk_EVP_KEYMGMT_pop_free(data->keymgmts, EVP_KEYMGMT_free); + OPENSSL_free(data->propq); OPENSSL_free(data->object_type); OPENSSL_free(data); } } -struct collected_data_st { - struct decoder_EVP_PKEY_data_st *process_data; - const char *keytype; - STACK_OF(OPENSSL_CSTRING) *names; - OSSL_DECODER_CTX *ctx; +static void collect_name(const char *name, void *arg) +{ + STACK_OF(OPENSSL_CSTRING) *names = arg; - unsigned int error_occured:1; -}; + sk_OPENSSL_CSTRING_push(names, name); +} static void collect_keymgmt(EVP_KEYMGMT *keymgmt, void *arg) { - struct collected_data_st *data = arg; - - if (data->keytype != NULL && !EVP_KEYMGMT_is_a(keymgmt, data->keytype)) - return; - if (data->error_occured) - return; - - data->error_occured = 1; /* Assume the worst */ + STACK_OF(EVP_KEYMGMT) *keymgmts = arg; if (!EVP_KEYMGMT_up_ref(keymgmt) /* ref++ */) return; - if (sk_EVP_KEYMGMT_push(data->process_data->keymgmts, keymgmt) <= 0) { + if (sk_EVP_KEYMGMT_push(keymgmts, keymgmt) <= 0) { EVP_KEYMGMT_free(keymgmt); /* ref-- */ return; } - - data->error_occured = 0; /* All is good now */ -} - -static void collect_name(const char *name, void *arg) -{ - struct collected_data_st *data = arg; - - if (data->error_occured) - return; - - data->error_occured = 1; /* Assume the worst */ - - if (sk_OPENSSL_CSTRING_push(data->names, name) <= 0) - return; - - data->error_occured = 0; /* All is good now */ } /* * The input structure check is only done on the initial decoder * implementations. */ -static int collect_decoder_check_input_structure(OSSL_DECODER_CTX *ctx, - OSSL_DECODER_INSTANCE *di) +static int decoder_check_input_structure(OSSL_DECODER_CTX *ctx, + OSSL_DECODER_INSTANCE *di) { int di_is_was_set = 0; const char *di_is = @@ -255,15 +210,21 @@ static int collect_decoder_check_input_structure(OSSL_DECODER_CTX *ctx, * If the caller did give an input structure name, the decoder must have * a matching input structure to be accepted. */ - if (di_is != NULL - && strcasecmp(ctx->input_structure, di_is) == 0) + if (di_is != NULL && strcasecmp(ctx->input_structure, di_is) == 0) return 1; return 0; } +struct collect_decoder_data_st { + STACK_OF(OPENSSL_CSTRING) *names; + OSSL_DECODER_CTX *ctx; + + unsigned int error_occured:1; +}; + static void collect_decoder(OSSL_DECODER *decoder, void *arg) { - struct collected_data_st *data = arg; + struct collect_decoder_data_st *data = arg; size_t i, end_i; const OSSL_PROVIDER *prov = OSSL_DECODER_provider(decoder); void *provctx = OSSL_PROVIDER_get0_provider_ctx(prov); @@ -295,7 +256,7 @@ static void collect_decoder(OSSL_DECODER *decoder, void *arg) /* If successful so far, don't free these directly */ decoderctx = NULL; - if (collect_decoder_check_input_structure(data->ctx, di) + if (decoder_check_input_structure(data->ctx, di) && ossl_decoder_ctx_add_decoder_inst(data->ctx, di)) di = NULL; /* If successfully added, don't free it */ } @@ -313,66 +274,71 @@ int ossl_decoder_ctx_setup_for_EVP_PKEY(OSSL_DECODER_CTX *ctx, OSSL_LIB_CTX *libctx, const char *propquery) { - struct collected_data_st *data = NULL; - size_t i, end_i; + struct decoder_EVP_PKEY_data_st *process_data = NULL; + STACK_OF(EVP_KEYMGMT) *keymgmts = NULL; + STACK_OF(OPENSSL_CSTRING) *names = NULL; int ok = 0; - if ((data = OPENSSL_zalloc(sizeof(*data))) == NULL - || (data->process_data = - OPENSSL_zalloc(sizeof(*data->process_data))) == NULL - || (data->process_data->keymgmts = sk_EVP_KEYMGMT_new_null()) == NULL - || (data->names = sk_OPENSSL_CSTRING_new_null()) == NULL) { + if ((process_data = OPENSSL_zalloc(sizeof(*process_data))) == NULL + || (propquery != NULL + && (process_data->propq = OPENSSL_strdup(propquery)) == NULL) + || (keymgmts = sk_EVP_KEYMGMT_new_null()) == NULL + || (names = sk_OPENSSL_CSTRING_new_null()) == NULL) { ERR_raise(ERR_LIB_OSSL_DECODER, ERR_R_MALLOC_FAILURE); goto err; } - data->process_data->object = (void **)pkey; - data->ctx = ctx; - data->keytype = keytype; - /* First, find all keymgmts to form goals */ - EVP_KEYMGMT_do_all_provided(libctx, collect_keymgmt, data); + process_data->object = (void **)pkey; + process_data->libctx = libctx; - if (data->error_occured) - goto err; + /* First, find all keymgmts to form goals */ + EVP_KEYMGMT_do_all_provided(libctx, collect_keymgmt, keymgmts); /* Then, we collect all the keymgmt names */ - end_i = sk_EVP_KEYMGMT_num(data->process_data->keymgmts); - for (i = 0; i < end_i; i++) { - EVP_KEYMGMT *keymgmt = - sk_EVP_KEYMGMT_value(data->process_data->keymgmts, i); + while (sk_EVP_KEYMGMT_num(keymgmts) > 0) { + EVP_KEYMGMT *keymgmt = sk_EVP_KEYMGMT_shift(keymgmts); - EVP_KEYMGMT_names_do_all(keymgmt, collect_name, data); + /* + * If the key type is given by the caller, we only use the matching + * KEYMGMTs, otherwise we use them all. + */ + if (keytype == NULL || EVP_KEYMGMT_is_a(keymgmt, keytype)) + EVP_KEYMGMT_names_do_all(keymgmt, collect_name, names); - if (data->error_occured) - goto err; + EVP_KEYMGMT_free(keymgmt); } + sk_EVP_KEYMGMT_free(keymgmts); /* * Finally, find all decoders that have any keymgmt of the collected * keymgmt names */ - OSSL_DECODER_do_all_provided(libctx, collect_decoder, data); + { + struct collect_decoder_data_st collect_decoder_data = { NULL, }; - if (data->error_occured) - goto err; + collect_decoder_data.names = names; + collect_decoder_data.ctx = ctx; + OSSL_DECODER_do_all_provided(libctx, + collect_decoder, &collect_decoder_data); + sk_OPENSSL_CSTRING_free(names); + + if (collect_decoder_data.error_occured) + goto err; + } if (OSSL_DECODER_CTX_get_num_decoders(ctx) != 0) { if (!OSSL_DECODER_CTX_set_construct(ctx, decoder_construct_EVP_PKEY) - || !OSSL_DECODER_CTX_set_construct_data(ctx, data->process_data) + || !OSSL_DECODER_CTX_set_construct_data(ctx, process_data) || !OSSL_DECODER_CTX_set_cleanup(ctx, decoder_clean_EVP_PKEY_construct_arg)) goto err; - data->process_data = NULL; /* Avoid it being freed */ + process_data = NULL; /* Avoid it being freed */ } ok = 1; err: - if (data != NULL) { - decoder_clean_EVP_PKEY_construct_arg(data->process_data); - sk_OPENSSL_CSTRING_free(data->names); - OPENSSL_free(data); - } + decoder_clean_EVP_PKEY_construct_arg(process_data); return ok; } |