Use centralized fetching errors
authorRichard Levitte <levitte@openssl.org>
Sat, 17 Oct 2020 05:07:41 +0000 (07:07 +0200)
committerRichard Levitte <levitte@openssl.org>
Tue, 12 Jan 2021 18:02:11 +0000 (19:02 +0100)
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)

crypto/context.c
crypto/encode_decode/decoder_meth.c
crypto/encode_decode/encoder_meth.c
crypto/err/err.c
crypto/err/openssl.txt
crypto/evp/evp_err.c
crypto/evp/evp_fetch.c
crypto/store/store_meth.c
include/internal/cryptlib.h
include/openssl/err.h.in
include/openssl/evperr.h

index 4dbfb723e1002750d113986f29f9ebf27b0cb834..c351ff96192b654165cbbaeb8b606c1b6fa072e7 100644 (file)
@@ -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
+}
index 0d389ac5a64b98d332491bb34e6110544bb4c8a9..915c91fd8066eaaa4cc17e1185d8e9e3231cee50 100644 (file)
@@ -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;
index 99c4a119d3b3929f8dd120ff64f8059e059316b4..d3eea415ff969e0d786455a984e5d041ad996520 100644 (file)
@@ -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;
index 9528158a0809744937564c0db8541f4c721f060c..bc7ce875d08bca6574fd39e0e9843ff656d0f08e 100644 (file)
@@ -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
index 4e36fc3394b2359d4ae0b11f0b44f91245173537..b67d2e266df6aa22d3f2c8d90e9da3288b9bb547 100644 (file)
@@ -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
index 894f0cebcb39712b48e46afa38f0de41c092ea3f..e08c373b33ef840631a085887af1529fd24d5cf2 100644 (file)
@@ -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"},
index 7a0a3fcda7fded942f3b1a515803d64bd7dbbfde..5d6d3dbb293f10f30381b72e759d3f4cd6dd02f7 100644 (file)
@@ -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);
     }
index 166b885806ffede6ddfdc2f6be5244791f30830c..979f42a16d3e521902bcc3100930f7b62f6630fd 100644 (file)
@@ -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;
index eae10dfb6cb389cbc78881254239962d8476f912..0267e3f82e91e1011506ffff3e84447249c7d864 100644 (file)
@@ -184,6 +184,7 @@ typedef void (ossl_lib_ctx_onfree_fn)(OSSL_LIB_CTX *ctx);
 int ossl_lib_ctx_run_once(OSSL_LIB_CTX *ctx, unsigned int idx,
                           ossl_lib_ctx_run_once_fn run_once_fn);
 int ossl_lib_ctx_onfree(OSSL_LIB_CTX *ctx, ossl_lib_ctx_onfree_fn onfreefn);
+const char *ossl_lib_ctx_get_descriptor(OSSL_LIB_CTX *libctx);
 
 OSSL_LIB_CTX *crypto_ex_data_get_ossl_lib_ctx(const CRYPTO_EX_DATA *ad);
 int crypto_new_ex_data_ex(OSSL_LIB_CTX *ctx, int class_index, void *obj,
index deb6117d82ec057bc3ea8acf037862a2224fd6c3..697186a2882b9af430eab3ef8c8ade28c9a869de 100644 (file)
@@ -355,6 +355,8 @@ static ossl_unused ossl_inline int ERR_COMMON_ERROR(unsigned long errcode)
 # define ERR_R_INTERRUPTED_OR_CANCELLED          (265|ERR_RFLAG_COMMON)
 # define ERR_R_NESTED_ASN1_ERROR                 (266|ERR_RFLAG_COMMON)
 # define ERR_R_MISSING_ASN1_EOS                  (267|ERR_RFLAG_COMMON)
+# define ERR_R_UNSUPPORTED                       (268|ERR_RFLAG_COMMON)
+# define ERR_R_FETCH_FAILED                      (269|ERR_RFLAG_COMMON)
 
 typedef struct ERR_string_data_st {
     unsigned long error;
index 4e9989899f5d263941e20fa1b001fe78b59be199..c25cc49025d52950a4d2a961855d4bcf44673752 100644 (file)
 # define EVP_R_EXPECTING_A_EC_KEY                         142
 # define EVP_R_EXPECTING_A_POLY1305_KEY                   164
 # define EVP_R_EXPECTING_A_SIPHASH_KEY                    175
-# define EVP_R_FETCH_FAILED                               202
 # define EVP_R_FINAL_ERROR                                188
 # define EVP_R_FIPS_MODE_NOT_SUPPORTED                    167
 # define EVP_R_GENERATE_ERROR                             214