Use centralized fetching errors

We've spread around FETCH_FAILED errors in quite a few places, and
that gives somewhat crude error records, as there's no way to tell if
the error was unavailable algorithms or some other error at such high
levels.

As an alternative, we take recording of these kinds of errors down to
the fetching functions, which are in a much better place to tell what
kind of error it was, thereby relieving the higher level calls from
having to guess.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13467)

8 files changed, 164 insertions, 31 deletions

diff --git a/crypto/context.c b/crypto/context.c
index 4dbfb723e1..c351ff9619 100644
--- a/crypto/context.c
+++ b/crypto/context.c
@@ -368,3 +368,16 @@ int ossl_lib_ctx_onfree(OSSL_LIB_CTX *ctx, ossl_lib_ctx_onfree_fn onfreefn)
 
     return 1;
 }
+
+const char *ossl_lib_ctx_get_descriptor(OSSL_LIB_CTX *libctx)
+{
+#ifdef FIPS_MODULE
+    return "FIPS internal library context";
+#else
+    if (ossl_lib_ctx_is_global_default(libctx))
+        return "Global default library context";
+    if (ossl_lib_ctx_is_default(libctx))
+        return "Thread-local default library context";
+    return "Non-default library context";
+#endif
+}
diff --git a/crypto/encode_decode/decoder_meth.c b/crypto/encode_decode/decoder_meth.c
index 0d389ac5a6..915c91fd80 100644
--- a/crypto/encode_decode/decoder_meth.c
+++ b/crypto/encode_decode/decoder_meth.c
@@ -87,6 +87,8 @@ struct decoder_data_st {
     int id;                      /* For get_decoder_from_store() */
     const char *names;           /* For get_decoder_from_store() */
     const char *propquery;       /* For get_decoder_from_store() */
+
+    unsigned int flag_construct_error_occured : 1;
 };
 
 /*
@@ -242,7 +244,7 @@ void *ossl_decoder_from_dispatch(int id, const OSSL_ALGORITHM *algodef,
  * then call ossl_decoder_from_dispatch() with that identity number.
  */
 static void *construct_decoder(const OSSL_ALGORITHM *algodef,
-                               OSSL_PROVIDER *prov, void *unused)
+                               OSSL_PROVIDER *prov, void *data)
 {
     /*
      * This function is only called if get_decoder_from_store() returned
@@ -250,6 +252,7 @@ static void *construct_decoder(const OSSL_ALGORITHM *algodef,
      * namemap entry, this is it.  Should the name already exist there, we
      * know that ossl_namemap_add() will return its corresponding number.
      */
+    struct decoder_data_st *methdata = data;
     OSSL_LIB_CTX *libctx = ossl_provider_libctx(prov);
     OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
     const char *names = algodef->algorithm_names;
@@ -259,6 +262,14 @@ static void *construct_decoder(const OSSL_ALGORITHM *algodef,
     if (id != 0)
         method = ossl_decoder_from_dispatch(id, algodef, 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;
 }
 
@@ -286,20 +297,32 @@ static OSSL_DECODER *inner_ossl_decoder_fetch(OSSL_LIB_CTX *libctx, int id,
     OSSL_METHOD_STORE *store = get_decoder_store(libctx);
     OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
     void *method = NULL;
+    int unsupported = 0;
 
-    if (store == NULL || namemap == NULL)
+    if (store == NULL || namemap == NULL) {
+        ERR_raise(ERR_LIB_OSSL_DECODER, ERR_R_PASSED_INVALID_ARGUMENT);
         return NULL;
+    }
 
     /*
      * If we have been passed neither a name_id or a name, we have an
      * internal programming error.
      */
-    if (!ossl_assert(id != 0 || name != NULL))
+    if (!ossl_assert(id != 0 || name != NULL)) {
+        ERR_raise(ERR_LIB_OSSL_DECODER, ERR_R_INTERNAL_ERROR);
         return NULL;
+    }
 
     if (id == 0)
         id = ossl_namemap_name2num(namemap, name);
 
+    /*
+     * If we haven't found the name yet, chances are that the algorithm to
+     * be fetched is unsupported.
+     */
+    if (id == 0)
+        unsupported = 1;
+
     if (id == 0
         || !ossl_method_store_cache_get(store, id, properties, &method)) {
         OSSL_METHOD_CONSTRUCT_METHOD mcm = {
@@ -317,6 +340,7 @@ static OSSL_DECODER *inner_ossl_decoder_fetch(OSSL_LIB_CTX *libctx, int id,
         mcmdata.id = id;
         mcmdata.names = name;
         mcmdata.propquery = properties;
+        mcmdata.flag_construct_error_occured = 0;
         if ((method = ossl_method_construct(libctx, OSSL_OP_DECODER,
                                             0 /* !force_cache */,
                                             &mcm, &mcmdata)) != NULL) {
@@ -331,6 +355,24 @@ static OSSL_DECODER *inner_ossl_decoder_fetch(OSSL_LIB_CTX *libctx, int id,
             ossl_method_store_cache_set(store, id, properties, method,
                                         up_ref_decoder, free_decoder);
         }
+
+        /*
+         * If we never were in the constructor, the algorithm to be fetched
+         * is unsupported.
+         */
+        unsupported = !mcmdata.flag_construct_error_occured;
+    }
+
+    if (method == NULL) {
+        int code = unsupported ? ERR_R_UNSUPPORTED : ERR_R_FETCH_FAILED;
+
+        if (name == NULL)
+            name = ossl_namemap_num2name(namemap, id, 0);
+        ERR_raise_data(ERR_LIB_OSSL_DECODER, code,
+                       "%s, Name (%s : %d), Properties (%s)",
+                       ossl_lib_ctx_get_descriptor(libctx),
+                       name = NULL ? "<null>" : name, id,
+                       properties == NULL ? "<null>" : properties);
     }
 
     return method;
diff --git a/crypto/encode_decode/encoder_meth.c b/crypto/encode_decode/encoder_meth.c
index 99c4a119d3..d3eea415ff 100644
--- a/crypto/encode_decode/encoder_meth.c
+++ b/crypto/encode_decode/encoder_meth.c
@@ -87,6 +87,8 @@ struct encoder_data_st {
     int id;                      /* For get_encoder_from_store() */
     const char *names;           /* For get_encoder_from_store() */
     const char *propquery;       /* For get_encoder_from_store() */
+
+    unsigned int flag_construct_error_occured : 1;
 };
 
 /*
@@ -254,7 +256,7 @@ static void *encoder_from_dispatch(int id, const OSSL_ALGORITHM *algodef,
  * then call encoder_from_dispatch() with that identity number.
  */
 static void *construct_encoder(const OSSL_ALGORITHM *algodef,
-                               OSSL_PROVIDER *prov, void *unused)
+                               OSSL_PROVIDER *prov, void *data)
 {
     /*
      * This function is only called if get_encoder_from_store() returned
@@ -262,6 +264,7 @@ static void *construct_encoder(const OSSL_ALGORITHM *algodef,
      * namemap entry, this is it.  Should the name already exist there, we
      * know that ossl_namemap_add() will return its corresponding number.
      */
+    struct encoder_data_st *methdata = data;
     OSSL_LIB_CTX *libctx = ossl_provider_libctx(prov);
     OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
     const char *names = algodef->algorithm_names;
@@ -271,6 +274,14 @@ static void *construct_encoder(const OSSL_ALGORITHM *algodef,
     if (id != 0)
         method = encoder_from_dispatch(id, algodef, 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;
 }
 
@@ -298,20 +309,32 @@ static OSSL_ENCODER *inner_ossl_encoder_fetch(OSSL_LIB_CTX *libctx,
     OSSL_METHOD_STORE *store = get_encoder_store(libctx);
     OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
     void *method = NULL;
+    int unsupported = 0;
 
-    if (store == NULL || namemap == NULL)
+    if (store == NULL || namemap == NULL) {
+        ERR_raise(ERR_LIB_OSSL_ENCODER, ERR_R_PASSED_INVALID_ARGUMENT);
         return NULL;
+    }
 
     /*
      * If we have been passed neither a name_id or a name, we have an
      * internal programming error.
      */
-    if (!ossl_assert(id != 0 || name != NULL))
+    if (!ossl_assert(id != 0 || name != NULL)) {
+        ERR_raise(ERR_LIB_OSSL_ENCODER, ERR_R_INTERNAL_ERROR);
         return NULL;
+    }
 
     if (id == 0)
         id = ossl_namemap_name2num(namemap, name);
 
+    /*
+     * If we haven't found the name yet, chances are that the algorithm to
+     * be fetched is unsupported.
+     */
+    if (id == 0)
+        unsupported = 1;
+
     if (id == 0
         || !ossl_method_store_cache_get(store, id, properties, &method)) {
         OSSL_METHOD_CONSTRUCT_METHOD mcm = {
@@ -329,6 +352,7 @@ static OSSL_ENCODER *inner_ossl_encoder_fetch(OSSL_LIB_CTX *libctx,
         mcmdata.id = id;
         mcmdata.names = name;
         mcmdata.propquery = properties;
+        mcmdata.flag_construct_error_occured = 0;
         if ((method = ossl_method_construct(libctx, OSSL_OP_ENCODER,
                                             0 /* !force_cache */,
                                             &mcm, &mcmdata)) != NULL) {
@@ -343,6 +367,24 @@ static OSSL_ENCODER *inner_ossl_encoder_fetch(OSSL_LIB_CTX *libctx,
             ossl_method_store_cache_set(store, id, properties, method,
                                         up_ref_encoder, free_encoder);
         }
+
+        /*
+         * If we never were in the constructor, the algorithm to be fetched
+         * is unsupported.
+         */
+        unsupported = !mcmdata.flag_construct_error_occured;
+    }
+
+    if (method == NULL) {
+        int code = unsupported ? ERR_R_UNSUPPORTED : ERR_R_FETCH_FAILED;
+
+        if (name == NULL)
+            name = ossl_namemap_num2name(namemap, id, 0);
+        ERR_raise_data(ERR_LIB_OSSL_ENCODER, code,
+                       "%s, Name (%s : %d), Properties (%s)",
+                       ossl_lib_ctx_get_descriptor(libctx),
+                       name = NULL ? "<null>" : name, id,
+                       properties == NULL ? "<null>" : properties);
     }
 
     return method;
diff --git a/crypto/err/err.c b/crypto/err/err.c
index 9528158a08..bc7ce875d0 100644
--- a/crypto/err/err.c
+++ b/crypto/err/err.c
@@ -117,6 +117,15 @@ static ERR_STRING_DATA ERR_str_reasons[] = {
     {ERR_R_INVALID_PROVIDER_FUNCTIONS, "invalid provider functions"},
     {ERR_R_INTERRUPTED_OR_CANCELLED, "interrupted or cancelled"},
 
+    /*
+     * Something is unsupported, exactly what is expressed with additional data
+     */
+    {ERR_R_UNSUPPORTED, "unsupported"},
+    /*
+     * A fetch failed for other reasons than the name to be fetched being
+     * unsupported.
+     */
+    {ERR_R_FETCH_FAILED, "fetch failed"},
     {0, NULL},
 };
 #endif
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index 4e36fc3394..b67d2e266d 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -2554,7 +2554,6 @@ EVP_R_EXPECTING_A_ECX_KEY:219:expecting a ecx key
 EVP_R_EXPECTING_A_EC_KEY:142:expecting a ec key
 EVP_R_EXPECTING_A_POLY1305_KEY:164:expecting a poly1305 key
 EVP_R_EXPECTING_A_SIPHASH_KEY:175:expecting a siphash key
-EVP_R_FETCH_FAILED:202:fetch failed
 EVP_R_FINAL_ERROR:188:final error
 EVP_R_FIPS_MODE_NOT_SUPPORTED:167:fips mode not supported
 EVP_R_GENERATE_ERROR:214:generate error
diff --git a/crypto/evp/evp_err.c b/crypto/evp/evp_err.c
index 894f0cebcb..e08c373b33 100644
--- a/crypto/evp/evp_err.c
+++ b/crypto/evp/evp_err.c
@@ -71,7 +71,6 @@ static const ERR_STRING_DATA EVP_str_reasons[] = {
     "expecting a poly1305 key"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_EXPECTING_A_SIPHASH_KEY),
     "expecting a siphash key"},
-    {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_FETCH_FAILED), "fetch failed"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_FINAL_ERROR), "final error"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_FIPS_MODE_NOT_SUPPORTED),
     "fips mode not supported"},
diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c
index 7a0a3fcda7..5d6d3dbb29 100644
--- a/crypto/evp/evp_fetch.c
+++ b/crypto/evp/evp_fetch.c
@@ -215,19 +215,6 @@ static void destruct_evp_method(void *method, void *data)
     methdata->destruct_method(method);
 }
 
-static const char *libctx_descriptor(OSSL_LIB_CTX *libctx)
-{
-#ifdef FIPS_MODULE
-    return "FIPS internal library context";
-#else
-    if (ossl_lib_ctx_is_global_default(libctx))
-        return "Global default library context";
-    if (ossl_lib_ctx_is_default(libctx))
-        return "Thread-local default library context";
-    return "Non-default library context";
-#endif
-}
-
 static void *
 inner_evp_generic_fetch(OSSL_LIB_CTX *libctx, int operation_id,
                         int name_id, const char *name,
@@ -245,7 +232,7 @@ inner_evp_generic_fetch(OSSL_LIB_CTX *libctx, int operation_id,
     int unsupported = 0;
 
     if (store == NULL || namemap == NULL) {
-        ERR_raise(ERR_LIB_CRYPTO, ERR_R_PASSED_INVALID_ARGUMENT);
+        ERR_raise(ERR_LIB_EVP, ERR_R_PASSED_INVALID_ARGUMENT);
         return NULL;
     }
 
@@ -254,7 +241,7 @@ inner_evp_generic_fetch(OSSL_LIB_CTX *libctx, int operation_id,
      * programming error.
      */
     if (!ossl_assert(operation_id > 0)) {
-        ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+        ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);
         return NULL;
     }
 
@@ -263,7 +250,7 @@ inner_evp_generic_fetch(OSSL_LIB_CTX *libctx, int operation_id,
      * internal programming error.
      */
     if (!ossl_assert(name_id != 0 || name != NULL)) {
-        ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+        ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);
         return NULL;
     }
 
@@ -280,7 +267,7 @@ inner_evp_generic_fetch(OSSL_LIB_CTX *libctx, int operation_id,
      * For all intents and purposes, this is an internal error.
      */
     if (name_id != 0 && (meth_id = evp_method_id(name_id, operation_id)) == 0) {
-        ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+        ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);
         return NULL;
     }
 
@@ -337,14 +324,13 @@ inner_evp_generic_fetch(OSSL_LIB_CTX *libctx, int operation_id,
     }
 
     if (method == NULL) {
-        int code =
-            unsupported ? EVP_R_UNSUPPORTED_ALGORITHM : EVP_R_FETCH_FAILED;
+        int code = unsupported ? ERR_R_UNSUPPORTED : ERR_R_FETCH_FAILED;
 
         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),
+                       ossl_lib_ctx_get_descriptor(libctx),
                        name = NULL ? "<null>" : name, name_id,
                        properties == NULL ? "<null>" : properties);
     }
diff --git a/crypto/store/store_meth.c b/crypto/store/store_meth.c
index 166b885806..979f42a16d 100644
--- a/crypto/store/store_meth.c
+++ b/crypto/store/store_meth.c
@@ -92,6 +92,8 @@ struct loader_data_st {
     int scheme_id;               /* For get_loader_from_store() */
     const char *scheme;          /* For get_loader_from_store() */
     const char *propquery;       /* For get_loader_from_store() */
+
+    unsigned int flag_construct_error_occured : 1;
 };
 
 /*
@@ -227,7 +229,7 @@ static void *loader_from_dispatch(int scheme_id, const OSSL_ALGORITHM *algodef,
  * then call loader_from_dispatch() with that identity number.
  */
 static void *construct_loader(const OSSL_ALGORITHM *algodef,
-                              OSSL_PROVIDER *prov, void *unused)
+                              OSSL_PROVIDER *prov, void *data)
 {
     /*
      * This function is only called if get_loader_from_store() returned
@@ -235,6 +237,7 @@ static void *construct_loader(const OSSL_ALGORITHM *algodef,
      * namemap entry, this is it.  Should the scheme already exist there, we
      * know that ossl_namemap_add() will return its corresponding number.
      */
+    struct loader_data_st *methdata = data;
     OSSL_LIB_CTX *libctx = ossl_provider_libctx(prov);
     OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
     const char *scheme = algodef->algorithm_names;
@@ -244,6 +247,14 @@ static void *construct_loader(const OSSL_ALGORITHM *algodef,
     if (id != 0)
         method = loader_from_dispatch(id, algodef, 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;
 }
 
@@ -261,20 +272,33 @@ static OSSL_STORE_LOADER *inner_loader_fetch(OSSL_LIB_CTX *libctx,
     OSSL_METHOD_STORE *store = get_loader_store(libctx);
     OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx);
     void *method = NULL;
+    int unsupported = 0;
 
-    if (store == NULL || namemap == NULL)
+    if (store == NULL || namemap == NULL) {
+        ERR_raise(ERR_LIB_OSSL_STORE, ERR_R_PASSED_INVALID_ARGUMENT);
         return NULL;
+    }
 
     /*
      * If we have been passed neither a scheme_id or a scheme, we have an
      * internal programming error.
      */
-    if (!ossl_assert(id != 0 || scheme != NULL))
+    if (!ossl_assert(id != 0 || scheme != NULL)) {
+        ERR_raise(ERR_LIB_OSSL_STORE, ERR_R_INTERNAL_ERROR);
         return NULL;
+    }
 
+    /* If we haven't received a name id yet, try to get one for the name */
     if (id == 0)
         id = ossl_namemap_name2num(namemap, scheme);
 
+    /*
+     * If we haven't found the name yet, chances are that the algorithm to
+     * be fetched is unsupported.
+     */
+    if (id == 0)
+        unsupported = 1;
+
     if (id == 0
         || !ossl_method_store_cache_get(store, id, properties, &method)) {
         OSSL_METHOD_CONSTRUCT_METHOD mcm = {
@@ -292,6 +316,7 @@ static OSSL_STORE_LOADER *inner_loader_fetch(OSSL_LIB_CTX *libctx,
         mcmdata.scheme_id = id;
         mcmdata.scheme = scheme;
         mcmdata.propquery = properties;
+        mcmdata.flag_construct_error_occured = 0;
         if ((method = ossl_method_construct(libctx, OSSL_OP_STORE,
                                             0 /* !force_cache */,
                                             &mcm, &mcmdata)) != NULL) {
@@ -305,6 +330,24 @@ static OSSL_STORE_LOADER *inner_loader_fetch(OSSL_LIB_CTX *libctx,
             ossl_method_store_cache_set(store, id, properties, method,
                                         up_ref_loader, free_loader);
         }
+
+        /*
+         * If we never were in the constructor, the algorithm to be fetched
+         * is unsupported.
+         */
+        unsupported = !mcmdata.flag_construct_error_occured;
+    }
+
+    if (method == NULL) {
+        int code = unsupported ? ERR_R_UNSUPPORTED : ERR_R_FETCH_FAILED;
+
+        if (scheme == NULL)
+            scheme = ossl_namemap_num2name(namemap, id, 0);
+        ERR_raise_data(ERR_LIB_OSSL_STORE, code,
+                       "%s, Scheme (%s : %d), Properties (%s)",
+                       ossl_lib_ctx_get_descriptor(libctx),
+                       scheme = NULL ? "<null>" : scheme, id,
+                       properties == NULL ? "<null>" : properties);
     }
 
     return method;