-
Notifications
You must be signed in to change notification settings - Fork 7.3k
tls: Re-enable checking of CN-ID in server identity verification #5221
Conversation
Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion. Commit tmuellerleile/node@2ae1963 has the following error(s):
The following commiters were not found in the CLA:
Please see CONTRIBUTING.md for more information |
RFC 6125 explicitly states that a client "MUST NOT seek a match for a reference identifier of CN-ID if the presented identifiers include a DNS-ID, SRV-ID, URI-ID, or any application-specific identifier types supported by the client", but it MAY do so if none of the mentioned identifier types (but others) are present.
Fixed commit message wording after Node-Jenkins' hint. |
It is also of note that all tests in test/simple/test-tls-check-server-identity.js still pass after applying the proposed change. Furthermore I think that this patch follows the intention of b4b750b by @indutny as the check in line 167:
is basically unnecessary in the current code, since |
I did some analysis of what chromium does for certs, I suggest to follow this criterias:
Therefore this commit looks good to me. Landed with some style fixes in f8bd53bb68216789efaf71e4376b5741f8472b16, thank you! |
Backported to v0.8 - 1a95ce5. |
As requested in nodejs#5221 Original commit message: fix debug command processor wrt restart frame. [email protected] BUG=v8:4757 LOG=N Review URL: https://codereview.chromium.org/1700693002 Cr-Commit-Position: refs/heads/master@{#33983} PR-URL: nodejs/node#6086 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
As requested in nodejs#5221 Original commit message: fix debug command processor wrt restart frame. [email protected] BUG=v8:4757 LOG=N Review URL: https://codereview.chromium.org/1700693002 Cr-Commit-Position: refs/heads/master@{#33983} PR-URL: nodejs/node#6086 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
As requested in nodejs#5221 Original commit message: fix debug command processor wrt restart frame. [email protected] BUG=v8:4757 LOG=N Review URL: https://codereview.chromium.org/1700693002 Cr-Commit-Position: refs/heads/master@{#33983} PR-URL: nodejs/node#6086 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
As requested in nodejs#5221 Original commit message: fix debug command processor wrt restart frame. [email protected] BUG=v8:4757 LOG=N Review URL: https://codereview.chromium.org/1700693002 Cr-Commit-Position: refs/heads/master@{#33983} PR-URL: nodejs/node#6086 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
As requested in nodejs#5221 Original commit message: fix debug command processor wrt restart frame. [email protected] BUG=v8:4757 LOG=N Review URL: https://codereview.chromium.org/1700693002 Cr-Commit-Position: refs/heads/master@{#33983} PR-URL: nodejs/node#6086 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
As requested in nodejs#5221 Original commit message: fix debug command processor wrt restart frame. [email protected] BUG=v8:4757 LOG=N Review URL: https://codereview.chromium.org/1700693002 Cr-Commit-Position: refs/heads/master@{#33983} PR-URL: nodejs/node#6086 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
As requested in nodejs#5221 Original commit message: fix debug command processor wrt restart frame. [email protected] BUG=v8:4757 LOG=N Review URL: https://codereview.chromium.org/1700693002 Cr-Commit-Position: refs/heads/master@{#33983} PR-URL: nodejs/node#6086 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
RFC 6125 explicitly states that a client "MUST NOT seek a match
for a reference identifier of CN-ID if the presented identifiers
include a DNS-ID, SRV-ID, URI-ID, or any application-specific
identifier types supported by the client", but it MAY do so if
none of the mentioned identifier types (but others) are present.
As certificates with unsupported identifiers in altnames (e. g.
email:
) are not uncommon, we should be more liberal by resorting to the (deprecated) CN-ID based verification in these cases.