Fix server identity check #2253
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
KunZhou-at
changed the title
KunZhou-at
changed the title
|
Also looking for this behavior--is there any timeline for when this might get reviewed/merged? @wellwelwel Thanks! |
Hi @mikewang333, I think there's an ongoing discussion about it in #2172. But in any case, these changes looks good to me 🚀 In addition, #2295 seems to do the same in another way when reviewing its changes. |
|
closing this as a duplicate of #2376 Sorry @KunZhou-at your PR didn't get enough attention |
Closed
|
@KunZhou-at, I noticed you brought it back to #2594. Could you explain the difference to PR #2376? |
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.
I don't think we are respecting the documented behavior for verifyIdentity. According to the documentation, in order to skip server identity check, we need to override checkServerIdentity by passing in a function as an argument to
tls.connect. Currently we do that in the callback by which time the server identity check has already happened and error bubbled up.