Make the activate setting more intuitive

Currently, a provider is activated from our config file using the
activate parameter.  However, the presence of the config parameter is
sufficient to trigger activation, leading to a counterintuitive
situation in which setting "activate = 0" still activates the provider

Make activation more intuitive by requiring that activate be set to one
of yes|true|1 to trigger activation.  Any other value, as well as
omitting the parameter entirely, prevents activation (and also maintains
backward compatibility.

It seems a bit heavyweight to create a test specifically to validate the
plurality of these settings.  Instead, modify the exiting openssl config
files in the test directory to use variants of these settings, and
augment the default.cnf file to include a provider section that is
explicitly disabled

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22906)

diff --git a/CHANGES.md b/CHANGES.md
index f1c8bce1b34f597512710c2c9f17bb78339e66b3..0f8db22dced5f24ba1f8c123816935b2d2c61371 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -28,6 +28,14 @@ OpenSSL 3.3
 
 ### Changes between 3.2 and 3.3 [xx XXX xxxx]
 
+ * The activate configuration setting for providers in openssl.cnf has been
+   updated to require a value of [1|yes|true|on] (in lower or UPPER case) to
+   activate the provider.  Conversely a setting [0|no|false|off] will prevent
+   provider activation.  All other values, or the omission of a value for this
+   setting will result in an error.
+
+    *Neil Horman*
+
  * In `openssl speed`, changed the default hash function used with `hmac` from
    `md5` to `sha256`.
 

diff --git a/crypto/provider_conf.c b/crypto/provider_conf.c
index 288ade6b4de25bce61d438669c734ad65e4e5071..d8454b7941c4cf08288d845757ea5a01885d3554 100644 (file)
--- a/crypto/provider_conf.c
+++ b/crypto/provider_conf.c
@@ -236,15 +236,43 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
         /* First handle some special pseudo confs */
 
         /* Override provider name to use */
-        if (strcmp(confname, "identity") == 0)
+        if (strcmp(confname, "identity") == 0) {
             name = confvalue;
-        else if (strcmp(confname, "soft_load") == 0)
+        } else if (strcmp(confname, "soft_load") == 0) {
             soft = 1;
         /* Load a dynamic PROVIDER */
-        else if (strcmp(confname, "module") == 0)
+        } else if (strcmp(confname, "module") == 0) {
             path = confvalue;
-        else if (strcmp(confname, "activate") == 0)
-            activate = 1;
+        } else if (strcmp(confname, "activate") == 0) {
+            if (confvalue == NULL) {
+                ERR_raise_data(ERR_LIB_CRYPTO, CRYPTO_R_PROVIDER_SECTION_ERROR,
+                               "section=%s activate set to unrecognized value",
+                               value);
+                return 0;
+            }
+            if ((strcmp(confvalue, "1") == 0)
+                || (strcmp(confvalue, "yes") == 0)
+                || (strcmp(confvalue, "YES") == 0)
+                || (strcmp(confvalue, "true") == 0)
+                || (strcmp(confvalue, "TRUE") == 0)
+                || (strcmp(confvalue, "on") == 0)
+                || (strcmp(confvalue, "ON") == 0)) {
+                activate = 1;
+            } else if ((strcmp(confvalue, "0") == 0)
+                       || (strcmp(confvalue, "no") == 0)
+                       || (strcmp(confvalue, "NO") == 0)
+                       || (strcmp(confvalue, "false") == 0)
+                       || (strcmp(confvalue, "FALSE") == 0)
+                       || (strcmp(confvalue, "off") == 0)
+                       || (strcmp(confvalue, "OFF") == 0)) {
+                activate = 0;
+            } else {
+                ERR_raise_data(ERR_LIB_CRYPTO, CRYPTO_R_PROVIDER_SECTION_ERROR,
+                               "section=%s activate set to unrecognized value",
+                               value);
+                return 0;
+            }
+        }
     }
 
     if (activate) {

diff --git a/doc/man5/config.pod b/doc/man5/config.pod
index 8d312c661fa099372c7a50004e4fd98d3efe216f..96eaa6ffd392715072b9ea87a76457cf46c6d335 100644 (file)
--- a/doc/man5/config.pod
+++ b/doc/man5/config.pod
@@ -265,8 +265,11 @@ Specifies the pathname of the module (typically a shared library) to load.
 
 =item B<activate>
 
-If present, the module is activated. The value assigned to this name is not
-significant.
+If present and set to one of the values yes, on, true or 1, then the associated
+provider will be activated. Conversely, setting this value to no, off, false, or
+0 will prevent the provider from being activated. Settings can be given in lower
+or uppercase. Setting activate to any other setting, or omitting a setting
+value will result in an error.
 
 =back
 

diff --git a/test/default-and-fips.cnf b/test/default-and-fips.cnf
index 2ca6487fd2fdda2bb5a69d5588749502d94414b7..71af0a385abba3e137cac18238ab0574a72943f8 100644 (file)
--- a/test/default-and-fips.cnf
+++ b/test/default-and-fips.cnf
@@ -13,4 +13,4 @@ default = default_sect
 fips = fips_sect
 
 [default_sect]
-activate = 1
+activate = yes

diff --git a/test/default.cnf b/test/default.cnf
index f29d0e92bae8e8c75496ab6e2240e1b3d22b8eed..21c7e070a9e0048f7ba57b9c2bd5985fa869c4f8 100644 (file)
--- a/test/default.cnf
+++ b/test/default.cnf
@@ -8,6 +8,10 @@ providers = provider_sect
 
 [provider_sect]
 default = default_sect
+legacy  = legacy_sect
 
 [default_sect]
-activate = 1
+activate = true
+
+[legacy_sect]
+activate = false

diff --git a/test/evp_fetch_prov_test.c b/test/evp_fetch_prov_test.c
index 19e780d8566673bdafe4c0380549fc04da0afbb9..876d6ccc0a3d9c2ddf04f5ed341a101635cc842c 100644 (file)
--- a/test/evp_fetch_prov_test.c
+++ b/test/evp_fetch_prov_test.c
@@ -121,6 +121,27 @@ static void unload_providers(OSSL_LIB_CTX **libctx, OSSL_PROVIDER *prov[])
     }
 }
 
+static int test_legacy_provider_unloaded(void)
+{
+    OSSL_LIB_CTX *ctx = NULL;
+    int rc = 0;
+
+    ctx = OSSL_LIB_CTX_new();
+    if (!TEST_ptr(ctx))
+        goto err;
+
+    if (!TEST_true(OSSL_LIB_CTX_load_config(ctx, config_file)))
+        goto err;
+
+    if (!TEST_int_eq(OSSL_PROVIDER_available(ctx, "legacy"), 0))
+        goto err;
+
+    rc = 1;
+err:
+    OSSL_LIB_CTX_free(ctx);
+    return rc;
+}
+
 static X509_ALGOR *make_algor(int nid)
 {
     X509_ALGOR *algor;
@@ -379,6 +400,7 @@ int setup_tests(void)
             return 0;
         }
     }
+    ADD_TEST(test_legacy_provider_unloaded);
     if (strcmp(alg, "digest") == 0) {
         ADD_TEST(test_implicit_EVP_MD_fetch);
         ADD_TEST(test_explicit_EVP_MD_fetch_by_name);