[openssl] master update
Richard Levitte
levitte at openssl.org
Wed Apr 21 08:54:53 UTC 2021
The branch master has been updated
via 1fbf7079e7aff51d02333aad63593386b27aa209 (commit)
via 7aef200089fbf4b306d13905d55772d646ceef76 (commit)
via 9cc97ddf3c8c3c6ef30b0505ad2559d3734c685d (commit)
via f99659535d180f15cd19c63cb53392c256e35534 (commit)
from a2502862f679c82b794869ac88ed0d8ca7bc291c (commit)
- Log -----------------------------------------------------------------
commit 1fbf7079e7aff51d02333aad63593386b27aa209
Author: Richard Levitte <levitte at openssl.org>
Date: Fri Apr 16 14:34:19 2021 +0200
STORE: Discard the error report filter in crypto/store/store_result.c
The error report filter was fragile, as it could potentially have to
be updated when other parts of libcrypto got updated, making a goose
chase and a maintenance problem.
We change this to regard d2i errors as something we don't care so much
about, since they are mainly part of the guessing mechanism. The
success of the ossl_store_handle_load_result() call is based on
whether an object was actually created or not anyway.
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14834)
commit 7aef200089fbf4b306d13905d55772d646ceef76
Author: Richard Levitte <levitte at openssl.org>
Date: Fri Apr 16 10:08:38 2021 +0200
TEST: Adapt the EVP test
The EVP test didn't recognise ERR_R_UNSUPPORTED, now does
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14834)
commit 9cc97ddf3c8c3c6ef30b0505ad2559d3734c685d
Author: Richard Levitte <levitte at openssl.org>
Date: Mon Apr 12 12:20:20 2021 +0200
Adapt our decoder implementations to the new way to indicate succes / failure
This includes the special decoder used in our STOREMGMT 'file:' implementation
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14834)
commit f99659535d180f15cd19c63cb53392c256e35534
Author: Richard Levitte <levitte at openssl.org>
Date: Mon Apr 12 12:11:07 2021 +0200
ENCODER & DECODER: Allow decoder implementations to specify "carry on"
So far, decoder implementations would return true (1) for a successful
decode all the way, including what the callback it called returned,
and false (0) in all other cases.
This construction didn't allow to stop to decoding process on fatal
errors, nor to choose what to report in the provider code.
This is now changed so that decoders implementations are made to
return false only on errors that should stop the decoding process from
carrying on with other implementations, and return true for all other
cases, even if that didn't result in a constructed object (EVP_PKEY
for example), essentially making it OK to return "empty handed".
The success of the decoding process is now all about successfully
constructing the final object, rather than about the return value of
the decoding chain. If no construction is attempted, the central
decoding processing code concludes that whatever the input consisted
of, it's not supported by the available decoder implementations.
Fixes #14423
Reviewed-by: Tomas Mraz <tomas at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14834)
-----------------------------------------------------------------------
Summary of changes:
crypto/encode_decode/decoder_err.c | 4 +-
crypto/encode_decode/decoder_lib.c | 89 ++++++++++++++++------
crypto/err/openssl.txt | 1 +
crypto/store/store_result.c | 88 ++++++++++++---------
doc/man7/provider-decoder.pod | 29 ++++++-
include/crypto/decodererr.h | 2 +-
include/openssl/decodererr.h | 1 +
.../implementations/encode_decode/decode_der2key.c | 51 +++++--------
.../encode_decode/decode_msblob2key.c | 29 ++++---
.../implementations/encode_decode/decode_pem2der.c | 15 +++-
.../implementations/encode_decode/decode_pvk2key.c | 26 +++++++
.../implementations/storemgmt/file_store_der2obj.c | 21 +++--
test/evp_test.c | 12 +--
test/recipes/30-test_evp.t | 2 +-
14 files changed, 249 insertions(+), 121 deletions(-)
diff --git a/crypto/encode_decode/decoder_err.c b/crypto/encode_decode/decoder_err.c
index cf68a4c7c5..1880c8f409 100644
--- a/crypto/encode_decode/decoder_err.c
+++ b/crypto/encode_decode/decoder_err.c
@@ -1,6 +1,6 @@
/*
* Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2020 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2021 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
@@ -15,6 +15,8 @@
#ifndef OPENSSL_NO_ERR
static const ERR_STRING_DATA OSSL_DECODER_str_reasons[] = {
+ {ERR_PACK(ERR_LIB_OSSL_DECODER, 0, OSSL_DECODER_R_COULD_NOT_DECODE_OBJECT),
+ "could not decode object"},
{ERR_PACK(ERR_LIB_OSSL_DECODER, 0, OSSL_DECODER_R_MISSING_GET_PARAMS),
"missing get params"},
{0, NULL}
diff --git a/crypto/encode_decode/decoder_lib.c b/crypto/encode_decode/decoder_lib.c
index a644924aeb..e37989fec4 100644
--- a/crypto/encode_decode/decoder_lib.c
+++ b/crypto/encode_decode/decoder_lib.c
@@ -32,6 +32,12 @@ struct decoder_process_data_st {
size_t current_decoder_inst_index;
/* For tracing, count recursion level */
size_t recursion;
+
+ /*-
+ * Flags
+ */
+ unsigned int flag_next_level_called : 1;
+ unsigned int flag_construct_called : 1;
};
static int decoder_process(const OSSL_PARAM params[], void *arg);
@@ -57,6 +63,29 @@ int OSSL_DECODER_from_bio(OSSL_DECODER_CTX *ctx, BIO *in)
ok = decoder_process(NULL, &data);
+ if (!data.flag_construct_called) {
+ const char *spaces
+ = ctx->start_input_type != NULL && ctx->input_structure != NULL
+ ? " " : "";
+ const char *input_type_label
+ = ctx->start_input_type != NULL ? "Input type: " : "";
+ const char *input_structure_label
+ = ctx->input_structure != NULL ? "Input structure: " : "";
+ const char *comma
+ = ctx->start_input_type != NULL && ctx->input_structure != NULL
+ ? ", " : "";
+ const char *input_type
+ = ctx->start_input_type != NULL ? ctx->start_input_type : "";
+ const char *input_structure
+ = ctx->input_structure != NULL ? ctx->input_structure : "";
+
+ ERR_raise_data(ERR_LIB_OSSL_DECODER, ERR_R_UNSUPPORTED,
+ "No supported for the data to decode.%s%s%s%s%s%s",
+ spaces, input_type_label, input_type, comma,
+ input_structure_label, input_structure);
+ ok = 0;
+ }
+
/* Clear any internally cached passphrase */
(void)ossl_pw_clear_passphrase_cache(&ctx->pwdata);
@@ -525,12 +554,18 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
BIO *bio = data->bio;
long loc;
size_t i;
- int err, lib, reason, ok = 0;
+ int ok = 0;
/* For recursions */
struct decoder_process_data_st new_data;
const char *data_type = NULL;
const char *data_structure = NULL;
+ /*
+ * This is an indicator up the call stack that something was indeed
+ * decoded, leading to a recursive call of this function.
+ */
+ data->flag_next_level_called = 1;
+
memset(&new_data, 0, sizeof(new_data));
new_data.ctx = data->ctx;
new_data.recursion = data->recursion + 1;
@@ -562,10 +597,14 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
data->current_decoder_inst_index);
decoder = OSSL_DECODER_INSTANCE_get_decoder(decoder_inst);
- if (ctx->construct != NULL
- && ctx->construct(decoder_inst, params, ctx->construct_data)) {
- ok = 1;
- goto end;
+ data->flag_construct_called = 0;
+ if (ctx->construct != NULL) {
+ int rv = ctx->construct(decoder_inst, params, ctx->construct_data);
+
+ data->flag_construct_called = 1;
+ ok = (rv > 0);
+ if (ok)
+ goto end;
}
/* The constructor didn't return success */
@@ -746,6 +785,12 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
(void *)new_decoder_inst);
} OSSL_TRACE_END(DECODER);
+ /*
+ * We only care about errors reported from decoder implementations
+ * if it returns false (i.e. there was a fatal error).
+ */
+ ERR_set_mark();
+
new_data.current_decoder_inst_index = i;
ok = new_decoder->decode(new_decoderctx, cbio,
new_data.ctx->selection,
@@ -755,31 +800,29 @@ 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\n",
+ "(ctx %p) %s [%u] Running decoder instance %p => %d"
+ " (recursed further: %s, construct called: %s)\n",
(void *)new_data.ctx, LEVEL, (unsigned int)i,
- (void *)new_decoder_inst, ok);
+ (void *)new_decoder_inst, ok,
+ new_data.flag_next_level_called ? "yes" : "no",
+ new_data.flag_construct_called ? "yes" : "no");
} OSSL_TRACE_END(DECODER);
- if (ok)
+ data->flag_construct_called = new_data.flag_construct_called;
+
+ /* Break on error or if we tried to construct an object already */
+ if (!ok || data->flag_construct_called) {
+ ERR_clear_last_mark();
break;
+ }
+ ERR_pop_to_mark();
/*
- * These errors are assumed to come from ossl_store_handle_load_result()
- * in crypto/store/store_result.c. They are currently considered fatal
- * errors, so we preserve them in the error queue and stop.
+ * Break if the decoder implementation that we called recursed, since
+ * that indicates that it successfully decoded something.
*/
- err = ERR_peek_last_error();
- lib = ERR_GET_LIB(err);
- reason = ERR_GET_REASON(err);
- if ((lib == ERR_LIB_EVP
- && reason == EVP_R_UNSUPPORTED_PRIVATE_KEY_ALGORITHM)
-#ifndef OPENSSL_NO_EC
- || (lib == ERR_LIB_EC && reason == EC_R_UNKNOWN_GROUP)
-#endif
- || (lib == ERR_LIB_X509 && reason == X509_R_UNSUPPORTED_ALGORITHM)
- || (lib == ERR_LIB_PKCS12
- && reason == PKCS12_R_PKCS12_CIPHERFINAL_ERROR))
- goto end;
+ if (new_data.flag_next_level_called)
+ break;
}
end:
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index eed0b71ada..81f9f1ef49 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -811,6 +811,7 @@ OCSP_R_STATUS_TOO_OLD:127:status too old
OCSP_R_UNKNOWN_MESSAGE_DIGEST:119:unknown message digest
OCSP_R_UNKNOWN_NID:120:unknown nid
OCSP_R_UNSUPPORTED_REQUESTORNAME_TYPE:129:unsupported requestorname type
+OSSL_DECODER_R_COULD_NOT_DECODE_OBJECT:101:could not decode object
OSSL_DECODER_R_MISSING_GET_PARAMS:100:missing get params
OSSL_ENCODER_R_ENCODER_NOT_FOUND:101:encoder not found
OSSL_ENCODER_R_INCORRECT_PROPERTY_QUERY:100:incorrect property query
diff --git a/crypto/store/store_result.c b/crypto/store/store_result.c
index 72f054be17..f69045994d 100644
--- a/crypto/store/store_result.c
+++ b/crypto/store/store_result.c
@@ -82,25 +82,6 @@ static int try_crl(struct extracted_param_data_st *, OSSL_STORE_INFO **,
static int try_pkcs12(struct extracted_param_data_st *, OSSL_STORE_INFO **,
OSSL_STORE_CTX *, OSSL_LIB_CTX *, const char *);
-#define SET_ERR_MARK() ERR_set_mark()
-#define CLEAR_ERR_MARK() \
- do { \
- int err = ERR_peek_last_error(); \
- \
- if (ERR_GET_LIB(err) == ERR_LIB_ASN1 \
- && (ERR_GET_REASON(err) == ASN1_R_UNKNOWN_PUBLIC_KEY_TYPE \
- || ERR_GET_REASON(err) == ASN1_R_NO_MATCHING_CHOICE_TYPE \
- || ERR_GET_REASON(err) == ERR_R_NESTED_ASN1_ERROR)) \
- ERR_pop_to_mark(); \
- else \
- ERR_clear_last_mark(); \
- } while(0)
-#define RESET_ERR_MARK() \
- do { \
- CLEAR_ERR_MARK(); \
- SET_ERR_MARK(); \
- } while(0)
-
int ossl_store_handle_load_result(const OSSL_PARAM params[], void *arg)
{
struct ossl_load_result_data_st *cbdata = arg;
@@ -145,22 +126,16 @@ int ossl_store_handle_load_result(const OSSL_PARAM params[], void *arg)
* The helper functions return 0 on actual errors, otherwise 1, even if
* they didn't fill out |*v|.
*/
- SET_ERR_MARK();
if (!try_name(&helper_data, v))
goto err;
- RESET_ERR_MARK();
if (!try_key(&helper_data, v, ctx, provider, libctx, propq))
goto err;
- RESET_ERR_MARK();
if (!try_cert(&helper_data, v, libctx, propq))
goto err;
- RESET_ERR_MARK();
if (!try_crl(&helper_data, v, libctx, propq))
goto err;
- RESET_ERR_MARK();
if (!try_pkcs12(&helper_data, v, ctx, libctx, propq))
goto err;
- CLEAR_ERR_MARK();
return (*v != NULL);
err:
@@ -304,16 +279,19 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
const unsigned char *der = data->octet_data, *derp;
long der_len = (long)data->octet_data_size;
- SET_ERR_MARK();
/* Try PUBKEY first, that's a real easy target */
if (ctx->expected_type == 0
|| ctx->expected_type == OSSL_STORE_INFO_PUBKEY) {
+ /* Discard DER decoding errors, it only means we return "empty handed" */
+ if (ctx->expected_type == 0)
+ ERR_set_mark();
derp = der;
pk = d2i_PUBKEY_ex(NULL, &derp, der_len, libctx, propq);
+ if (ctx->expected_type == 0)
+ ERR_pop_to_mark();
+
if (pk != NULL)
*store_info_new = OSSL_STORE_INFO_new_PUBKEY;
-
- RESET_ERR_MARK();
}
/* Try private keys next */
@@ -324,9 +302,18 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
X509_SIG *p8 = NULL;
PKCS8_PRIV_KEY_INFO *p8info = NULL;
- /* See if it's an encrypted PKCS#8 and decrypt it */
+ /*
+ * See if it's an encrypted PKCS#8 and decrypt it. Discard DER
+ * decoding errors, a failed decode only means we return "empty handed"
+ */
+ if (ctx->expected_type == 0)
+ ERR_set_mark();
derp = der;
- if ((p8 = d2i_X509_SIG(NULL, &derp, der_len)) != NULL) {
+ p8 = d2i_X509_SIG(NULL, &derp, der_len);
+ if (ctx->expected_type == 0)
+ ERR_pop_to_mark();
+
+ if (p8 != NULL) {
char pbuf[PEM_BUFSIZE];
size_t plen = 0;
@@ -351,17 +338,22 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
}
X509_SIG_free(p8);
}
- RESET_ERR_MARK();
/*
* If the encrypted PKCS#8 couldn't be decrypted,
* |der| is NULL
*/
if (der != NULL) {
- /* Try to unpack an unencrypted PKCS#8, that's easy */
+ /*
+ * Try to unpack an unencrypted PKCS#8, that's easy. Discard DER
+ * decoding errors, a failed decode only means we return "empty
+ * handed"
+ */
+ ERR_set_mark();
derp = der;
p8info = d2i_PKCS8_PRIV_KEY_INFO(NULL, &derp, der_len);
- RESET_ERR_MARK();
+ ERR_pop_to_mark();
+
if (p8info != NULL) {
pk = EVP_PKCS82PKEY_ex(p8info, libctx, propq);
PKCS8_PRIV_KEY_INFO_free(p8info);
@@ -373,7 +365,6 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
OPENSSL_free(new_der);
}
- CLEAR_ERR_MARK();
return pk;
}
@@ -473,11 +464,16 @@ static int try_cert(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
&& (strcasecmp(data->data_type, PEM_STRING_X509_TRUSTED) == 0))
ignore_trusted = 0;
+ /* Discard DER decoding errors, it only means we return "empty handed" */
+ if (data->object_type == OSSL_OBJECT_UNKNOWN)
+ ERR_set_mark();
cert = d2i_X509_AUX(NULL, (const unsigned char **)&data->octet_data,
data->octet_data_size);
if (cert == NULL && ignore_trusted)
cert = d2i_X509(NULL, (const unsigned char **)&data->octet_data,
data->octet_data_size);
+ if (data->object_type == OSSL_OBJECT_UNKNOWN)
+ ERR_pop_to_mark();
if (cert != NULL)
/* We determined the object type */
@@ -504,8 +500,14 @@ static int try_crl(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
|| data->object_type == OSSL_OBJECT_CRL) {
X509_CRL *crl;
+ /* Discard DER decoding errors, it only means we return "empty handed" */
+ if (data->object_type == OSSL_OBJECT_UNKNOWN)
+ ERR_set_mark();
crl = d2i_X509_CRL(NULL, (const unsigned char **)&data->octet_data,
data->octet_data_size);
+ if (data->object_type == OSSL_OBJECT_UNKNOWN)
+ ERR_pop_to_mark();
+
if (crl != NULL)
/* We determined the object type */
data->object_type = OSSL_OBJECT_CRL;
@@ -528,13 +530,20 @@ static int try_pkcs12(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
OSSL_STORE_CTX *ctx,
OSSL_LIB_CTX *libctx, const char *propq)
{
+ int ok = 1;
+
/* There is no specific object type for PKCS12 */
if (data->object_type == OSSL_OBJECT_UNKNOWN) {
/* Initial parsing */
PKCS12 *p12;
- if ((p12 = d2i_PKCS12(NULL, (const unsigned char **)&data->octet_data,
- data->octet_data_size)) != NULL) {
+ /* Discard DER decoding errors, it only means we return "empty handed" */
+ ERR_set_mark();
+ p12 = d2i_PKCS12(NULL, (const unsigned char **)&data->octet_data,
+ data->octet_data_size);
+ ERR_pop_to_mark();
+
+ if (p12 != NULL) {
char *pass = NULL;
char tpass[PEM_BUFSIZE];
size_t tpass_len;
@@ -544,6 +553,8 @@ static int try_pkcs12(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
data->object_type = OSSL_OBJECT_PKCS12;
+ ok = 0; /* Assume decryption or parse error */
+
if (PKCS12_verify_mac(p12, "", 0)
|| PKCS12_verify_mac(p12, NULL, 0)) {
pass = "";
@@ -577,7 +588,8 @@ static int try_pkcs12(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
OSSL_STORE_INFO *osi_pkey = NULL;
OSSL_STORE_INFO *osi_cert = NULL;
OSSL_STORE_INFO *osi_ca = NULL;
- int ok = 1;
+
+ ok = 1; /* Parsing went through correctly! */
if ((infos = sk_OSSL_STORE_INFO_new_null()) != NULL) {
if (pkey != NULL) {
@@ -627,5 +639,5 @@ static int try_pkcs12(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
*v = sk_OSSL_STORE_INFO_shift(ctx->cached_info);
}
- return 1;
+ return ok;
}
diff --git a/doc/man7/provider-decoder.pod b/doc/man7/provider-decoder.pod
index 73f653e063..23b4fbc9df 100644
--- a/doc/man7/provider-decoder.pod
+++ b/doc/man7/provider-decoder.pod
@@ -210,6 +210,32 @@ The decoding functions also take an B<OSSL_PASSPHRASE_CALLBACK> function
pointer along with a pointer to application data I<cbarg>, which should be
used when a pass phrase prompt is needed.
+It's important to understand that the return value from this function is
+interpreted as follows:
+
+=over 4
+
+=item True (1)
+
+This means "carry on the decoding process", and is meaningful even though
+this function couldn't decode the input into anything, because there may be
+another decoder implementation that can decode it into something.
+
+The I<data_cb> callback should never be called when this function can't
+decode the input into anything.
+
+=item False (0)
+
+This means "stop the decoding process", and is meaningful when the input
+could be decoded into some sort of object that this function understands,
+but further treatment of that object results into errors that won't be
+possible for some other decoder implementation to get a different result.
+
+=back
+
+The conditions to stop the decoding process are at the discretion of the
+implementation.
+
=head2 Decoder parameters
The decoder implementation itself has parameters that can be used to
@@ -315,7 +341,8 @@ constant B<OSSL_PARAM> elements.
OSSL_FUNC_decoder_does_selection() returns 1 if the decoder implementation
supports any of the I<selection> bits, otherwise 0.
-OSSL_FUNC_decoder_decode() returns 1 on success, or 0 on failure.
+OSSL_FUNC_decoder_decode() returns 1 to signal that the decoding process
+should continue, or 0 to signal that it should stop.
=head1 SEE ALSO
diff --git a/include/crypto/decodererr.h b/include/crypto/decodererr.h
index c19f70c1c6..edf826798d 100644
--- a/include/crypto/decodererr.h
+++ b/include/crypto/decodererr.h
@@ -1,6 +1,6 @@
/*
* Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 2020 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2020-2021 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
diff --git a/include/openssl/decodererr.h b/include/openssl/decodererr.h
index 886c3750fe..824a0a9253 100644
--- a/include/openssl/decodererr.h
+++ b/include/openssl/decodererr.h
@@ -21,6 +21,7 @@
/*
* OSSL_DECODER reason codes.
*/
+# define OSSL_DECODER_R_COULD_NOT_DECODE_OBJECT 101
# define OSSL_DECODER_R_MISSING_GET_PARAMS 100
#endif
diff --git a/providers/implementations/encode_decode/decode_der2key.c b/providers/implementations/encode_decode/decode_der2key.c
index f50fca3896..73acf527c1 100644
--- a/providers/implementations/encode_decode/decode_der2key.c
+++ b/providers/implementations/encode_decode/decode_der2key.c
@@ -36,26 +36,6 @@
#include "prov/implementations.h"
#include "endecoder_local.h"
-#define SET_ERR_MARK() ERR_set_mark()
-#define CLEAR_ERR_MARK() \
- do { \
- int err = ERR_peek_last_error(); \
- \
- if (ERR_GET_LIB(err) == ERR_LIB_ASN1 \
- && (ERR_GET_REASON(err) == ASN1_R_HEADER_TOO_LONG \
- || ERR_GET_REASON(err) == ASN1_R_UNSUPPORTED_TYPE \
- || ERR_GET_REASON(err) == ERR_R_NESTED_ASN1_ERROR \
- || ERR_GET_REASON(err) == ASN1_R_NOT_ENOUGH_DATA)) \
- ERR_pop_to_mark(); \
- else \
- ERR_clear_last_mark(); \
- } while(0)
-#define RESET_ERR_MARK() \
- do { \
- CLEAR_ERR_MARK(); \
- SET_ERR_MARK(); \
- } while(0)
-
struct der2key_ctx_st; /* Forward declaration */
typedef int check_key_fn(void *, struct der2key_ctx_st *ctx);
typedef void adjust_key_fn(void *, struct der2key_ctx_st *ctx);
@@ -143,6 +123,7 @@ static void *der2key_decode_p8(const unsigned char **input_der,
void *key = NULL;
ctx->flag_fatal = 0;
+
if ((p8 = d2i_X509_SIG(NULL, input_der, input_der_len)) != NULL) {
char pbuf[PEM_BUFSIZE];
size_t plen = 0;
@@ -162,6 +143,7 @@ static void *der2key_decode_p8(const unsigned char **input_der,
&& OBJ_obj2nid(alg->algorithm) == ctx->desc->evp_type)
key = key_from_pkcs8(p8inf, PROV_LIBCTX_OF(ctx->provctx), NULL);
PKCS8_PRIV_KEY_INFO_free(p8inf);
+
return key;
}
@@ -284,12 +266,13 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
return 0;
}
- SET_ERR_MARK();
- if (!read_der(ctx->provctx, cin, &der, &der_len))
+ ok = read_der(ctx->provctx, cin, &der, &der_len);
+ if (!ok)
goto next;
+ ok = 0; /* Assume that we fail */
+
if ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) {
- RESET_ERR_MARK();
derp = der;
if (ctx->desc->d2i_PKCS8 != NULL) {
key = ctx->desc->d2i_PKCS8(NULL, &derp, der_len, ctx,
@@ -303,7 +286,6 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
goto next;
}
if (key == NULL && (selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) {
- RESET_ERR_MARK();
derp = der;
if (ctx->desc->d2i_PUBKEY != NULL)
key = ctx->desc->d2i_PUBKEY(NULL, &derp, der_len);
@@ -313,19 +295,25 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
goto next;
}
if (key == NULL && (selection & OSSL_KEYMGMT_SELECT_ALL_PARAMETERS) != 0) {
- RESET_ERR_MARK();
derp = der;
if (ctx->desc->d2i_key_params != NULL)
key = ctx->desc->d2i_key_params(NULL, &derp, der_len);
if (key == NULL && orig_selection != 0)
goto next;
}
- RESET_ERR_MARK();
+
+ /*
+ * Last minute check to see if this was the correct type of key. This
+ * should never lead to a fatal error, i.e. the decoding itself was
+ * correct, it was just an unexpected key type. This is generally for
+ * classes of key types that have subtle variants, like RSA-PSS keys as
+ * opposed to plain RSA keys.
+ */
if (key != NULL
&& ctx->desc->check_key != NULL
&& !ctx->desc->check_key(key, ctx)) {
- CLEAR_ERR_MARK();
- goto end;
+ ctx->desc->free_key(key);
+ key = NULL;
}
if (key != NULL && ctx->desc->adjust_key != NULL)
@@ -333,11 +321,10 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
next:
/*
- * Prune low-level ASN.1 parse errors from error queue, assuming
- * that this is called by decoder_process() in a loop trying several
- * formats.
+ * Indicated that we successfully decoded something, or not at all.
+ * Ending up "empty handed" is not an error.
*/
- CLEAR_ERR_MARK();
+ ok = 1;
/*
* We free memory here so it's not held up during the callback, because
diff --git a/providers/implementations/encode_decode/decode_msblob2key.c b/providers/implementations/encode_decode/decode_msblob2key.c
index f47d06f59d..84b259591b 100644
--- a/providers/implementations/encode_decode/decode_msblob2key.c
+++ b/providers/implementations/encode_decode/decode_msblob2key.c
@@ -116,30 +116,35 @@ static int msblob2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
if (BIO_read(in, hdr_buf, 16) != 16) {
ERR_raise(ERR_LIB_PEM, PEM_R_KEYBLOB_TOO_SHORT);
- goto err;
+ goto next;
}
+ ERR_set_mark();
p = hdr_buf;
- if (ossl_do_blob_header(&p, 16, &magic, &bitlen, &isdss, &ispub) <= 0)
- goto err;
+ ok = ossl_do_blob_header(&p, 16, &magic, &bitlen, &isdss, &ispub) > 0;
+ ERR_pop_to_mark();
+ if (!ok)
+ goto next;
+
+ ok = 0; /* Assume that we fail */
if ((isdss && ctx->desc->type != EVP_PKEY_DSA)
|| (!isdss && ctx->desc->type != EVP_PKEY_RSA))
- goto err;
+ goto next;
length = ossl_blob_length(bitlen, isdss, ispub);
if (length > BLOB_MAX_LENGTH) {
ERR_raise(ERR_LIB_PEM, PEM_R_HEADER_TOO_LONG);
- goto err;
+ goto next;
}
buf = OPENSSL_malloc(length);
if (buf == NULL) {
ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE);
- goto err;
+ goto end;
}
p = buf;
if (BIO_read(in, buf, length) != (int)length) {
ERR_raise(ERR_LIB_PEM, PEM_R_KEYBLOB_TOO_SHORT);
- goto err;
+ goto next;
}
if ((selection == 0
@@ -150,7 +155,7 @@ static int msblob2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
memset(&pwdata, 0, sizeof(pwdata));
if (!ossl_pw_set_ossl_passphrase_cb(&pwdata, pw_cb, pw_cbarg))
- goto err;
+ goto end;
p = buf;
key = ctx->desc->read_private_key(&p, bitlen, ispub);
if (selection != 0 && key == NULL)
@@ -170,6 +175,12 @@ static int msblob2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
ctx->desc->adjust_key(key, ctx);
next:
+ /*
+ * Indicated that we successfully decoded something, or not at all.
+ * Ending up "empty handed" is not an error.
+ */
+ ok = 1;
+
/*
* We free resources here so it's not held up during the callback, because
* we know the process is recursive and the allocated chunks of memory
@@ -198,7 +209,7 @@ static int msblob2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
ok = data_cb(params, data_cbarg);
}
- err:
+ end:
BIO_free(in);
OPENSSL_free(buf);
ctx->desc->free_key(key);
diff --git a/providers/implementations/encode_decode/decode_pem2der.c b/providers/implementations/encode_decode/decode_pem2der.c
index fe6839965d..4249ce9cc7 100644
--- a/providers/implementations/encode_decode/decode_pem2der.c
+++ b/providers/implementations/encode_decode/decode_pem2der.c
@@ -145,9 +145,11 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
int objtype = OSSL_OBJECT_UNKNOWN;
const char *data_structure = NULL;
- if (read_pem(ctx->provctx, cin, &pem_name, &pem_header,
- &der, &der_len) <= 0)
- return 0;
+ ok = read_pem(ctx->provctx, cin, &pem_name, &pem_header,
+ &der, &der_len) > 0;
+ /* We return "empty handed". This is not an error. */
+ if (!ok)
+ return 1;
/*
* 10 is the number of characters in "Proc-Type:", which
@@ -159,6 +161,7 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
EVP_CIPHER_INFO cipher;
struct pem2der_pass_data_st pass_data;
+ ok = 0; /* Assume that we fail */
pass_data.cb = pw_cb;
pass_data.cbarg = pw_cbarg;
if (!PEM_get_EVP_CIPHER_INFO(pem_header, &cipher)
@@ -167,6 +170,12 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
goto end;
}
+ /*
+ * Indicated that we successfully decoded something, or not at all.
+ * Ending up "empty handed" is not an error.
+ */
+ ok = 1;
+
/*
* Peal off certain strings from the end of |pem_name|, as they serve
* no further purpose.
diff --git a/providers/implementations/encode_decode/decode_pvk2key.c b/providers/implementations/encode_decode/decode_pvk2key.c
index 3f2c80abdc..702c89a928 100644
--- a/providers/implementations/encode_decode/decode_pvk2key.c
+++ b/providers/implementations/encode_decode/decode_pvk2key.c
@@ -20,6 +20,7 @@
#include <openssl/core_object.h>
#include <openssl/crypto.h>
#include <openssl/params.h>
+#include <openssl/err.h>
#include <openssl/pem.h> /* For public PVK functions */
#include <openssl/x509.h>
#include "internal/passphrase.h"
@@ -111,11 +112,30 @@ static int pvk2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
|| (selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0)
&& ctx->desc->read_private_key != NULL) {
struct ossl_passphrase_data_st pwdata;
+ int err, lib, reason;
memset(&pwdata, 0, sizeof(pwdata));
if (!ossl_pw_set_ossl_passphrase_cb(&pwdata, pw_cb, pw_cbarg))
goto end;
+
key = ctx->desc->read_private_key(in, ossl_pw_pem_password, &pwdata);
+
+ /*
+ * Because the PVK API doesn't have a separate decrypt call, we need
+ * to check the error queue for certain well known errors that are
+ * considered fatal and which we pass through, while the rest gets
+ * thrown away.
+ */
+ err = ERR_peek_last_error();
+ lib = ERR_GET_LIB(err);
+ reason = ERR_GET_REASON(err);
+ if (lib == ERR_LIB_PEM
+ && (reason == PEM_R_BAD_PASSWORD_READ
+ || reason == PEM_R_BAD_DECRYPT)) {
+ ERR_clear_last_mark();
+ goto end;
+ }
+
if (selection != 0 && key == NULL)
goto next;
}
@@ -124,6 +144,12 @@ static int pvk2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
ctx->desc->adjust_key(key, ctx);
next:
+ /*
+ * Indicated that we successfully decoded something, or not at all.
+ * Ending up "empty handed" is not an error.
+ */
+ ok = 1;
+
/*
* We free resources here so it's not held up during the callback, because
* we know the process is recursive and the allocated chunks of memory
diff --git a/providers/implementations/storemgmt/file_store_der2obj.c b/providers/implementations/storemgmt/file_store_der2obj.c
index 94bc467e3e..2ecf20bac7 100644
--- a/providers/implementations/storemgmt/file_store_der2obj.c
+++ b/providers/implementations/storemgmt/file_store_der2obj.c
@@ -98,16 +98,22 @@ static int der2obj_decode(void *provctx, OSSL_CORE_BIO *cin, int selection,
* Prune low-level ASN.1 parse errors from error queue, assuming that
* this is called by decoder_process() in a loop trying several formats.
*/
- err = ERR_peek_last_error();
- if (ERR_GET_LIB(err) == ERR_LIB_ASN1
+ if (!ok) {
+ err = ERR_peek_last_error();
+ if (ERR_GET_LIB(err) == ERR_LIB_ASN1
&& (ERR_GET_REASON(err) == ASN1_R_HEADER_TOO_LONG
|| ERR_GET_REASON(err) == ASN1_R_UNSUPPORTED_TYPE
|| ERR_GET_REASON(err) == ERR_R_NESTED_ASN1_ERROR
- || ERR_GET_REASON(err) == ASN1_R_NOT_ENOUGH_DATA))
- ERR_pop_to_mark();
- else
- ERR_clear_last_mark();
- if (ok) {
+ || ERR_GET_REASON(err) == ASN1_R_NOT_ENOUGH_DATA)) {
+ ERR_pop_to_mark();
+ } else {
+ ERR_clear_last_mark();
+ goto end;
+ }
+ }
+
+ ok = 1;
+ if (mem != NULL) {
OSSL_PARAM params[3];
int object_type = OSSL_OBJECT_UNKNOWN;
@@ -122,6 +128,7 @@ static int der2obj_decode(void *provctx, OSSL_CORE_BIO *cin, int selection,
OPENSSL_free(mem->data);
OPENSSL_free(mem);
}
+ end:
BIO_free(in);
return ok;
}
diff --git a/test/evp_test.c b/test/evp_test.c
index 08341e4617..7bfe97f4ae 100644
--- a/test/evp_test.c
+++ b/test/evp_test.c
@@ -3264,9 +3264,11 @@ static void free_key_list(KEY_LIST *lst)
static int key_unsupported(void)
{
long err = ERR_peek_last_error();
+ int lib = ERR_GET_LIB(err);
+ long reason = ERR_GET_REASON(err);
- if (ERR_GET_LIB(err) == ERR_LIB_EVP
- && (ERR_GET_REASON(err) == EVP_R_UNSUPPORTED_ALGORITHM)) {
+ if ((lib == ERR_LIB_EVP && reason == EVP_R_UNSUPPORTED_ALGORITHM)
+ || reason == ERR_R_UNSUPPORTED) {
ERR_clear_error();
return 1;
}
@@ -3276,9 +3278,9 @@ static int key_unsupported(void)
* hint to an unsupported algorithm/curve (e.g. if binary EC support is
* disabled).
*/
- if (ERR_GET_LIB(err) == ERR_LIB_EC
- && (ERR_GET_REASON(err) == EC_R_UNKNOWN_GROUP
- || ERR_GET_REASON(err) == EC_R_INVALID_CURVE)) {
+ if (lib == ERR_LIB_EC
+ && (reason == EC_R_UNKNOWN_GROUP
+ || reason == EC_R_INVALID_CURVE)) {
ERR_clear_error();
return 1;
}
diff --git a/test/recipes/30-test_evp.t b/test/recipes/30-test_evp.t
index 87bb501095..2cb25478d7 100644
--- a/test/recipes/30-test_evp.t
+++ b/test/recipes/30-test_evp.t
@@ -171,7 +171,7 @@ SKIP: {
ok(test_errors(key => 'server-dsa-pubkey.pem',
out => 'server-dsa-pubkey.err',
args => [ '-pubin' ],
- expected => 'unsupported algorithm'),
+ expected => 'unsupported'),
"expected error loading unsupported dsa public key");
}
More information about the openssl-commits
mailing list