crypto: use X509_V_FLAG_TRUSTED_FIRST for tls #457
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
Cherry-pick the X509_V_FLAG_TRUSTED_FIRST patch from upstream OpenSSL. This flag tells OpenSSL to check the CA chain against the trusted store first.
This reverts commit 0926cb9. This will be addressed by using the X509_V_FLAG_TRUSTED_FIRST flag during verification.
Tell OpenSSL to check the CA chain against the certificates in the trusted store first. It's possible to connect with https://bbuseruploads.s3.amazonaws.com/ again now, even though it uses a deprecated 1024 bits RSA certificate in its CA chain.
|
@bnoordhuis I'm kind of -1 on this. We already got a reply from OpenSSL dev team that this fix is kind of dangerous, because it is exposing timing of CA store lookup. Let's wait for reply from the OpenSSL before continuing with this. |
|
Ah, I do wish people would hit reply to all when answering a mailing list post, I didn't receive the reply. For posterity: http://www.mail-archive.com/[email protected]/msg37892.html |
|
Aaah, sorry. I didn't know that you wasn't in recipient list. |
|
I also had a trouble to receive the openssl-dev mail last night. Seeing them, let's wait for a while. I've waited years for openssl-1.0.2 release to use ALPN. |
|
@shigeki I have requested comments for my patch too, may be we could adapt it for our purposes somehow before 1.0.2 will see the true fix. |
|
@indutny Indeed. Applying RT3621 is a good choice after reviewed. We should prepare to manage our private patches in deps/openssl not to forget to apply them in future updates. |
|
I'm closing this PR. We can continue the discussion in #402. |
rb-cohen
pushed a commit
to rb-cohen/node
that referenced
this pull request
Mar 4, 2022
Original commit message: Asterisks should be allowed in host validation as CNAMEs may reference wildcard domains CloudFlare appears to use this logic in CNAMEs as per nodejs#42171 Fixes: nodejs#457 Fix By: Brad House (@bradh352)
rb-cohen
pushed a commit
to rb-cohen/node
that referenced
this pull request
Mar 4, 2022
Original commit message: Asterisks should be allowed in host validation as CNAMEs may reference wildcard domains CloudFlare appears to use this logic in CNAMEs as per nodejs#42171 Fixes: nodejs#457 Fix By: Brad House (@bradh352)
benjamingr
pushed a commit
that referenced
this pull request
Mar 13, 2022
Original commit message: Asterisks should be allowed in host validation as CNAMEs may reference wildcard domains CloudFlare appears to use this logic in CNAMEs as per #42171 Fixes: c-ares/c-ares#457 Fix By: Brad House (@bradh352) PR-URL: #42216 Fixes: #42171 Fixes: #457 Refs: c-ares/c-ares#457 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
bengl
pushed a commit
that referenced
this pull request
Mar 21, 2022
Original commit message: Asterisks should be allowed in host validation as CNAMEs may reference wildcard domains CloudFlare appears to use this logic in CNAMEs as per #42171 Fixes: c-ares/c-ares#457 Fix By: Brad House (@bradh352) PR-URL: #42216 Fixes: #42171 Fixes: #457 Refs: c-ares/c-ares#457 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
danielleadams
pushed a commit
to danielleadams/node
that referenced
this pull request
Apr 21, 2022
Original commit message: Asterisks should be allowed in host validation as CNAMEs may reference wildcard domains CloudFlare appears to use this logic in CNAMEs as per nodejs#42171 Fixes: c-ares/c-ares#457 Fix By: Brad House (@bradh352) PR-URL: nodejs#42216 Fixes: nodejs#42171 Fixes: nodejs#457 Refs: c-ares/c-ares#457 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
danielleadams
pushed a commit
that referenced
this pull request
Apr 24, 2022
Original commit message: Asterisks should be allowed in host validation as CNAMEs may reference wildcard domains CloudFlare appears to use this logic in CNAMEs as per #42171 Fixes: c-ares/c-ares#457 Fix By: Brad House (@bradh352) PR-URL: #42216 Fixes: #42171 Fixes: #457 Refs: c-ares/c-ares#457 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
danielleadams
pushed a commit
that referenced
this pull request
Apr 24, 2022
Original commit message: Asterisks should be allowed in host validation as CNAMEs may reference wildcard domains CloudFlare appears to use this logic in CNAMEs as per #42171 Fixes: c-ares/c-ares#457 Fix By: Brad House (@bradh352) PR-URL: #42216 Fixes: #42171 Fixes: #457 Refs: c-ares/c-ares#457 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
danielleadams
pushed a commit
that referenced
this pull request
Apr 24, 2022
Original commit message: Asterisks should be allowed in host validation as CNAMEs may reference wildcard domains CloudFlare appears to use this logic in CNAMEs as per #42171 Fixes: c-ares/c-ares#457 Fix By: Brad House (@bradh352) PR-URL: #42216 Fixes: #42171 Fixes: #457 Refs: c-ares/c-ares#457 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
xtx1130
pushed a commit
to xtx1130/node
that referenced
this pull request
Apr 25, 2022
Original commit message: Asterisks should be allowed in host validation as CNAMEs may reference wildcard domains CloudFlare appears to use this logic in CNAMEs as per nodejs#42171 Fixes: c-ares/c-ares#457 Fix By: Brad House (@bradh352) PR-URL: nodejs#42216 Fixes: nodejs#42171 Fixes: nodejs#457 Refs: c-ares/c-ares#457 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
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.
Tell OpenSSL to check the CA chain against the certificates in the
trusted store first.
It's possible to connect with https://bbuseruploads.s3.amazonaws.com/
again now, even though it uses a deprecated 1024 bits RSA certificate
in its CA chain.
R=@indutny, /cc @shigeki
Before merging this, I would like some discussion on whether X509_V_FLAG_TRUSTED_FIRST is really the best possible approach. I raised some questions about it here.