property: Move global default properties to the library context.
authorPauli <paul.dale@oracle.com>
Fri, 12 Jun 2020 00:34:46 +0000 (10:34 +1000)
committerPauli <paul.dale@oracle.com>
Tue, 23 Jun 2020 11:44:47 +0000 (21:44 +1000)
Fixes a problem where global properties don't work with a NULL query.
Specifying an algorithm with a NULL query ignores the default properties.

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

crypto/err/openssl.txt
crypto/evp/evp_err.c
crypto/evp/evp_fetch.c
crypto/property/property.c
crypto/property/property_local.h
doc/internal/man3/OSSL_METHOD_STORE.pod
include/internal/cryptlib.h
include/internal/property.h
include/openssl/evperr.h
test/evp_extra_test.c

index c308036003ccca8e4ecacf21c18f3e6040901a0c..e5df6bc0921e9eef24a0e9e5703590b98d6ab428 100644 (file)
@@ -2522,6 +2522,7 @@ EVP_R_CTRL_NOT_IMPLEMENTED:132:ctrl not implemented
 EVP_R_CTRL_OPERATION_NOT_IMPLEMENTED:133:ctrl operation not implemented
 EVP_R_DATA_NOT_MULTIPLE_OF_BLOCK_LENGTH:138:data not multiple of block length
 EVP_R_DECODE_ERROR:114:decode error
+EVP_R_DEFAULT_QUERY_PARSE_ERROR:210:default query parse error
 EVP_R_DIFFERENT_KEY_TYPES:101:different key types
 EVP_R_DIFFERENT_PARAMETERS:153:different parameters
 EVP_R_ERROR_LOADING_SECTION:165:error loading section
index 0908b1ca160fb3106b686add251601629349ce6b..3dba7f2931c89dd11cda6011e4eae1636cf93819 100644 (file)
@@ -44,6 +44,8 @@ static const ERR_STRING_DATA EVP_str_reasons[] = {
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_DATA_NOT_MULTIPLE_OF_BLOCK_LENGTH),
     "data not multiple of block length"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_DECODE_ERROR), "decode error"},
+    {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_DEFAULT_QUERY_PARSE_ERROR),
+    "default query parse error"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_DIFFERENT_KEY_TYPES),
     "different key types"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_DIFFERENT_PARAMETERS),
index 596f5925350236ec5f3d0fc6d05ab0d659cd0560..f63e135d3ab3bcb190a56d7f2a0fa3d80964938e 100644 (file)
@@ -361,33 +361,63 @@ void *evp_generic_fetch_by_number(OPENSSL_CTX *libctx, int operation_id,
     return ret;
 }
 
-int EVP_set_default_properties(OPENSSL_CTX *libctx, const char *propq)
+static int evp_set_default_properties(OPENSSL_CTX *libctx,
+                                      OSSL_PROPERTY_LIST *def_prop)
 {
     OSSL_METHOD_STORE *store = get_evp_method_store(libctx);
-
-    if (store != NULL)
-        return ossl_method_store_set_global_properties(store, propq);
+    OSSL_PROPERTY_LIST **plp = ossl_ctx_global_properties(libctx);
+
+    if (plp != NULL) {
+        ossl_property_free(*plp);
+        *plp = def_prop;
+        if (store != NULL)
+            ossl_method_store_flush_cache(store);
+        return 1;
+    }
     EVPerr(0, ERR_R_INTERNAL_ERROR);
     return 0;
 }
 
+int EVP_set_default_properties(OPENSSL_CTX *libctx, const char *propq)
+{
+    OSSL_PROPERTY_LIST *pl = NULL;
+
+    if (propq != NULL && (pl = ossl_parse_query(libctx, propq)) == NULL) {
+        EVPerr(0, EVP_R_DEFAULT_QUERY_PARSE_ERROR);
+        return 0;
+    }
+    return evp_set_default_properties(libctx, pl);
+}
+
 
 static int evp_default_properties_merge(OPENSSL_CTX *libctx, const char *propq)
 {
-    OSSL_METHOD_STORE *store = get_evp_method_store(libctx);
-
-    if (store != NULL)
-        return ossl_method_store_merge_global_properties(store, propq);
-    EVPerr(0, ERR_R_INTERNAL_ERROR);
-    return 0;
+    OSSL_PROPERTY_LIST **plp = ossl_ctx_global_properties(libctx);
+    OSSL_PROPERTY_LIST *pl1, *pl2;
+
+    if (propq == NULL)
+        return 1;
+    if (plp == NULL || *plp == NULL)
+        return EVP_set_default_properties(libctx, propq);
+    if ((pl1 = ossl_parse_query(libctx, propq)) == NULL) {
+        EVPerr(0, EVP_R_DEFAULT_QUERY_PARSE_ERROR);
+        return 0;
+    }
+    pl2 = ossl_property_merge(pl1, *plp);
+    ossl_property_free(pl1);
+    if (pl2 == NULL) {
+        EVPerr(0, ERR_R_MALLOC_FAILURE);
+        return 0;
+    }
+    return evp_set_default_properties(libctx, pl2);
 }
 
 static int evp_default_property_is_enabled(OPENSSL_CTX *libctx,
                                            const char *prop_name)
 {
-    OSSL_METHOD_STORE *store = get_evp_method_store(libctx);
+    OSSL_PROPERTY_LIST **plp = ossl_ctx_global_properties(libctx);
 
-    return ossl_method_store_global_property_is_enabled(store, prop_name);
+    return plp != NULL && ossl_property_is_enabled(libctx, prop_name, *plp);
 }
 
 int EVP_default_properties_is_fips_enabled(OPENSSL_CTX *libctx)
index ef39057c54a86f64a1e33dfee92c4814f089f0f0..a72ccb02b4d6f5d46a3f4df2617e3a75ca9e48ba 100644 (file)
@@ -60,7 +60,6 @@ struct ossl_method_store_st {
     OPENSSL_CTX *ctx;
     size_t nelem;
     SPARSE_ARRAY_OF(ALGORITHM) *algs;
-    OSSL_PROPERTY_LIST *global_properties;
     int need_flush;
     CRYPTO_RWLOCK *lock;
 };
@@ -74,7 +73,34 @@ typedef struct {
 DEFINE_SPARSE_ARRAY_OF(ALGORITHM);
 
 static void ossl_method_cache_flush(OSSL_METHOD_STORE *store, int nid);
-static void ossl_method_cache_flush_all(OSSL_METHOD_STORE *c);
+
+/* Global properties are stored per library context */
+static void ossl_ctx_global_properties_free(void *vstore)
+{
+    OSSL_PROPERTY_LIST **plp = vstore;
+
+    if (plp != NULL) {
+        ossl_property_free(*plp);
+        OPENSSL_free(plp);
+    }
+}
+
+static void *ossl_ctx_global_properties_new(OPENSSL_CTX *ctx)
+{
+    return OPENSSL_zalloc(sizeof(OSSL_PROPERTY_LIST **));
+}
+
+
+static const OPENSSL_CTX_METHOD ossl_ctx_global_properties_method = {
+    ossl_ctx_global_properties_new,
+    ossl_ctx_global_properties_free,
+};
+
+OSSL_PROPERTY_LIST **ossl_ctx_global_properties(OPENSSL_CTX *libctx)
+{
+    return openssl_ctx_get_data(libctx, OPENSSL_CTX_GLOBAL_PROPERTIES,
+                                &ossl_ctx_global_properties_method);
+}
 
 static int ossl_method_up_ref(METHOD *method)
 {
@@ -166,7 +192,6 @@ void ossl_method_store_free(OSSL_METHOD_STORE *store)
     if (store != NULL) {
         ossl_sa_ALGORITHM_doall(store->algs, &alg_cleanup);
         ossl_sa_ALGORITHM_free(store->algs);
-        ossl_property_free(store->global_properties);
         CRYPTO_THREAD_lock_free(store->lock);
         OPENSSL_free(store);
     }
@@ -296,11 +321,13 @@ int ossl_method_store_remove(OSSL_METHOD_STORE *store, int nid,
 }
 
 int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
-                            const char *prop_query, void **method)
+                            const char *prop_query,
+                            void **method)
 {
+    OSSL_PROPERTY_LIST **plp = ossl_ctx_global_properties(store->ctx);
     ALGORITHM *alg;
     IMPLEMENTATION *impl;
-    OSSL_PROPERTY_LIST *pq = NULL, *p2;
+    OSSL_PROPERTY_LIST *pq = NULL, *p2 = NULL;
     METHOD *best_method = NULL;
     int ret = 0;
     int j, best = -1, score, optional;
@@ -323,23 +350,28 @@ int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
         return 0;
     }
 
-    if (prop_query == NULL) {
+    if (prop_query != NULL) {
+        p2 = pq = ossl_parse_query(store->ctx, prop_query);
+    }
+    if (plp != NULL && *plp != NULL) {
+        if (pq == NULL) {
+            pq = *plp;
+        } else {
+            p2 = ossl_property_merge(pq, *plp);
+            if (p2 == NULL)
+                goto fin;
+            ossl_property_free(pq);
+            pq = p2;
+        }
+    }
+
+    if (pq == NULL) {
         if ((impl = sk_IMPLEMENTATION_value(alg->impls, 0)) != NULL) {
             best_method = &impl->method;
             ret = 1;
         }
         goto fin;
     }
-    pq = ossl_parse_query(store->ctx, prop_query);
-    if (pq == NULL)
-        goto fin;
-    if (store->global_properties != NULL) {
-        p2 = ossl_property_merge(pq, store->global_properties);
-        if (p2 == NULL)
-            goto fin;
-        ossl_property_free(pq);
-        pq = p2;
-    }
     optional = ossl_property_has_optional(pq);
     for (j = 0; j < sk_IMPLEMENTATION_num(alg->impls); j++) {
         impl = sk_IMPLEMENTATION_value(alg->impls, j);
@@ -358,88 +390,10 @@ fin:
     else
         ret = 0;
     ossl_property_unlock(store);
-    ossl_property_free(pq);
-    return ret;
-}
-
-int ossl_method_store_global_property_is_enabled(OSSL_METHOD_STORE *store,
-                                                 const char *prop_name)
-{
-    int ret = 0;
-
-    if (store == NULL)
-        return 0;
-
-    ossl_property_read_lock(store);
-    ret = ossl_property_is_enabled(store->ctx, prop_name,
-                                   store->global_properties);
-    ossl_property_unlock(store);
-    return ret;
-}
-
-int ossl_method_store_set_global_properties(OSSL_METHOD_STORE *store,
-                                            const char *prop_query)
-{
-    int ret = 0;
-
-    if (store == NULL)
-        return 1;
-
-    ossl_property_write_lock(store);
-    ossl_method_cache_flush_all(store);
-
-    ossl_property_free(store->global_properties);
-    store->global_properties = NULL;
-
-    if (prop_query == NULL) {
-        ossl_property_unlock(store);
-        return 1;
-    }
-    store->global_properties = ossl_parse_query(store->ctx, prop_query);
-    ret = store->global_properties != NULL;
-    ossl_property_unlock(store);
-    return ret;
-}
-
-int ossl_method_store_merge_global_properties(OSSL_METHOD_STORE *store,
-                                              const char *prop_query)
-{
-    int ret = 0;
-    OSSL_PROPERTY_LIST *prop = NULL, *global;
-
-    if (store == NULL)
-        return 1;
-
-    ossl_property_write_lock(store);
-    ossl_method_cache_flush_all(store);
-    if (prop_query == NULL) {
-        ossl_property_free(store->global_properties);
-        store->global_properties = NULL;
-        goto success;
-    }
-    prop = ossl_parse_query(store->ctx, prop_query);
-    if (prop == NULL)
-        goto end;
-
-    if (store->global_properties == NULL) {
-        store->global_properties = prop;
-        prop = NULL;
-        goto success;
-    }
-    global = ossl_property_merge(prop, store->global_properties);
-    if (global == NULL)
-        goto end;
-    ossl_property_free(store->global_properties);
-    store->global_properties = global;
- success:
-    ret = 1;
- end:
-    ossl_property_unlock(store);
-    ossl_property_free(prop);
+    ossl_property_free(p2);
     return ret;
 }
 
-
 static void impl_cache_flush_alg(ossl_uintmax_t idx, ALGORITHM *alg)
 {
     lh_QUERY_doall(alg->cache, &impl_cache_free);
@@ -456,10 +410,12 @@ static void ossl_method_cache_flush(OSSL_METHOD_STORE *store, int nid)
     }
 }
 
-static void ossl_method_cache_flush_all(OSSL_METHOD_STORE *store)
+void ossl_method_store_flush_cache(OSSL_METHOD_STORE *store)
 {
+    ossl_property_write_lock(store);
     ossl_sa_ALGORITHM_doall(store->algs, &impl_cache_flush_alg);
     store->nelem = 0;
+    ossl_property_unlock(store);
 }
 
 IMPLEMENT_LHASH_DOALL_ARG(QUERY, IMPL_CACHE_FLUSH);
index 950a0c6c0722a1e5213b19eb40b67ae6fd771420..2b5a1e663eb056245d87395a2d7ddbc4a3eb2721 100644 (file)
@@ -22,8 +22,6 @@ OSSL_PROPERTY_IDX ossl_property_value(OPENSSL_CTX *ctx, const char *s,
 /* Property list functions */
 void ossl_property_free(OSSL_PROPERTY_LIST *p);
 int ossl_property_has_optional(const OSSL_PROPERTY_LIST *query);
-OSSL_PROPERTY_LIST *ossl_property_merge(const OSSL_PROPERTY_LIST *a,
-                                        const OSSL_PROPERTY_LIST *b);
 
 /* Property definition cache functions */
 OSSL_PROPERTY_LIST *ossl_prop_defn_get(OPENSSL_CTX *ctx, const char *prop);
index 2768524e0c5b6d15560dcf4f5b378197003b060e..53be60a9316d34347c98da98369583a650287cfc 100644 (file)
@@ -5,8 +5,8 @@
 OSSL_METHOD_STORE, ossl_method_store_new, ossl_method_store_free,
 ossl_method_store_init, ossl_method_store_cleanup,
 ossl_method_store_add, ossl_method_store_remove, ossl_method_store_fetch,
-ossl_method_store_set_global_properties,
-ossl_method_store_cache_get, ossl_method_store_cache_set
+ossl_method_store_cache_get, ossl_method_store_cache_set,
+ossl_method_store_flush_cache
 - implementation method store and query
 
 =head1 SYNOPSIS
@@ -28,14 +28,13 @@ ossl_method_store_cache_get, ossl_method_store_cache_set
  int ossl_method_store_fetch(OSSL_METHOD_STORE *store,
                              int nid, const char *properties,
                              void **method);
- int ossl_method_store_set_global_properties(OSSL_METHOD_STORE *store,
-                                            const char *prop_query);
  int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, int nid,
                                  const char *prop_query, void **method);
  int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
                                  const char *prop_query, void *method,
                                  int (*method_up_ref)(void *),
                                  void (*method_destruct)(void *));
+ void ossl_method_store_flush_cache(OSSL_METHOD_STORE *store);
 
 =head1 DESCRIPTION
 
@@ -82,10 +81,8 @@ ossl_method_store_fetch() queries I<store> for a method identified by I<nid>
 that matches the property query I<prop_query>.
 The result, if any, is returned in I<method>.
 
-ossl_method_store_set_global_properties() sets method I<store> wide query
-properties to I<prop_query>.
-All subsequent fetches will need to meet both these global query properties
-and the ones passed to the ossl_method_store_free().
+ossl_method_store_flush_cache() flushes all cached entries associated with
+I<store>.
 
 =head2 Cache Functions
 
@@ -107,7 +104,7 @@ ossl_method_store_new() returns a new method store object or NULL on failure.
 
 ossl_method_store_free(), ossl_method_store_add(),
 ossl_method_store_remove(), ossl_method_store_fetch(),
-ossl_method_store_set_global_properties(), ossl_method_store_cache_get()
+ossl_method_store_cache_get()
 and ossl_method_store_cache_set() return B<1> on success and B<0> on error.
 
 ossl_method_store_free() and ossl_method_store_cleanup() do not return any value.
index a4f18a5d3f5520bf93b7ca0a7a0e729a731cf3c7..5118bfbe575b14bb21b5aa37bc0a1d481414ab34 100644 (file)
@@ -158,7 +158,8 @@ typedef struct ossl_ex_data_global_st {
 # define OPENSSL_CTX_SERIALIZER_STORE_INDEX        10
 # define OPENSSL_CTX_SELF_TEST_CB_INDEX            11
 # define OPENSSL_CTX_BIO_PROV_INDEX                12
-# define OPENSSL_CTX_MAX_INDEXES                   13
+# define OPENSSL_CTX_GLOBAL_PROPERTIES             13
+# define OPENSSL_CTX_MAX_INDEXES                   14
 
 typedef struct openssl_ctx_method {
     void *(*new_func)(OPENSSL_CTX *ctx);
index 2b2332b237c1371167cfa533211d5dc4378bd83c..d8ff3582eb484e001d3f92d97d0a00bd957f9e6c 100644 (file)
@@ -42,13 +42,10 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
 int ossl_method_store_remove(OSSL_METHOD_STORE *store, int nid,
                              const void *method);
 int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
-                            const char *prop_query, void **result);
-int ossl_method_store_set_global_properties(OSSL_METHOD_STORE *store,
-                                            const char *prop_query);
-int ossl_method_store_merge_global_properties(OSSL_METHOD_STORE *store,
-                                              const char *prop_query);
-int ossl_method_store_global_property_is_enabled(OSSL_METHOD_STORE *store,
-                                                 const char *prop_name);
+                            const char *prop_query, void **method);
+
+/* Get the global properties associate with the specified library context */
+OSSL_PROPERTY_LIST **ossl_ctx_global_properties(OPENSSL_CTX *ctx);
 
 /* property query cache functions */
 int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, int nid,
@@ -57,4 +54,10 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
                                 const char *prop_query, void *result,
                                 int (*method_up_ref)(void *),
                                 void (*method_destruct)(void *));
+void ossl_method_store_flush_cache(OSSL_METHOD_STORE *store);
+
+/* Merge two property queries together */
+OSSL_PROPERTY_LIST *ossl_property_merge(const OSSL_PROPERTY_LIST *a,
+                                        const OSSL_PROPERTY_LIST *b);
+
 #endif
index a18b30e49750f81e1521db6e800641e757ceab2e..063cf8e2d843fed994e7b0eb65899446a7f5e4fd 100644 (file)
@@ -72,8 +72,6 @@ int ERR_load_EVP_strings(void);
 #  define EVP_F_EVP_KEYEXCH_FROM_DISPATCH                  0
 #  define EVP_F_EVP_MAC_CTRL                               0
 #  define EVP_F_EVP_MAC_CTRL_STR                           0
-#  define EVP_F_EVP_MAC_CTX_DUP                            0
-#  define EVP_F_EVP_MAC_CTX_NEW                            0
 #  define EVP_F_EVP_MAC_INIT                               0
 #  define EVP_F_EVP_MD_BLOCK_SIZE                          0
 #  define EVP_F_EVP_MD_CTX_COPY_EX                         0
@@ -178,6 +176,7 @@ int ERR_load_EVP_strings(void);
 # define EVP_R_CTRL_OPERATION_NOT_IMPLEMENTED             133
 # define EVP_R_DATA_NOT_MULTIPLE_OF_BLOCK_LENGTH          138
 # define EVP_R_DECODE_ERROR                               114
+# define EVP_R_DEFAULT_QUERY_PARSE_ERROR                  210
 # define EVP_R_DIFFERENT_KEY_TYPES                        101
 # define EVP_R_DIFFERENT_PARAMETERS                       153
 # define EVP_R_ERROR_LOADING_SECTION                      165
index 7f07ab738e1b3a9729715eedfd7022d3619ea43a..fe139fbf17c1290b7b528de3a907f8dad71857ab 100644 (file)
@@ -471,6 +471,35 @@ static EVP_PKEY *load_example_hmac_key(void)
     return pkey;
 }
 
+static int test_EVP_set_default_properties(void)
+{
+    OPENSSL_CTX *ctx;
+    EVP_MD *md = NULL;
+    int res = 0;
+
+    if (!TEST_ptr(ctx = OPENSSL_CTX_new())
+            || !TEST_ptr(md = EVP_MD_fetch(ctx, "sha256", NULL)))
+        goto err;
+    EVP_MD_free(md);
+    md = NULL;
+
+    if (!TEST_true(EVP_set_default_properties(ctx, "provider=fizzbang"))
+            || !TEST_ptr_null(md = EVP_MD_fetch(ctx, "sha256", NULL))
+            || !TEST_ptr(md = EVP_MD_fetch(ctx, "sha256", "-provider")))
+        goto err;
+    EVP_MD_free(md);
+    md = NULL;
+
+    if (!TEST_true(EVP_set_default_properties(ctx, NULL))
+            || !TEST_ptr(md = EVP_MD_fetch(ctx, "sha256", NULL)))
+        goto err;
+    res = 1;
+err:
+    EVP_MD_free(md);
+    OPENSSL_CTX_free(ctx);
+    return res;
+}
+
 static int test_EVP_Enveloped(void)
 {
     int ret = 0;
@@ -1748,6 +1777,7 @@ int setup_tests(void)
     if (!TEST_ptr(testctx))
         return 0;
 
+    ADD_TEST(test_EVP_set_default_properties);
     ADD_ALL_TESTS(test_EVP_DigestSignInit, 9);
     ADD_TEST(test_EVP_DigestVerifyInit);
     ADD_TEST(test_EVP_Enveloped);