Skip to content

Commit

Permalink
Make the activate setting more intuitive
Browse files Browse the repository at this point in the history
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 #22906)
  • Loading branch information
nhorman committed Dec 21, 2023
1 parent 5528bfb commit 506ff20
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 9 deletions.
8 changes: 8 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
38 changes: 33 additions & 5 deletions crypto/provider_conf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 5 additions & 2 deletions doc/man5/config.pod
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion test/default-and-fips.cnf
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ default = default_sect
fips = fips_sect

[default_sect]
activate = 1
activate = yes
6 changes: 5 additions & 1 deletion test/default.cnf
Original file line number Diff line number Diff line change
Expand Up @@ -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
22 changes: 22 additions & 0 deletions test/evp_fetch_prov_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 506ff20

Please sign in to comment.