crypto: Read OpenSSL config before init #6374
Conversation
The OpenSSL configuration file allows custom crypto engines but those directives will not be respected if the config file is loaded after initializing all crypto subsystems. This patch reads the configuration file first.
stefanmb
added
tls
|
LGTM if CI is green. |
|
would this actually be semver-minor or major? |
|
I'd say minor |
|
CI is green. I will land in 48 hrs if there are no objections. @jasnell I labeled it as a semver-minor since the functionality already exists in master since #5181, reordering the order of the calls does not break any existing interface/contract, but enables other uses (like the GOST engine). Please also note I've fixed the labels - this PR should not be landed in v4.x and v5.x since they do not read the OpenSSL config file at all - adding that functionality would be semver-major (which is why it was not done in the first place pre 6.x). |
|
Ok, SGTM! |
|
LGTM |
1 similar comment
|
LGTM |
estliberitas
force-pushed
the
master
branch
2 times, most recently
from
7da4fd4
to
c7066fb
Compare
|
@stefanmb ... I'm going to go ahead and land this now so it makes it into v6 |
The OpenSSL configuration file allows custom crypto engines but those directives will not be respected if the config file is loaded after initializing all crypto subsystems. This patch reads the configuration file first. PR-URL: #6374 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 56b9478 |
|
@jasnell Thank you! I wanted to go by the book and wait for 48 hrs but I think this should make it into 6 on day one. |
The OpenSSL configuration file allows custom crypto engines but those directives will not be respected if the config file is loaded after initializing all crypto subsystems. This patch reads the configuration file first. PR-URL: #6374 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
@Anatoliy4041 |
|
@fast0490f Thanks, I appreciate it 👍 . However I think It sould be applied it the latest versions of NodeJS, shouldn't it? |
|
@indutny --openssl-config |
Checklist
Affected core subsystem(s)
crypto
Description of change
The OpenSSL configuration file allows a directive for a custom crypto engine to be used, but such a directive will not be respected if the config file is loaded after initializing all crypto subsystems.
This PR reads the configuration file first, and is meant to directly address the issues raised by @burmisov and @fast0490f in #5101.
The previous PR (#5739) proposed to address the same problem introduces new runtime argument which is not strictly necessary and thus has a higher impact.
In this PR I've opted, after discussion, to propose the minimal possible change to resolve the issue reported with the GOST engine support. For more information, please see #5739 (comment).