Make the activate setting more intuitive #22906
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
t8m
reviewed
Dec 4, 2023
t8m
added
branch: master
|
OTC: What is the appropriate behavior for various bogus values of the activate key in a provider section? |
|
I'd allow a reasonable set of yes/no pairs and error out on unknown values. |
|
I don't think we can should error out on unknown values as that would be a behavioral change from the test if the 3.x stream. I think any such change would have to wait until 4.0 I think |
mattcaswell
requested changes
Dec 11, 2023
|
OTC: We should add explicit values for disabling activation and report an error on unknown values. |
|
fixed activation setting to be in line with OTC response |
mspncp
reviewed
Dec 20, 2023
t8m
requested changes
Dec 20, 2023
mspncp
requested changes
Dec 20, 2023
t8m
added
approval: review pending
t8m
approved these changes
Dec 20, 2023
t8m
removed
the
approval: otc review pending
mattcaswell
approved these changes
Dec 20, 2023
mattcaswell
added
approval: done
mspncp
approved these changes
Dec 20, 2023
openssl-machine
added
approval: ready to merge
|
This pull request is ready to merge |
openssl-machine
pushed a commit
that referenced
this pull request
Dec 21, 2023
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)
|
merged |
wanghao75
pushed a commit
to openeuler-mirror/openssl
that referenced
this pull request
Dec 22, 2023
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 openssl/openssl#22906) Signed-off-by: lanming1120 <lanming1120@126.com>
wbeck10
pushed a commit
to wbeck10/openssl
that referenced
this pull request
Jan 8, 2024
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 openssl#22906)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Checklist