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)

2 files changed, 90 insertions, 6 deletions

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;