Skip to content

Commit

Permalink
Further decoder tuning possibly better perf
Browse files Browse the repository at this point in the history
- The decoder should consider fewer options based on
  more precise tracking of the desired input type
  (DER, PVK, MSBLOB), algorithm (RSA, EC, ...),
  input structure (SPKI, P8, ...).

How much this affects actual use-cases is harder to estimate, we'll just
have to run before/after perf tests.

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
(Merged from openssl/openssl#26927)
  • Loading branch information
Viktor Dukhovni authored and ghen2 committed Mar 1, 2025
1 parent 89dbc6a commit 31b5f3f
Show file tree
Hide file tree
Showing 14 changed files with 199 additions and 47 deletions.
4 changes: 4 additions & 0 deletions apps/lib/apps.c
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,10 @@ static const char *format2string(int format)
return "PEM";
case FORMAT_ASN1:
return "DER";
case FORMAT_PVK:
return "PVK";
case FORMAT_MSBLOB:
return "MSBLOB";
}
return NULL;
}
Expand Down
46 changes: 38 additions & 8 deletions crypto/encode_decode/decoder_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,34 @@ int OSSL_DECODER_CTX_set_input_structure(OSSL_DECODER_CTX *ctx,
return 1;
}

OSSL_DECODER_INSTANCE *
ossl_decoder_instance_new_forprov(OSSL_DECODER *decoder, void *provctx,
const char *input_structure)
{
void *decoderctx;

if (!ossl_assert(decoder != NULL)) {
ERR_raise(ERR_LIB_OSSL_DECODER, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}

decoderctx = decoder->newctx(provctx);
if (decoderctx == NULL)
return 0;
if (input_structure != NULL && decoder->set_ctx_params != NULL) {
OSSL_PARAM params[] = { OSSL_PARAM_END, OSSL_PARAM_END };

params[0] =
OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_STRUCTURE,
(char *)input_structure, 0);
if (!decoder->set_ctx_params(decoderctx, params)) {
decoder->freectx(decoderctx);
return 0;
}
}
return ossl_decoder_instance_new(decoder, decoderctx);
}

OSSL_DECODER_INSTANCE *ossl_decoder_instance_new(OSSL_DECODER *decoder,
void *decoderctx)
{
Expand Down Expand Up @@ -448,15 +476,15 @@ static void collect_extra_decoder(OSSL_DECODER *decoder, void *arg)
if ((decoderctx = decoder->newctx(provctx)) == NULL)
return;

if (data->ctx->input_structure != NULL) {
if (decoder->set_ctx_params != NULL
&& 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)) {
if (!decoder->set_ctx_params(decoderctx, params)) {
decoder->freectx(decoderctx);
return;
}
Expand Down Expand Up @@ -906,6 +934,7 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
sk_OSSL_DECODER_INSTANCE_value(ctx->decoder_insts, i);
OSSL_DECODER *new_decoder =
OSSL_DECODER_INSTANCE_get_decoder(new_decoder_inst);
const char *new_decoder_name = NULL;
void *new_decoderctx =
OSSL_DECODER_INSTANCE_get_decoder_ctx(new_decoder_inst);
const char *new_input_type =
Expand All @@ -916,12 +945,13 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
&n_i_s_was_set);

OSSL_TRACE_BEGIN(DECODER) {
new_decoder_name = OSSL_DECODER_get0_name(new_decoder);
BIO_printf(trc_out,
"(ctx %p) %s [%u] Considering decoder instance %p (decoder %p):\n"
" %s with %s\n",
(void *)new_data.ctx, LEVEL, (unsigned int)i,
(void *)new_decoder_inst, (void *)new_decoder,
OSSL_DECODER_get0_name(new_decoder),
new_decoder_name,
OSSL_DECODER_get0_properties(new_decoder));
} OSSL_TRACE_END(DECODER);

Expand Down Expand Up @@ -1026,9 +1056,9 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
/* Recurse */
OSSL_TRACE_BEGIN(DECODER) {
BIO_printf(trc_out,
"(ctx %p) %s [%u] Running decoder instance %p\n",
"(ctx %p) %s [%u] Running decoder instance %s (%p)\n",
(void *)new_data.ctx, LEVEL, (unsigned int)i,
(void *)new_decoder_inst);
new_decoder_name, (void *)new_decoder_inst);
} OSSL_TRACE_END(DECODER);

/*
Expand All @@ -1048,10 +1078,10 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)

OSSL_TRACE_BEGIN(DECODER) {
BIO_printf(trc_out,
"(ctx %p) %s [%u] Running decoder instance %p => %d"
"(ctx %p) %s [%u] Running decoder instance %s (%p) => %d"
" (recursed further: %s, construct called: %s)\n",
(void *)new_data.ctx, LEVEL, (unsigned int)i,
(void *)new_decoder_inst, ok,
new_decoder_name, (void *)new_decoder_inst, ok,
new_data.flag_next_level_called ? "yes" : "no",
new_data.flag_construct_called ? "yes" : "no");
} OSSL_TRACE_END(DECODER);
Expand Down
14 changes: 14 additions & 0 deletions crypto/encode_decode/decoder_pkey.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,20 @@ static void collect_decoder_keymgmt(EVP_KEYMGMT *keymgmt, OSSL_DECODER *decoder,
return;
}

/*
* Input types must be compatible, but we must accept DER encoders when the
* start input type is "PEM".
*/
if (data->ctx->start_input_type != NULL
&& di->input_type != NULL
&& OPENSSL_strcasecmp(di->input_type, data->ctx->start_input_type) != 0
&& (OPENSSL_strcasecmp(di->input_type, "DER") != 0
|| OPENSSL_strcasecmp(data->ctx->start_input_type, "PEM") != 0)) {
/* Mismatch is not an error, continue. */
ossl_decoder_instance_free(di);
return;
}

OSSL_TRACE_BEGIN(DECODER) {
BIO_printf(trc_out,
"(ctx %p) Checking out decoder %p:\n"
Expand Down
1 change: 1 addition & 0 deletions crypto/err/openssl.txt
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,7 @@ PEM_R_UNSUPPORTED_CIPHER:113:unsupported cipher
PEM_R_UNSUPPORTED_ENCRYPTION:114:unsupported encryption
PEM_R_UNSUPPORTED_KEY_COMPONENTS:126:unsupported key components
PEM_R_UNSUPPORTED_PUBLIC_KEY_TYPE:110:unsupported public key type
PEM_R_UNSUPPORTED_PVK_KEY_TYPE:133:unsupported pvk key type
PKCS12_R_CALLBACK_FAILED:115:callback failed
PKCS12_R_CANT_PACK_STRUCTURE:100:can't pack structure
PKCS12_R_CONTENT_TYPE_NOT_DATA:121:content type not data
Expand Down
4 changes: 3 additions & 1 deletion crypto/pem/pem_err.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Generated by util/mkerr.pl DO NOT EDIT
* Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved.
* Copyright 1995-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
Expand Down Expand Up @@ -59,6 +59,8 @@ static const ERR_STRING_DATA PEM_str_reasons[] = {
"unsupported key components"},
{ERR_PACK(ERR_LIB_PEM, 0, PEM_R_UNSUPPORTED_PUBLIC_KEY_TYPE),
"unsupported public key type"},
{ERR_PACK(ERR_LIB_PEM, 0, PEM_R_UNSUPPORTED_PVK_KEY_TYPE),
"unsupported pvk key type"},
{0, NULL}
};

Expand Down
27 changes: 22 additions & 5 deletions crypto/pem/pvkfmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ int i2b_PublicKey_bio(BIO *out, const EVP_PKEY *pk)
}

int ossl_do_PVK_header(const unsigned char **in, unsigned int length,
int skip_magic,
int skip_magic, int *isdss,
unsigned int *psaltlen, unsigned int *pkeylen)
{
const unsigned char *p = *in;
Expand All @@ -782,9 +782,26 @@ int ossl_do_PVK_header(const unsigned char **in, unsigned int length,
}
/* Skip reserved */
p += 4;
/*
* keytype =
*/ read_ledword(&p);
/* Check the key type */
switch (read_ledword(&p)) {
case MS_KEYTYPE_KEYX:
if (*isdss == 1) {
ERR_raise(ERR_LIB_PEM, PEM_R_EXPECTING_RSA_KEY_BLOB);
return 0;
}
*isdss = 0;
break;
case MS_KEYTYPE_SIGN:
if (*isdss == 0) {
ERR_raise(ERR_LIB_PEM, PEM_R_EXPECTING_DSS_KEY_BLOB);
return 0;
}
*isdss = 1;
break;
default:
ERR_raise(ERR_LIB_PEM, PEM_R_UNSUPPORTED_PVK_KEY_TYPE);
return 0;
}
is_encrypted = read_ledword(&p);
*psaltlen = read_ledword(&p);
*pkeylen = read_ledword(&p);
Expand Down Expand Up @@ -945,7 +962,7 @@ static void *do_PVK_key_bio(BIO *in, pem_password_cb *cb, void *u,
}
p = pvk_hdr;

if (!ossl_do_PVK_header(&p, 24, 0, &saltlen, &keylen))
if (!ossl_do_PVK_header(&p, 24, 0, isdss, &saltlen, &keylen))
return 0;
buflen = (int)keylen + saltlen;
buf = OPENSSL_malloc(buflen);
Expand Down
7 changes: 6 additions & 1 deletion crypto/store/store_result.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
struct extracted_param_data_st {
int object_type;
const char *data_type;
const char *input_type;
const char *data_structure;
const char *utf8_data;
const void *octet_data;
Expand Down Expand Up @@ -115,6 +116,10 @@ int ossl_store_handle_load_result(const OSSL_PARAM params[], void *arg)
if (p != NULL
&& !OSSL_PARAM_get_utf8_string_ptr(p, &helper_data.data_structure))
return 0;
p = OSSL_PARAM_locate_const(params, OSSL_OBJECT_PARAM_INPUT_TYPE);
if (p != NULL
&& !OSSL_PARAM_get_utf8_string_ptr(p, &helper_data.input_type))
return 0;
p = OSSL_PARAM_locate_const(params, OSSL_OBJECT_PARAM_REFERENCE);
if (p != NULL && !OSSL_PARAM_get_octet_string_ptr(p, &helper_data.ref,
&helper_data.ref_size))
Expand Down Expand Up @@ -288,7 +293,7 @@ static EVP_PKEY *try_key_value(struct extracted_param_data_st *data,
}

decoderctx =
OSSL_DECODER_CTX_new_for_pkey(&pk, NULL, data->data_structure,
OSSL_DECODER_CTX_new_for_pkey(&pk, data->input_type, data->data_structure,
data->data_type, selection, libctx,
propq);
(void)OSSL_DECODER_CTX_set_passphrase_cb(decoderctx, cb, cbarg);
Expand Down
3 changes: 2 additions & 1 deletion engines/e_loader_attic.c
Original file line number Diff line number Diff line change
Expand Up @@ -1368,12 +1368,13 @@ static OSSL_STORE_INFO *file_try_read_PVK(BIO *bp, const UI_METHOD *ui_method,

{
unsigned int saltlen = 0, keylen = 0;
int isdss = -1;
unsigned char peekbuf[24] = { 0, };
const unsigned char *p = peekbuf;

if (BIO_buffer_peek(bp, peekbuf, sizeof(peekbuf)) <= 0)
return 0;
if (!ossl_do_PVK_header(&p, sizeof(peekbuf), 0, &saltlen, &keylen))
if (!ossl_do_PVK_header(&p, sizeof(peekbuf), 0, &isdss, &saltlen, &keylen))
return 0;
}

Expand Down
3 changes: 3 additions & 0 deletions include/crypto/decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ void *ossl_decoder_from_algorithm(int id, const OSSL_ALGORITHM *algodef,
OSSL_PROVIDER *prov);

OSSL_DECODER_INSTANCE *
ossl_decoder_instance_new_forprov(OSSL_DECODER *decoder, void *provctx,
const char *input_structure);
OSSL_DECODER_INSTANCE *
ossl_decoder_instance_new(OSSL_DECODER *decoder, void *decoderctx);
void ossl_decoder_instance_free(OSSL_DECODER_INSTANCE *decoder_inst);
int ossl_decoder_ctx_get_harderr(const OSSL_DECODER_CTX *ctx);
Expand Down
2 changes: 1 addition & 1 deletion include/crypto/pem.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ int ossl_do_blob_header(const unsigned char **in, unsigned int length,
int *pisdss, int *pispub);
unsigned int ossl_blob_length(unsigned bitlen, int isdss, int ispub);
int ossl_do_PVK_header(const unsigned char **in, unsigned int length,
int skip_magic,
int skip_magic, int *isdss,
unsigned int *psaltlen, unsigned int *pkeylen);
# ifndef OPENSSL_NO_DEPRECATED_3_0
# ifndef OPENSSL_NO_DSA
Expand Down
3 changes: 2 additions & 1 deletion include/openssl/pemerr.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Generated by util/mkerr.pl DO NOT EDIT
* Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved.
* Copyright 1995-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
Expand Down Expand Up @@ -54,5 +54,6 @@
# define PEM_R_UNSUPPORTED_ENCRYPTION 114
# define PEM_R_UNSUPPORTED_KEY_COMPONENTS 126
# define PEM_R_UNSUPPORTED_PUBLIC_KEY_TYPE 110
# define PEM_R_UNSUPPORTED_PVK_KEY_TYPE 133

#endif
31 changes: 26 additions & 5 deletions providers/implementations/storemgmt/file_store.c
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ static int file_setup_decoders(struct file_ctx_st *ctx)
{
OSSL_LIB_CTX *libctx = ossl_prov_ctx_get0_libctx(ctx->provctx);
const OSSL_ALGORITHM *to_algo = NULL;
const char *input_structure = NULL;
int ok = 0;

/* Setup for this session, so only if not already done */
Expand All @@ -440,8 +441,9 @@ static int file_setup_decoders(struct file_ctx_st *ctx)
*/
switch (ctx->expected_type) {
case OSSL_STORE_INFO_PUBKEY:
input_structure = "SubjectPublicKeyInfo";
if (!OSSL_DECODER_CTX_set_input_structure(ctx->_.file.decoderctx,
"SubjectPublicKeyInfo")) {
input_structure)) {
ERR_raise(ERR_LIB_PROV, ERR_R_OSSL_DECODER_LIB);
goto err;
}
Expand All @@ -456,22 +458,25 @@ static int file_setup_decoders(struct file_ctx_st *ctx)
* might then get a password prompt for a key when reading only
* certs from a file.
*/
input_structure = "EncryptedPrivateKeyInfo";
if (!OSSL_DECODER_CTX_set_input_structure(ctx->_.file.decoderctx,
"EncryptedPrivateKeyInfo")) {
input_structure)) {
ERR_raise(ERR_LIB_PROV, ERR_R_OSSL_DECODER_LIB);
goto err;
}
break;
case OSSL_STORE_INFO_CERT:
input_structure = "Certificate";
if (!OSSL_DECODER_CTX_set_input_structure(ctx->_.file.decoderctx,
"Certificate")) {
input_structure)) {
ERR_raise(ERR_LIB_PROV, ERR_R_OSSL_DECODER_LIB);
goto err;
}
break;
case OSSL_STORE_INFO_CRL:
input_structure = "CertificateList";
if (!OSSL_DECODER_CTX_set_input_structure(ctx->_.file.decoderctx,
"CertificateList")) {
input_structure)) {
ERR_raise(ERR_LIB_PROV, ERR_R_OSSL_DECODER_LIB);
goto err;
}
Expand All @@ -485,6 +490,7 @@ static int file_setup_decoders(struct file_ctx_st *ctx)
to_algo++) {
OSSL_DECODER *to_obj = NULL;
OSSL_DECODER_INSTANCE *to_obj_inst = NULL;
const char *input_type;

/*
* Create the internal last resort decoder implementation
Expand All @@ -494,10 +500,25 @@ static int file_setup_decoders(struct file_ctx_st *ctx)
*/
to_obj = ossl_decoder_from_algorithm(0, to_algo, NULL);
if (to_obj != NULL)
to_obj_inst = ossl_decoder_instance_new(to_obj, ctx->provctx);
to_obj_inst =
ossl_decoder_instance_new_forprov(to_obj, ctx->provctx,
input_structure);
OSSL_DECODER_free(to_obj);
if (to_obj_inst == NULL)
goto err;
/*
* The input type has to match unless, the input type is PEM
* and the decoder input type is DER, in which case we'll pick
* up additional decoders.
*/
input_type = OSSL_DECODER_INSTANCE_get_input_type(to_obj_inst);
if (ctx->_.file.input_type != NULL
&& OPENSSL_strcasecmp(input_type, ctx->_.file.input_type) != 0
&& (OPENSSL_strcasecmp(ctx->_.file.input_type, "PEM") != 0
|| OPENSSL_strcasecmp(input_type, "der") != 0)) {
ossl_decoder_instance_free(to_obj_inst);
continue;
}

if (!ossl_decoder_ctx_add_decoder_inst(ctx->_.file.decoderctx,
to_obj_inst)) {
Expand Down
Loading

0 comments on commit 31b5f3f

Please sign in to comment.