packages/openssl
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
openssl/backport-Decoder-resolution-performance-optimizations.patch

567 lines
22 KiB
Diff
Raw Normal View History

backport patch for performance improvements
2024-11-27 16:24:34 +08:00
From 4a1108eb5906cd3cf47a3f70bd58722dbe2023a4 Mon Sep 17 00:00:00 2001
From: Hugo Landau <hlandau@openssl.org>
Date: Thu, 17 Mar 2022 17:29:22 +0000
Subject: [PATCH] Decoder resolution performance optimizations
This refactors decoder functionality to reduce calls to
OSSL_DECODER_is_a / EVP_KEYMGMT_is_a, which are substantial bottlenecks
in the performance of repeated decode operations (see #15199).
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17921)
(cherry picked from commit 247554458435eaab175cdc9d36878158b9eb6f6e)
---
crypto/encode_decode/decoder_lib.c | 20 +-
crypto/encode_decode/decoder_meth.c | 18 ++
crypto/encode_decode/decoder_pkey.c | 323 +++++++++++++++------------
crypto/encode_decode/encoder_local.h | 4 +
4 files changed, 222 insertions(+), 143 deletions(-)
diff --git a/crypto/encode_decode/decoder_lib.c b/crypto/encode_decode/decoder_lib.c
index 817f335453..8863c316d6 100644
--- a/crypto/encode_decode/decoder_lib.c
+++ b/crypto/encode_decode/decoder_lib.c
@@ -18,6 +18,7 @@
#include <openssl/trace.h>
#include "internal/bio.h"
#include "internal/provider.h"
+#include "internal/namemap.h"
#include "crypto/decoder.h"
#include "encoder_local.h"
#include "e_os.h"
@@ -241,6 +242,7 @@ OSSL_DECODER_INSTANCE *ossl_decoder_instance_new(OSSL_DECODER *decoder,
/* The "input" property is mandatory */
prop = ossl_property_find_property(props, libctx, "input");
decoder_inst->input_type = ossl_property_get_string_value(libctx, prop);
+ decoder_inst->input_type_id = 0;
if (decoder_inst->input_type == NULL) {
ERR_raise_data(ERR_LIB_OSSL_DECODER, ERR_R_INVALID_PROPERTY_DEFINITION,
"the mandatory 'input' property is missing "
@@ -343,6 +345,8 @@ int OSSL_DECODER_CTX_add_decoder(OSSL_DECODER_CTX *ctx, OSSL_DECODER *decoder)
struct collect_extra_decoder_data_st {
OSSL_DECODER_CTX *ctx;
const char *output_type;
+ int output_type_id;
+
/*
* 0 to check that the decoder's input type is the same as the decoder name
* 1 to check that the decoder's input type differs from the decoder name
@@ -370,7 +374,7 @@ static void collect_extra_decoder(OSSL_DECODER *decoder, void *arg)
const OSSL_PROVIDER *prov = OSSL_DECODER_get0_provider(decoder);
void *provctx = OSSL_PROVIDER_get0_provider_ctx(prov);
- if (OSSL_DECODER_is_a(decoder, data->output_type)) {
+ if (ossl_decoder_fast_is_a(decoder, data->output_type, &data->output_type_id)) {
void *decoderctx = NULL;
OSSL_DECODER_INSTANCE *di = NULL;
@@ -413,8 +417,9 @@ static void collect_extra_decoder(OSSL_DECODER *decoder, void *arg)
switch (data->type_check) {
case IS_SAME:
/* If it differs, this is not a decoder to add for now. */
- if (!OSSL_DECODER_is_a(decoder,
- OSSL_DECODER_INSTANCE_get_input_type(di))) {
+ if (!ossl_decoder_fast_is_a(decoder,
+ OSSL_DECODER_INSTANCE_get_input_type(di),
+ &di->input_type_id)) {
ossl_decoder_instance_free(di);
OSSL_TRACE_BEGIN(DECODER) {
BIO_printf(trc_out,
@@ -425,8 +430,9 @@ static void collect_extra_decoder(OSSL_DECODER *decoder, void *arg)
break;
case IS_DIFFERENT:
/* If it's the same, this is not a decoder to add for now. */
- if (OSSL_DECODER_is_a(decoder,
- OSSL_DECODER_INSTANCE_get_input_type(di))) {
+ if (ossl_decoder_fast_is_a(decoder,
+ OSSL_DECODER_INSTANCE_get_input_type(di),
+ &di->input_type_id)) {
ossl_decoder_instance_free(di);
OSSL_TRACE_BEGIN(DECODER) {
BIO_printf(trc_out,
@@ -534,6 +540,7 @@ int OSSL_DECODER_CTX_add_extra(OSSL_DECODER_CTX *ctx,
data.output_type
= OSSL_DECODER_INSTANCE_get_input_type(decoder_inst);
+ data.output_type_id = 0;
for (j = 0; j < numdecoders; j++)
collect_extra_decoder(sk_OSSL_DECODER_value(skdecoders, j),
@@ -867,7 +874,8 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
* |new_input_type| holds the value of the "input-type" parameter
* for the decoder we're currently considering.
*/
- if (decoder != NULL && !OSSL_DECODER_is_a(decoder, new_input_type)) {
+ if (decoder != NULL && !ossl_decoder_fast_is_a(decoder, new_input_type,
+ &new_decoder_inst->input_type_id)) {
OSSL_TRACE_BEGIN(DECODER) {
BIO_printf(trc_out,
"(ctx %p) %s [%u] the input type doesn't match the name of the previous decoder (%p), skipping...\n",
diff --git a/crypto/encode_decode/decoder_meth.c b/crypto/encode_decode/decoder_meth.c
index 56899a9269..496fbe3320 100644
--- a/crypto/encode_decode/decoder_meth.c
+++ b/crypto/encode_decode/decoder_meth.c
@@ -558,6 +558,24 @@ int OSSL_DECODER_is_a(const OSSL_DECODER *decoder, const char *name)
return 0;
}
+static int resolve_name(OSSL_DECODER *decoder, const char *name)
+{
+ OSSL_LIB_CTX *libctx = ossl_provider_libctx(decoder->base.prov);
+ OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
+
+ return ossl_namemap_name2num(namemap, name);
+}
+
+int ossl_decoder_fast_is_a(OSSL_DECODER *decoder, const char *name, int *id_cache)
+{
+ int id = *id_cache;
+
+ if (id <= 0)
+ *id_cache = id = resolve_name(decoder, name);
+
+ return id > 0 && ossl_decoder_get_number(decoder) == id;
+}
+
struct do_one_data_st {
void (*user_fn)(OSSL_DECODER *decoder, void *arg);
void *user_arg;
diff --git a/crypto/encode_decode/decoder_pkey.c b/crypto/encode_decode/decoder_pkey.c
index ed10bb1cee..00d3d48420 100644
--- a/crypto/encode_decode/decoder_pkey.c
+++ b/crypto/encode_decode/decoder_pkey.c
@@ -17,7 +17,9 @@
#include <openssl/trace.h>
#include "crypto/evp.h"
#include "crypto/decoder.h"
+#include "crypto/evp/evp_local.h"
#include "encoder_local.h"
+#include "internal/namemap.h"
int OSSL_DECODER_CTX_set_passphrase(OSSL_DECODER_CTX *ctx,
const unsigned char *kstr,
@@ -195,53 +197,83 @@ static void decoder_clean_pkey_construct_arg(void *construct_data)
}
}
-static void collect_name(const char *name, void *arg)
-{
- STACK_OF(OPENSSL_CSTRING) *names = arg;
+struct collect_data_st {
+ OSSL_LIB_CTX *libctx;
+ OSSL_DECODER_CTX *ctx;
- sk_OPENSSL_CSTRING_push(names, name);
-}
+ const char *keytype; /* the keytype requested, if any */
+ int keytype_id; /* if keytype_resolved is set, keymgmt name_id; else 0 */
+ int sm2_id; /* if keytype_resolved is set and EC, SM2 name_id; else 0 */
+ int total; /* number of matching results */
+ char error_occurred;
+ char keytype_resolved;
-static void collect_keymgmt(EVP_KEYMGMT *keymgmt, void *arg)
+ STACK_OF(EVP_KEYMGMT) *keymgmts;
+};
+
+static void collect_decoder_keymgmt(EVP_KEYMGMT *keymgmt, OSSL_DECODER *decoder,
+ void *provctx, struct collect_data_st *data)
{
- STACK_OF(EVP_KEYMGMT) *keymgmts = arg;
+ void *decoderctx = NULL;
+ OSSL_DECODER_INSTANCE *di = NULL;
+
+ /*
+ * We already checked the EVP_KEYMGMT is applicable in check_keymgmt so we
+ * don't check it again here.
+ */
- if (!EVP_KEYMGMT_up_ref(keymgmt) /* ref++ */)
+ if (keymgmt->name_id != decoder->base.id)
+ /* Mismatch is not an error, continue. */
return;
- if (sk_EVP_KEYMGMT_push(keymgmts, keymgmt) <= 0) {
- EVP_KEYMGMT_free(keymgmt); /* ref-- */
+
+ if ((decoderctx = decoder->newctx(provctx)) == NULL) {
+ data->error_occurred = 1;
return;
}
-}
-struct collect_decoder_data_st {
- STACK_OF(OPENSSL_CSTRING) *names;
- OSSL_DECODER_CTX *ctx;
+ if ((di = ossl_decoder_instance_new(decoder, decoderctx)) == NULL) {
+ decoder->freectx(decoderctx);
+ data->error_occurred = 1;
+ return;
+ }
- int total;
- unsigned int error_occurred:1;
-};
+ OSSL_TRACE_BEGIN(DECODER) {
+ BIO_printf(trc_out,
+ "(ctx %p) Checking out decoder %p:\n"
+ " %s with %s\n",
+ (void *)data->ctx, (void *)decoder,
+ OSSL_DECODER_get0_name(decoder),
+ OSSL_DECODER_get0_properties(decoder));
+ } OSSL_TRACE_END(DECODER);
+
+ if (!ossl_decoder_ctx_add_decoder_inst(data->ctx, di)) {
+ ossl_decoder_instance_free(di);
+ data->error_occurred = 1;
+ return;
+ }
+
+ ++data->total;
+}
static void collect_decoder(OSSL_DECODER *decoder, void *arg)
{
- struct collect_decoder_data_st *data = arg;
+ struct collect_data_st *data = arg;
+ STACK_OF(EVP_KEYMGMT) *keymgmts = data->keymgmts;
size_t i, end_i;
- const OSSL_PROVIDER *prov = OSSL_DECODER_get0_provider(decoder);
- void *provctx = OSSL_PROVIDER_get0_provider_ctx(prov);
+ EVP_KEYMGMT *keymgmt;
+ const OSSL_PROVIDER *prov;
+ void *provctx;
if (data->error_occurred)
return;
- if (data->names == NULL) {
- data->error_occurred = 1;
- return;
- }
+ prov = OSSL_DECODER_get0_provider(decoder);
+ provctx = OSSL_PROVIDER_get0_provider_ctx(prov);
/*
- * 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.
+ * Either the caller didn't give us 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))
@@ -256,68 +288,101 @@ static void collect_decoder(OSSL_DECODER *decoder, void *arg)
OSSL_DECODER_get0_properties(decoder));
} OSSL_TRACE_END(DECODER);
- 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);
-
- if (OSSL_DECODER_is_a(decoder, name)) {
- void *decoderctx = NULL;
- OSSL_DECODER_INSTANCE *di = NULL;
-
- 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;
- }
-
- OSSL_TRACE_BEGIN(DECODER) {
- BIO_printf(trc_out,
- "(ctx %p) Checking out decoder %p:\n"
- " %s with %s\n",
- (void *)data->ctx, (void *)decoder,
- OSSL_DECODER_get0_name(decoder),
- OSSL_DECODER_get0_properties(decoder));
- } OSSL_TRACE_END(DECODER);
-
- if (!ossl_decoder_ctx_add_decoder_inst(data->ctx, di)) {
- ossl_decoder_instance_free(di);
- data->error_occurred = 1;
- return;
- }
- data->total++;
-
- /* Success */
+ end_i = sk_EVP_KEYMGMT_num(keymgmts);
+ for (i = 0; i < end_i; ++i) {
+ keymgmt = sk_EVP_KEYMGMT_value(keymgmts, i);
+
+ collect_decoder_keymgmt(keymgmt, decoder, provctx, data);
+ if (data->error_occurred)
return;
- }
+ }
+}
+
+/*
+ * Is this EVP_KEYMGMT applicable given the key type given in the call to
+ * ossl_decoder_ctx_setup_for_pkey (if any)?
+ */
+static int check_keymgmt(EVP_KEYMGMT *keymgmt, struct collect_data_st *data)
+{
+ /* If no keytype was specified, everything matches. */
+ if (data->keytype == NULL)
+ return 1;
+
+ if (!data->keytype_resolved) {
+ /* We haven't cached the IDs from the keytype string yet. */
+ OSSL_NAMEMAP *namemap = ossl_namemap_stored(data->libctx);
+ data->keytype_id = ossl_namemap_name2num(namemap, data->keytype);
+
+ /*
+ * If keytype is a value ambiguously used for both EC and SM2,
+ * collect the ID for SM2 as well.
+ */
+ if (data->keytype_id != 0
+ && (strcmp(data->keytype, "id-ecPublicKey") == 0
+ || strcmp(data->keytype, "1.2.840.10045.2.1") == 0))
+ data->sm2_id = ossl_namemap_name2num(namemap, "SM2");
+
+ /*
+ * If keytype_id is zero the name was not found, but we still
+ * set keytype_resolved to avoid trying all this again.
+ */
+ data->keytype_resolved = 1;
}
- /* Decoder not suitable - but not a fatal error */
- data->error_occurred = 0;
+ /* Specified keytype could not be resolved, so nothing matches. */
+ if (data->keytype_id == 0)
+ return 0;
+
+ /* Does not match the keytype specified, so skip. */
+ if (keymgmt->name_id != data->keytype_id
+ && keymgmt->name_id != data->sm2_id)
+ return 0;
+
+ return 1;
}
+static void collect_keymgmt(EVP_KEYMGMT *keymgmt, void *arg)
+{
+ struct collect_data_st *data = arg;
+
+ if (!check_keymgmt(keymgmt, data))
+ return;
+
+ /*
+ * We have to ref EVP_KEYMGMT here because in the success case,
+ * data->keymgmts is referenced by the constructor we register in the
+ * OSSL_DECODER_CTX. The registered cleanup function
+ * (decoder_clean_pkey_construct_arg) unrefs every element of the stack and
+ * frees it.
+ */
+ if (!EVP_KEYMGMT_up_ref(keymgmt))
+ return;
+
+ if (sk_EVP_KEYMGMT_push(data->keymgmts, keymgmt) <= 0) {
+ EVP_KEYMGMT_free(keymgmt);
+ data->error_occurred = 1;
+ }
+}
+
+/*
+ * This function does the actual binding of decoders to the OSSL_DECODER_CTX. It
+ * searches for decoders matching 'keytype', which is a string like "RSA", "DH",
+ * etc. If 'keytype' is NULL, decoders for all keytypes are bound.
+ */
int ossl_decoder_ctx_setup_for_pkey(OSSL_DECODER_CTX *ctx,
EVP_PKEY **pkey, const char *keytype,
OSSL_LIB_CTX *libctx,
const char *propquery)
{
- struct decoder_pkey_data_st *process_data = NULL;
- STACK_OF(OPENSSL_CSTRING) *names = NULL;
- const char *input_type = ctx->start_input_type;
- const char *input_structure = ctx->input_structure;
int ok = 0;
- int isecoid = 0;
- int i, end;
-
- if (keytype != NULL
- && (strcmp(keytype, "id-ecPublicKey") == 0
- || strcmp(keytype, "1.2.840.10045.2.1") == 0))
- isecoid = 1;
+ struct decoder_pkey_data_st *process_data = NULL;
+ struct collect_data_st collect_data = { NULL };
+ STACK_OF(EVP_KEYMGMT) *keymgmts = NULL;
OSSL_TRACE_BEGIN(DECODER) {
+ const char *input_type = ctx->start_input_type;
+ const char *input_structure = ctx->input_structure;
+
BIO_printf(trc_out,
"(ctx %p) Looking for decoders producing %s%s%s%s%s%s\n",
(void *)ctx,
@@ -329,81 +394,67 @@ int ossl_decoder_ctx_setup_for_pkey(OSSL_DECODER_CTX *ctx,
input_structure != NULL ? input_structure : "");
} OSSL_TRACE_END(DECODER);
+ /* Allocate data. */
if ((process_data = OPENSSL_zalloc(sizeof(*process_data))) == NULL
|| (propquery != NULL
- && (process_data->propq = OPENSSL_strdup(propquery)) == NULL)
- || (process_data->keymgmts = sk_EVP_KEYMGMT_new_null()) == NULL
- || (names = sk_OPENSSL_CSTRING_new_null()) == NULL) {
+ && (process_data->propq = OPENSSL_strdup(propquery)) == NULL)) {
ERR_raise(ERR_LIB_OSSL_DECODER, ERR_R_MALLOC_FAILURE);
goto err;
}
- process_data->object = (void **)pkey;
- process_data->libctx = libctx;
+ /* Allocate our list of EVP_KEYMGMTs. */
+ keymgmts = sk_EVP_KEYMGMT_new_null();
+ if (keymgmts == NULL) {
+ ERR_raise(ERR_LIB_OSSL_DECODER, ERR_R_MALLOC_FAILURE);
+ goto err;
+ }
+
+ process_data->object = (void **)pkey;
+ process_data->libctx = libctx;
process_data->selection = ctx->selection;
+ process_data->keymgmts = keymgmts;
- /* First, find all keymgmts to form goals */
- EVP_KEYMGMT_do_all_provided(libctx, collect_keymgmt,
- process_data->keymgmts);
+ /*
+ * Enumerate all keymgmts into a stack.
+ *
+ * We could nest EVP_KEYMGMT_do_all_provided inside
+ * OSSL_DECODER_do_all_provided or vice versa but these functions become
+ * bottlenecks if called repeatedly, which is why we collect the
+ * EVP_KEYMGMTs into a stack here and call both functions only once.
+ *
+ * We resolve the keytype string to a name ID so we don't have to resolve it
+ * multiple times, avoiding repeated calls to EVP_KEYMGMT_is_a, which is a
+ * performance bottleneck. However, we do this lazily on the first call to
+ * collect_keymgmt made by EVP_KEYMGMT_do_all_provided, rather than do it
+ * upfront, as this ensures that the names for all loaded providers have
+ * been registered by the time we try to resolve the keytype string.
+ */
+ collect_data.ctx = ctx;
+ collect_data.libctx = libctx;
+ collect_data.keymgmts = keymgmts;
+ collect_data.keytype = keytype;
+ EVP_KEYMGMT_do_all_provided(libctx, collect_keymgmt, &collect_data);
- /* Then, we collect all the keymgmt names */
- end = sk_EVP_KEYMGMT_num(process_data->keymgmts);
- for (i = 0; i < end; i++) {
- EVP_KEYMGMT *keymgmt = sk_EVP_KEYMGMT_value(process_data->keymgmts, i);
+ if (collect_data.error_occurred)
+ goto err;
- /*
- * If the key type is given by the caller, we only use the matching
- * KEYMGMTs, otherwise we use them all.
- * We have to special case SM2 here because of its abuse of the EC OID.
- * The EC OID can be used to identify an EC key or an SM2 key - so if
- * we have seen that OID we try both key types
- */
- if (keytype == NULL
- || EVP_KEYMGMT_is_a(keymgmt, keytype)
- || (isecoid && EVP_KEYMGMT_is_a(keymgmt, "SM2"))) {
- if (!EVP_KEYMGMT_names_do_all(keymgmt, collect_name, names)) {
- ERR_raise(ERR_LIB_OSSL_DECODER, ERR_R_INTERNAL_ERROR);
- goto err;
- }
- }
- }
+ /* Enumerate all matching decoders. */
+ OSSL_DECODER_do_all_provided(libctx, collect_decoder, &collect_data);
+
+ if (collect_data.error_occurred)
+ goto err;
OSSL_TRACE_BEGIN(DECODER) {
- end = sk_OPENSSL_CSTRING_num(names);
BIO_printf(trc_out,
- " Found %d keytypes (possibly with duplicates)",
- end);
- for (i = 0; i < end; i++)
- BIO_printf(trc_out, "%s%s",
- i == 0 ? ": " : ", ",
- sk_OPENSSL_CSTRING_value(names, i));
- BIO_printf(trc_out, "\n");
+ "(ctx %p) Got %d decoders producing keys\n",
+ (void *)ctx, collect_data.total);
} OSSL_TRACE_END(DECODER);
/*
- * Finally, find all decoders that have any keymgmt of the collected
- * keymgmt names
+ * Finish initializing the decoder context. If one or more decoders matched
+ * above then the number of decoders attached to the OSSL_DECODER_CTX will
+ * be nonzero. Else nothing was found and we do nothing.
*/
- {
- struct collect_decoder_data_st collect_decoder_data = { NULL, };
-
- 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);
- names = NULL;
-
- if (collect_decoder_data.error_occurred)
- goto err;
-
- OSSL_TRACE_BEGIN(DECODER) {
- BIO_printf(trc_out,
- "(ctx %p) Got %d decoders producing keys\n",
- (void *)ctx, collect_decoder_data.total);
- } OSSL_TRACE_END(DECODER);
- }
-
if (OSSL_DECODER_CTX_get_num_decoders(ctx) != 0) {
if (!OSSL_DECODER_CTX_set_construct(ctx, decoder_construct_pkey)
|| !OSSL_DECODER_CTX_set_construct_data(ctx, process_data)
@@ -417,8 +468,6 @@ int ossl_decoder_ctx_setup_for_pkey(OSSL_DECODER_CTX *ctx,
ok = 1;
err:
decoder_clean_pkey_construct_arg(process_data);
- sk_OPENSSL_CSTRING_free(names);
-
return ok;
}
diff --git a/crypto/encode_decode/encoder_local.h b/crypto/encode_decode/encoder_local.h
index c1885ffc77..939e831727 100644
--- a/crypto/encode_decode/encoder_local.h
+++ b/crypto/encode_decode/encoder_local.h
@@ -108,6 +108,7 @@ struct ossl_decoder_instance_st {
void *decoderctx; /* Never NULL */
const char *input_type; /* Never NULL */
const char *input_structure; /* May be NULL */
+ int input_type_id;
unsigned int flag_input_structure_was_set : 1;
};
@@ -162,3 +163,6 @@ const OSSL_PROPERTY_LIST *
ossl_decoder_parsed_properties(const OSSL_DECODER *decoder);
const OSSL_PROPERTY_LIST *
ossl_encoder_parsed_properties(const OSSL_ENCODER *encoder);
+
+int ossl_decoder_fast_is_a(OSSL_DECODER *decoder,
+ const char *name, int *id_cache);
--
2.33.0
Reference in New Issue Copy Permalink