From: Viktor Dukhovni Date: Tue, 25 Feb 2025 07:17:02 +0000 (+1100) Subject: Use better data type info in decoders X-Git-Tag: openssl-3.5.0-alpha1~56 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=44a64029c3c5357c5b32dbe45b7f55ab7866ad3f;p=thirdparty%2Fopenssl.git Use better data type info in decoders The decoders in some cases failed to capture or propagate information about what is being decoded, causing more work happen to try unrelated decoders as a fallback. We now try harder to keep track of the expected object (private key or public key, if known), and the algorithm determined from the OID of a PKCS8 object or SPKI. This leads in many cases to fewer decoder invocations. With so many more algorithms now, trying every decoder is increasingly best avoided. Reviewed-by: Dmitry Belyavskiy Reviewed-by: Tim Hudson (Merged from https://github.com/openssl/openssl/pull/26892) --- diff --git a/crypto/encode_decode/decoder_lib.c b/crypto/encode_decode/decoder_lib.c index b43581efe5b..32ec18594f6 100644 --- a/crypto/encode_decode/decoder_lib.c +++ b/crypto/encode_decode/decoder_lib.c @@ -448,6 +448,20 @@ static void collect_extra_decoder(OSSL_DECODER *decoder, void *arg) if ((decoderctx = decoder->newctx(provctx)) == NULL) return; + if (data->ctx->input_structure != NULL) { + OSSL_PARAM params[] = { OSSL_PARAM_END, OSSL_PARAM_END }; + const char *str = data->ctx->input_structure; + + params[0] = + OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_STRUCTURE, + (char *)str, 0); + if (decoder->set_ctx_params != NULL + && !decoder->set_ctx_params(decoderctx, params)) { + decoder->freectx(decoderctx); + return; + } + } + if ((di = ossl_decoder_instance_new(decoder, decoderctx)) == NULL) { decoder->freectx(decoderctx); return; @@ -733,6 +747,8 @@ static int decoder_process(const OSSL_PARAM params[], void *arg) struct decoder_process_data_st new_data; const char *data_type = NULL; const char *data_structure = NULL; + /* Saved to restore on return, mutated in PEM->DER transition. */ + const char *start_input_type = ctx->start_input_type; /* * This is an indicator up the call stack that something was indeed @@ -823,6 +839,23 @@ static int decoder_process(const OSSL_PARAM params[], void *arg) if (p != NULL && !OSSL_PARAM_get_utf8_string_ptr(p, &data_structure)) goto end; + /* Get the new input type if there is one */ + p = OSSL_PARAM_locate_const(params, OSSL_OBJECT_PARAM_INPUT_TYPE); + if (p != NULL) { + if (!OSSL_PARAM_get_utf8_string_ptr(p, &ctx->start_input_type)) + goto end; + /* + * When switching PKCS8 from PEM to DER we decrypt the data if needed + * and then determine the algorithm OID. Likewise, with SPKI, only + * this time sans decryption. + */ + if (ctx->input_structure != NULL + && (OPENSSL_strcasecmp(ctx->input_structure, "SubjectPublicKeyInfo") == 0 + || OPENSSL_strcasecmp(data_structure, "PrivateKeyInfo") == 0 + || OPENSSL_strcasecmp(ctx->input_structure, "PrivateKeyInfo") == 0)) + data->flag_input_structure_checked = 1; + } + /* * If the data structure is "type-specific" and the data type is * given, we drop the data structure. The reasoning is that the @@ -1043,5 +1076,6 @@ static int decoder_process(const OSSL_PARAM params[], void *arg) end: ossl_core_bio_free(cbio); BIO_free(new_data.bio); + ctx->start_input_type = start_input_type; return ok; } diff --git a/crypto/encode_decode/decoder_pkey.c b/crypto/encode_decode/decoder_pkey.c index 824bfb050dd..def735b874a 100644 --- a/crypto/encode_decode/decoder_pkey.c +++ b/crypto/encode_decode/decoder_pkey.c @@ -752,20 +752,27 @@ OSSL_DECODER_CTX_new_for_pkey(EVP_PKEY **pkey, { OSSL_DECODER_CTX *ctx = NULL; OSSL_PARAM decoder_params[] = { + OSSL_PARAM_END, OSSL_PARAM_END, OSSL_PARAM_END }; DECODER_CACHE *cache = ossl_lib_ctx_get_data(libctx, OSSL_LIB_CTX_DECODER_CACHE_INDEX); DECODER_CACHE_ENTRY cacheent, *res, *newcache = NULL; + int i = 0; if (cache == NULL) { ERR_raise(ERR_LIB_OSSL_DECODER, ERR_R_OSSL_DECODER_LIB); return NULL; } + if (input_structure != NULL) + decoder_params[i++] = + OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_STRUCTURE, + (char *)input_structure, 0); if (propquery != NULL) - decoder_params[0] = OSSL_PARAM_construct_utf8_string(OSSL_DECODER_PARAM_PROPERTIES, - (char *)propquery, 0); + decoder_params[i++] = + OSSL_PARAM_construct_utf8_string(OSSL_DECODER_PARAM_PROPERTIES, + (char *)propquery, 0); /* It is safe to cast away the const here */ cacheent.input_type = (char *)input_type; diff --git a/include/internal/sizes.h b/include/internal/sizes.h index f6496c81826..f7b68361cc1 100644 --- a/include/internal/sizes.h +++ b/include/internal/sizes.h @@ -18,5 +18,6 @@ # define OSSL_MAX_NAME_SIZE 50 /* Algorithm name */ # define OSSL_MAX_PROPQUERY_SIZE 256 /* Property query strings */ # define OSSL_MAX_ALGORITHM_ID_SIZE 256 /* AlgorithmIdentifier DER */ +# define OSSL_MAX_CODEC_STRUCT_SIZE 32 /* DATA_STRUCTURE name */ #endif diff --git a/providers/decoders.inc b/providers/decoders.inc index ffae575c618..8354cfa373f 100644 --- a/providers/decoders.inc +++ b/providers/decoders.inc @@ -127,13 +127,21 @@ DECODER_w_structure("ML-DSA-87", der, SubjectPublicKeyInfo, ml_dsa_87, yes), /* * A decoder that takes a SubjectPublicKeyInfo and figures out the types of key - * that it contains. The output is the same SubjectPublicKeyInfo + * that it contains. The output is the same SubjectPublicKeyInfo. */ DECODER_w_structure("DER", der, SubjectPublicKeyInfo, der, yes), +/* + * General-purpose PEM to DER decoder. When the user-specified data structure + * is a possibly encrypted PKCS#8 PrivateKeyInfo or a SubjectPublicKeyInfo + * public key, or interest in such a key is indicated via the "selection", and + * the current object is of that type, decodes the PKCS#8 or SPKI identifying + * algorithm name or OID, and delegates further decoding in DER form to the + * identified algorithm. + */ DECODER("DER", pem, der, yes), /* - * A decoder that recognises PKCS#8 EncryptedPrivateKeyInfo structure - * and decrypts it, passing on the unencrypted PrivateKeyInfo in DER - * form to the next decoder. + * A decoder that recognises PKCS#8 EncryptedPrivateKeyInfo structure and + * decrypts it, obtaining the algorithm name or OID, and delegates the + * unencrypted PrivateKeyInfo in DER form to the identified algorithm. */ DECODER_w_structure("DER", der, EncryptedPrivateKeyInfo, der, yes), diff --git a/providers/implementations/encode_decode/decode_epki2pki.c b/providers/implementations/encode_decode/decode_epki2pki.c index 37d9bd1858f..7de9620a52d 100644 --- a/providers/implementations/encode_decode/decode_epki2pki.c +++ b/providers/implementations/encode_decode/decode_epki2pki.c @@ -20,6 +20,7 @@ #include "internal/asn1.h" #include "internal/sizes.h" #include "prov/bio.h" +#include "prov/decoders.h" #include "prov/implementations.h" #include "endecoder_local.h" @@ -87,11 +88,7 @@ static int epki2pki_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, struct epki2pki_ctx_st *ctx = vctx; BUF_MEM *mem = NULL; unsigned char *der = NULL; - const unsigned char *pder = NULL; long der_len = 0; - X509_SIG *p8 = NULL; - PKCS8_PRIV_KEY_INFO *p8inf = NULL; - const X509_ALGOR *alg = NULL; BIO *in = ossl_bio_new_from_core_bio(ctx->provctx, cin); int ok = 0; @@ -105,11 +102,29 @@ static int epki2pki_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, if (!ok) return 1; - pder = der = (unsigned char *)mem->data; + der = (unsigned char *)mem->data; der_len = (long)mem->length; OPENSSL_free(mem); - ok = 1; /* Assume good */ + ok = ossl_epki2pki_der_decode(der, der_len, selection, data_cb, data_cbarg, + pw_cb, pw_cbarg, PROV_LIBCTX_OF(ctx->provctx), + ctx->propq); + OPENSSL_free(der); + return ok; +} + +int ossl_epki2pki_der_decode(unsigned char *der, long der_len, int selection, + OSSL_CALLBACK *data_cb, void *data_cbarg, + OSSL_PASSPHRASE_CALLBACK *pw_cb, void *pw_cbarg, + OSSL_LIB_CTX *libctx, const char *propq) +{ + const unsigned char *pder = der; + unsigned char *new_der = NULL; + X509_SIG *p8 = NULL; + PKCS8_PRIV_KEY_INFO *p8inf = NULL; + const X509_ALGOR *alg = NULL; + int ok = 1; /* Assume good */ + ERR_set_mark(); if ((p8 = d2i_X509_SIG(NULL, &pder, der_len)) != NULL) { char pbuf[1024]; @@ -122,18 +137,15 @@ static int epki2pki_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, ok = 0; } else { const ASN1_OCTET_STRING *oct; - unsigned char *new_der = NULL; int new_der_len = 0; X509_SIG_get0(p8, &alg, &oct); if (!PKCS12_pbe_crypt_ex(alg, pbuf, plen, oct->data, oct->length, &new_der, &new_der_len, 0, - PROV_LIBCTX_OF(ctx->provctx), - ctx->propq)) { + libctx, propq)) { ok = 0; } else { - OPENSSL_free(der); der = new_der; der_len = new_der_len; } @@ -155,13 +167,15 @@ static int epki2pki_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, * pass all the applicable data to the callback. */ char keytype[OSSL_MAX_NAME_SIZE]; - OSSL_PARAM params[5], *p = params; + OSSL_PARAM params[6], *p = params; int objtype = OSSL_OBJECT_PKEY; OBJ_obj2txt(keytype, sizeof(keytype), alg->algorithm, 0); *p++ = OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_TYPE, keytype, 0); + *p++ = OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_INPUT_TYPE, + "DER", 0); *p++ = OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_STRUCTURE, "PrivateKeyInfo", 0); *p++ = OSSL_PARAM_construct_octet_string(OSSL_OBJECT_PARAM_DATA, @@ -172,7 +186,7 @@ static int epki2pki_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, ok = data_cb(params, data_cbarg); } PKCS8_PRIV_KEY_INFO_free(p8inf); - OPENSSL_free(der); + OPENSSL_free(new_der); return ok; } diff --git a/providers/implementations/encode_decode/decode_pem2der.c b/providers/implementations/encode_decode/decode_pem2der.c index ea6eb7f9610..1c23a57c88a 100644 --- a/providers/implementations/encode_decode/decode_pem2der.c +++ b/providers/implementations/encode_decode/decode_pem2der.c @@ -24,7 +24,9 @@ #include #include #include "internal/nelem.h" +#include "internal/sizes.h" #include "prov/bio.h" +#include "prov/decoders.h" #include "prov/implementations.h" #include "endecoder_local.h" @@ -52,6 +54,8 @@ static OSSL_FUNC_decoder_decode_fn pem2der_decode; */ struct pem2der_ctx_st { PROV_CTX *provctx; + char data_structure[OSSL_MAX_CODEC_STRUCT_SIZE]; + char propq[OSSL_MAX_PROPQUERY_SIZE]; }; static void *pem2der_newctx(void *provctx) @@ -70,6 +74,37 @@ static void pem2der_freectx(void *vctx) OPENSSL_free(ctx); } +static const OSSL_PARAM *pem2der_settable_ctx_params(ossl_unused void *provctx) +{ + static const OSSL_PARAM settables[] = { + OSSL_PARAM_utf8_string(OSSL_DECODER_PARAM_PROPERTIES, NULL, 0), + OSSL_PARAM_utf8_string(OSSL_OBJECT_PARAM_DATA_STRUCTURE, NULL, 0), + OSSL_PARAM_END + }; + return settables; +} + +static int pem2der_set_ctx_params(void *vctx, const OSSL_PARAM params[]) +{ + struct pem2der_ctx_st *ctx = vctx; + const OSSL_PARAM *p; + char *str; + + p = OSSL_PARAM_locate_const(params, OSSL_DECODER_PARAM_PROPERTIES); + str = ctx->propq; + if (p != NULL + && !OSSL_PARAM_get_utf8_string(p, &str, sizeof(ctx->propq))) + return 0; + + p = OSSL_PARAM_locate_const(params, OSSL_OBJECT_PARAM_DATA_STRUCTURE); + str = ctx->data_structure; + if (p != NULL + && !OSSL_PARAM_get_utf8_string(p, &str, sizeof(ctx->data_structure))) + return 0; + + return 1; +} + /* pem_password_cb compatible function */ struct pem2der_pass_data_st { OSSL_PASSPHRASE_CALLBACK *cb; @@ -88,10 +123,6 @@ static int pem2der_pass_helper(char *buf, int num, int w, void *data) return (int)plen; } -/* - * The selection parameter in pem2der_decode() is not used by this function - * because it's not relevant just to decode PEM to DER. - */ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, OSSL_CALLBACK *data_cb, void *data_cbarg, OSSL_PASSPHRASE_CALLBACK *pw_cb, void *pw_cbarg) @@ -109,8 +140,9 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, /* PKCS#8 and SubjectPublicKeyInfo */ { PEM_STRING_PKCS8, OSSL_OBJECT_PKEY, NULL, "EncryptedPrivateKeyInfo" }, { PEM_STRING_PKCS8INF, OSSL_OBJECT_PKEY, NULL, "PrivateKeyInfo" }, +#define PKCS8_LAST_IDX 1 { PEM_STRING_PUBLIC, OSSL_OBJECT_PKEY, NULL, "SubjectPublicKeyInfo" }, - +#define SPKI_LAST_IDX 2 /* Our set of type specific PEM types */ { PEM_STRING_DHPARAMS, OSSL_OBJECT_PKEY, "DH", "type-specific" }, { PEM_STRING_DHXPARAMS, OSSL_OBJECT_PKEY, "X9.42 DH", "type-specific" }, @@ -183,6 +215,31 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, char *data_type = (char *)pem_name_map[i].data_type; char *data_structure = (char *)pem_name_map[i].data_structure; + /* + * Since this may perform decryption, we need to check the selection to + * avoid password prompts for objects of no interest. + */ + if (i <= PKCS8_LAST_IDX + && ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) + || OPENSSL_strcasecmp(ctx->data_structure, "EncryptedPrivateKeyInfo") == 0 + || OPENSSL_strcasecmp(ctx->data_structure, "PrivateKeyInfo") == 0)) { + ok = ossl_epki2pki_der_decode(der, der_len, selection, data_cb, + data_cbarg, pw_cb, pw_cbarg, + PROV_LIBCTX_OF(ctx->provctx), + ctx->propq); + goto end; + } + + if (i <= SPKI_LAST_IDX + && ((selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) + || OPENSSL_strcasecmp(ctx->data_structure, "SubjectPublicKeyInfo") == 0)) { + ok = ossl_spki2typespki_der_decode(der, der_len, selection, data_cb, + data_cbarg, pw_cb, pw_cbarg, + PROV_LIBCTX_OF(ctx->provctx), + ctx->propq); + goto end; + } + objtype = pem_name_map[i].object_type; if (data_type != NULL) *p++ = @@ -216,5 +273,9 @@ const OSSL_DISPATCH ossl_pem_to_der_decoder_functions[] = { { OSSL_FUNC_DECODER_NEWCTX, (void (*)(void))pem2der_newctx }, { OSSL_FUNC_DECODER_FREECTX, (void (*)(void))pem2der_freectx }, { OSSL_FUNC_DECODER_DECODE, (void (*)(void))pem2der_decode }, + { OSSL_FUNC_DECODER_SETTABLE_CTX_PARAMS, + (void (*)(void))pem2der_settable_ctx_params }, + { OSSL_FUNC_DECODER_SET_CTX_PARAMS, + (void (*)(void))pem2der_set_ctx_params }, OSSL_DISPATCH_END }; diff --git a/providers/implementations/encode_decode/decode_spki2typespki.c b/providers/implementations/encode_decode/decode_spki2typespki.c index 7074be93d6b..9e0a6143e79 100644 --- a/providers/implementations/encode_decode/decode_spki2typespki.c +++ b/providers/implementations/encode_decode/decode_spki2typespki.c @@ -17,6 +17,7 @@ #include "crypto/x509.h" #include "crypto/ec.h" #include "prov/bio.h" +#include "prov/decoders.h" #include "prov/implementations.h" #include "endecoder_local.h" @@ -78,22 +79,35 @@ static int spki2typespki_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, OSSL_PASSPHRASE_CALLBACK *pw_cb, void *pw_cbarg) { struct spki2typespki_ctx_st *ctx = vctx; - unsigned char *der, *derp; + unsigned char *der; long len; int ok = 0; - int objtype = OSSL_OBJECT_PKEY; + + if (!ossl_read_der(ctx->provctx, cin, &der, &len)) + return 1; + + ok = ossl_spki2typespki_der_decode(der, len, selection, data_cb, data_cbarg, + pw_cb, pw_cbarg, + PROV_LIBCTX_OF(ctx->provctx), ctx->propq); + OPENSSL_free(der); + return ok; +} + +int ossl_spki2typespki_der_decode(unsigned char *der, long len, int selection, + OSSL_CALLBACK *data_cb, void *data_cbarg, + OSSL_PASSPHRASE_CALLBACK *pw_cb, void *pw_cbarg, + OSSL_LIB_CTX *libctx, const char *propq) +{ + const unsigned char *derp = der; X509_PUBKEY *xpub = NULL; X509_ALGOR *algor = NULL; const ASN1_OBJECT *oid = NULL; char dataname[OSSL_MAX_NAME_SIZE]; - OSSL_PARAM params[5], *p = params; + OSSL_PARAM params[6], *p = params; + int objtype = OSSL_OBJECT_PKEY; + int ok = 0; - if (!ossl_read_der(ctx->provctx, cin, &der, &len)) - return 1; - derp = der; - xpub = ossl_d2i_X509_PUBKEY_INTERNAL((const unsigned char **)&derp, len, - PROV_LIBCTX_OF(ctx->provctx), - ctx->propq); + xpub = ossl_d2i_X509_PUBKEY_INTERNAL(&derp, len, libctx, propq); if (xpub == NULL) { /* We return "empty handed". This is not an error. */ @@ -122,6 +136,9 @@ static int spki2typespki_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_TYPE, dataname, 0); + *p++ = OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_INPUT_TYPE, + "DER", 0); + *p++ = OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_STRUCTURE, "SubjectPublicKeyInfo", @@ -137,7 +154,6 @@ static int spki2typespki_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, end: ossl_X509_PUBKEY_INTERNAL_free(xpub); - OPENSSL_free(der); return ok; } diff --git a/providers/implementations/include/prov/decoders.h b/providers/implementations/include/prov/decoders.h new file mode 100644 index 00000000000..654efee607d --- /dev/null +++ b/providers/implementations/include/prov/decoders.h @@ -0,0 +1,20 @@ +/* + * Copyright 2025 The OpenSSL Project Authors. All Rights Reserved. + * + * Licensed under the Apache License 2.0 (the "License"). You may not use + * this file except in compliance with the License. You can obtain a copy + * in the file LICENSE in the source distribution or at + * https://www.openssl.org/source/license.html + */ + +#include + +int ossl_epki2pki_der_decode(unsigned char *der, long der_len, int selection, + OSSL_CALLBACK *data_cb, void *data_cbarg, + OSSL_PASSPHRASE_CALLBACK *pw_cb, void *pw_cbarg, + OSSL_LIB_CTX *libctx, const char *propq); + +int ossl_spki2typespki_der_decode(unsigned char *der, long len, int selection, + OSSL_CALLBACK *data_cb, void *data_cbarg, + OSSL_PASSPHRASE_CALLBACK *pw_cb, void *pw_cbarg, + OSSL_LIB_CTX *libctx, const char *propq); diff --git a/providers/implementations/storemgmt/file_store.c b/providers/implementations/storemgmt/file_store.c index eea8f2a0f85..b1b32b56f61 100644 --- a/providers/implementations/storemgmt/file_store.c +++ b/providers/implementations/storemgmt/file_store.c @@ -439,6 +439,29 @@ static int file_setup_decoders(struct file_ctx_st *ctx) * for this load. */ switch (ctx->expected_type) { + case OSSL_STORE_INFO_PUBKEY: + if (!OSSL_DECODER_CTX_set_input_structure(ctx->_.file.decoderctx, + "SubjectPublicKeyInfo")) { + ERR_raise(ERR_LIB_PROV, ERR_R_OSSL_DECODER_LIB); + goto err; + } + break; + case OSSL_STORE_INFO_PKEY: + /* + * The user's OSSL_STORE_INFO_PKEY covers PKCS#8, whether encrypted + * or not. The decoder will figure out whether decryption is + * applicable and fall back as necessary. We just need to indicate + * that it is OK to try and encrypt, which may involve a password + * prompt, so not done unless the data type is explicit, as we + * might then get a password prompt for a key when reading only + * certs from a file. + */ + if (!OSSL_DECODER_CTX_set_input_structure(ctx->_.file.decoderctx, + "EncryptedPrivateKeyInfo")) { + ERR_raise(ERR_LIB_PROV, ERR_R_OSSL_DECODER_LIB); + goto err; + } + break; case OSSL_STORE_INFO_CERT: if (!OSSL_DECODER_CTX_set_input_structure(ctx->_.file.decoderctx, "Certificate")) { diff --git a/util/perl/OpenSSL/paramnames.pm b/util/perl/OpenSSL/paramnames.pm index 9d536f3f559..2b3abb19d78 100644 --- a/util/perl/OpenSSL/paramnames.pm +++ b/util/perl/OpenSSL/paramnames.pm @@ -80,6 +80,7 @@ my %params = ( 'OBJECT_PARAM_REFERENCE' => "reference",# OCTET_STRING 'OBJECT_PARAM_DATA' => "data",# OCTET_STRING or UTF8_STRING 'OBJECT_PARAM_DESC' => "desc", # UTF8_STRING + 'OBJECT_PARAM_INPUT_TYPE' => "input-type", # UTF8_STRING # Algorithm parameters # If "engine",or "properties",are specified, they should always be paired