Skip to content

Commit

Permalink
EVP: Centralise fetching error reporting
Browse files Browse the repository at this point in the history 
Instead of sometimes, and sometimes not reporting an error in the
caller of EVP_XXX_fetch(), where the error may or may not be very
accurate, it's now centralised to the inner EVP fetch functionality.
It's made in such a way that it can determine if an error occured
because the algorithm in question is not there, or if something else
went wrong, and will report EVP_R_UNSUPPORTED_ALGORITHM for the
former, and EVP_R_FETCH_FAILED for the latter.

This helps our own test/evp_test.c when it tries to figure out why an
EVP_PKEY it tried to load failed to do so.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from openssl#12857)
  • Loading branch information
@levitte
levitte committed Sep 13, 2020
1 parent 225c966 commit ec0ce18
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 69 deletions.
4 changes: 1 addition & 3 deletions crypto/evp/digest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,8 @@ int EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *impl)
#else
EVP_MD *provmd = EVP_MD_fetch(NULL, OBJ_nid2sn(type->type), "");

if (provmd == NULL) {
EVPerr(EVP_F_EVP_DIGESTINIT_EX, EVP_R_INITIALIZATION_ERROR);
if (provmd == NULL)
return 0;
}
type = provmd;
EVP_MD_free(ctx->fetched_digest);
ctx->fetched_digest = provmd;
Expand Down
4 changes: 1 addition & 3 deletions crypto/evp/evp_enc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,8 @@ int EVP_CipherInit_ex(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher,
: OBJ_nid2sn(cipher->nid),
"");

if (provciph == NULL) {
EVPerr(EVP_F_EVP_CIPHERINIT_EX, EVP_R_INITIALIZATION_ERROR);
if (provciph == NULL)
return 0;
}
cipher = provciph;
EVP_CIPHER_free(ctx->fetched_cipher);
ctx->fetched_cipher = provciph;
Expand Down
139 changes: 78 additions & 61 deletions crypto/evp/evp_fetch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ struct evp_method_data_st {
int name_id; /* For get_evp_method_from_store() */
const char *names; /* For get_evp_method_from_store() */
const char *propquery; /* For get_evp_method_from_store() */

unsigned int flag_construct_error_occured : 1;

void *(*method_from_dispatch)(int name_id, const OSSL_DISPATCH *,
OSSL_PROVIDER *);
int (*refcnt_up_method)(void *method);
Expand Down Expand Up @@ -186,11 +189,23 @@ static void *construct_evp_method(const OSSL_ALGORITHM *algodef,
OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
const char *names = algodef->algorithm_names;
int name_id = ossl_namemap_add_names(namemap, 0, names, NAME_SEPARATOR);
void *method;

if (name_id == 0)
return NULL;
return methdata->method_from_dispatch(name_id, algodef->implementation,
prov);

method = methdata->method_from_dispatch(name_id, algodef->implementation,
prov);

/*
* Flag to indicate that there was actual construction errors. This
* helps inner_evp_generic_fetch() determine what error it should
* record on inaccessible algorithms.
*/
if (method == NULL)
methdata->flag_construct_error_occured = 1;

return method;
}

static void destruct_evp_method(void *method, void *data)
Expand All @@ -200,6 +215,19 @@ static void destruct_evp_method(void *method, void *data)
methdata->destruct_method(method);
}

static const char *libctx_descriptor(OPENSSL_CTX *libctx)
{
#ifdef FIPS_MODULE
return "FIPS internal library context";
#else
if (openssl_ctx_is_global_default(libctx))
return "Global default library context";
if (openssl_ctx_is_default(libctx))
return "Thread-local default library context";
return "Non-default library context";
#endif
}

static void *
inner_evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
int name_id, const char *name,
Expand All @@ -214,23 +242,30 @@ inner_evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
uint32_t meth_id = 0;
void *method = NULL;
int unsupported = 0;

if (store == NULL || namemap == NULL)
if (store == NULL || namemap == NULL) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_PASSED_INVALID_ARGUMENT);
return NULL;
}

/*
* If there's ever an operation_id == 0 passed, we have an internal
* programming error.
*/
if (!ossl_assert(operation_id > 0))
if (!ossl_assert(operation_id > 0)) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
return NULL;
}

/*
* If we have been passed neither a name_id or a name, we have an
* internal programming error.
*/
if (!ossl_assert(name_id != 0 || name != NULL))
if (!ossl_assert(name_id != 0 || name != NULL)) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
return NULL;
}

/* If we haven't received a name id yet, try to get one for the name */
if (name_id == 0)
Expand All @@ -242,9 +277,19 @@ inner_evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
* evp_method_id returns 0 if we have too many operations (more than
* about 2^8) or too many names (more than about 2^24). In that case,
* we can't create any new method.
* For all intents and purposes, this is an internal error.
*/
if (name_id != 0 && (meth_id = evp_method_id(name_id, operation_id)) == 0)
if (name_id != 0 && (meth_id = evp_method_id(name_id, operation_id)) == 0) {
ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
return NULL;
}

/*
* If we haven't found the name yet, chances are that the algorithm to
* be fetched is unsupported.
*/
if (name_id == 0)
unsupported = 1;

if (meth_id == 0
|| !ossl_method_store_cache_get(store, meth_id, properties, &method)) {
Expand All @@ -267,6 +312,7 @@ inner_evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
mcmdata.method_from_dispatch = new_method;
mcmdata.refcnt_up_method = up_ref_method;
mcmdata.destruct_method = free_method;
mcmdata.flag_construct_error_occured = 0;
if ((method = ossl_method_construct(libctx, operation_id,
0 /* !force_cache */,
&mcm, &mcmdata)) != NULL) {
Expand All @@ -282,21 +328,29 @@ inner_evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
ossl_method_store_cache_set(store, meth_id, properties, method,
up_ref_method, free_method);
}

/*
* If we never were in the constructor, the algorithm to be fetched
* is unsupported.
*/
unsupported = !mcmdata.flag_construct_error_occured;
}

return method;
}
if (method == NULL) {
int code =
unsupported ? EVP_R_UNSUPPORTED_ALGORITHM : EVP_R_FETCH_FAILED;

#ifndef FIPS_MODULE
static const char *libctx_descriptor(OPENSSL_CTX *libctx)
{
if (openssl_ctx_is_global_default(libctx))
return "Global default library context";
if (openssl_ctx_is_default(libctx))
return "Thread-local default library context";
return "Non-default library context";
if (name == NULL)
name = ossl_namemap_num2name(namemap, name_id, 0);
ERR_raise_data(ERR_LIB_EVP, code,
"%s, Algorithm (%s : %d), Properties (%s)",
libctx_descriptor(libctx),
name = NULL ? "<null>" : name, name_id,
properties == NULL ? "<null>" : properties);
}

return method;
}
#endif

void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
const char *name, const char *properties,
Expand All @@ -306,24 +360,9 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
int (*up_ref_method)(void *),
void (*free_method)(void *))
{
void *ret = inner_evp_generic_fetch(libctx,
operation_id, 0, name, properties,
new_method, up_ref_method, free_method);

if (ret == NULL) {
int code = EVP_R_FETCH_FAILED;

#ifdef FIPS_MODULE
ERR_raise(ERR_LIB_EVP, code);
#else
ERR_raise_data(ERR_LIB_EVP, code,
"%s, Algorithm (%s), Properties (%s)",
libctx_descriptor(libctx),
name = NULL ? "<null>" : name,
properties == NULL ? "<null>" : properties);
#endif
}
return ret;
return inner_evp_generic_fetch(libctx,
operation_id, 0, name, properties,
new_method, up_ref_method, free_method);
}

/*
Expand All @@ -341,32 +380,10 @@ void *evp_generic_fetch_by_number(OPENSSL_CTX *libctx, int operation_id,
int (*up_ref_method)(void *),
void (*free_method)(void *))
{
void *ret = inner_evp_generic_fetch(libctx,
operation_id, name_id, NULL,
properties, new_method, up_ref_method,
free_method);

if (ret == NULL) {
int code = EVP_R_FETCH_FAILED;

#ifdef FIPS_MODULE
ERR_raise(ERR_LIB_EVP, code);
#else
{
OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
const char *name = (namemap == NULL)
? NULL
: ossl_namemap_num2name(namemap, name_id, 0);

ERR_raise_data(ERR_LIB_EVP, code,
"%s, Algorithm (%s), Properties (%s)",
libctx_descriptor(libctx),
name = NULL ? "<null>" : name,
properties == NULL ? "<null>" : properties);
}
#endif
}
return ret;
return inner_evp_generic_fetch(libctx,
operation_id, name_id, NULL,
properties, new_method, up_ref_method,
free_method);
}

void evp_method_store_flush(OPENSSL_CTX *libctx)
Expand Down
3 changes: 1 addition & 2 deletions test/evp_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3254,8 +3254,7 @@ static int key_unsupported(void)
long err = ERR_peek_last_error();

if (ERR_GET_LIB(err) == ERR_LIB_EVP
&& (ERR_GET_REASON(err) == EVP_R_UNSUPPORTED_ALGORITHM
|| ERR_GET_REASON(err) == EVP_R_FETCH_FAILED)) {
&& (ERR_GET_REASON(err) == EVP_R_UNSUPPORTED_ALGORITHM)) {
ERR_clear_error();
return 1;
}
Expand Down

0 comments on commit ec0ce18

Please sign in to comment.