From: Matt Caswell Date: Fri, 21 May 2021 11:21:32 +0000 (+0100) Subject: Don't try the same decoder multiple times X-Git-Tag: openssl-3.0.0-beta1~409 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=669967fdd8e2174ed2b812df8488715c82f21360;p=thirdparty%2Fopenssl.git Don't try the same decoder multiple times The function collect_decoder decides whether a given decoder should be tried or not. It loops through all the names for matching keymgmts to see if any are a match or not. If there is a match then the decoder gets added. However, each keymgmt may have multiple aliases and a decoder was being added for each one. For example DHX has 4 alias names, and therefore 4 instances of the DHX decoder were added and being tried. Reviewed-by: Tomas Mraz Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/15404) --- diff --git a/crypto/encode_decode/decoder_pkey.c b/crypto/encode_decode/decoder_pkey.c index 08deb2d0886..fb8f0d219b4 100644 --- a/crypto/encode_decode/decoder_pkey.c +++ b/crypto/encode_decode/decoder_pkey.c @@ -232,41 +232,57 @@ static void collect_decoder(OSSL_DECODER *decoder, void *arg) if (data->error_occurred) return; - data->error_occurred = 1; /* Assume the worst */ - if (data->names == NULL) + if (data->names == NULL) { + data->error_occurred = 1; + return; + } + + /* + * Either the caller didn't give a selection, or if they did, + * the decoder must tell us if it supports that selection to + * be accepted. If the decoder doesn't have |does_selection|, + * it's seen as taking anything. + */ + if (decoder->does_selection != NULL + && !decoder->does_selection(provctx, data->ctx->selection)) return; end_i = sk_OPENSSL_CSTRING_num(data->names); for (i = 0; i < end_i; i++) { const char *name = sk_OPENSSL_CSTRING_value(data->names, i); - void *decoderctx = NULL; - OSSL_DECODER_INSTANCE *di = NULL; - if (OSSL_DECODER_is_a(decoder, name) - /* - * Either the caller didn't give a selection, or if they did, - * the decoder must tell us if it supports that selection to - * be accepted. If the decoder doesn't have |does_selection|, - * it's seen as taking anything. - */ - && (decoder->does_selection == NULL - || decoder->does_selection(provctx, data->ctx->selection)) - && (decoderctx = decoder->newctx(provctx)) != NULL - && (di = ossl_decoder_instance_new(decoder, decoderctx)) != NULL) { - /* If successful so far, don't free these directly */ - decoderctx = NULL; - - 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 */ - } + if (OSSL_DECODER_is_a(decoder, name)) { + void *decoderctx = NULL; + OSSL_DECODER_INSTANCE *di = NULL; - /* Free what can be freed */ - ossl_decoder_instance_free(di); - decoder->freectx(decoderctx); + if ((decoderctx = decoder->newctx(provctx)) == NULL) { + data->error_occurred = 1; + return; + } + if ((di = ossl_decoder_instance_new(decoder, decoderctx)) == NULL) { + decoder->freectx(decoderctx); + data->error_occurred = 1; + return; + } + + if (!decoder_check_input_structure(data->ctx, di)) { + ossl_decoder_instance_free(di); + /* Not a fatal error. Just return */ + return; + } + if (!ossl_decoder_ctx_add_decoder_inst(data->ctx, di)) { + ossl_decoder_instance_free(di); + data->error_occurred = 1; + return; + } + + /* Success */ + return; + } } - data->error_occurred = 0; /* All is good now */ + /* Decoder not suitable - but not a fatal error */ + data->error_occurred = 0; } int ossl_decoder_ctx_setup_for_pkey(OSSL_DECODER_CTX *ctx,