Do not lookup IP addresses of X509 certificate subject CNs #1967
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
A true-vs-false `nodns` parameter value bug in a recent commit 22b2a7a caused, in some environments, significant startup delays and/or runtime stalls because getaddrinfo(3) performed blocking DNS lookups when parsing common names of X509 certificate subjects. Squid parses CNs when loading configured and validating received certificates. Other side effects may have included Squid-generated certificates having wrong alternative subject names and/or wrong certificate validation results. Negative names and context-disassociated boolean constants strike again! Fortunately, associated problematic Ip::Address::lookupHostIP() will be replaced when the existing Ip::Address::Parse() TODO is addressed.
rousskov
commented
Dec 19, 2024
There was a problem hiding this comment.
@eduard-bagdasaryan, thank you for discovering and triaging this bug! Please check whether this PR addresses the problems you could reproduce in your environment.
rousskov
added
the
S-waiting-for-reviewer
eduard-bagdasaryan
approved these changes
Dec 20, 2024
rousskov
added
M-cleared-for-merge
yadij
approved these changes
Dec 27, 2024
yadij
removed
the
S-could-use-an-approval
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.
A true-vs-false
nodnsparameter value bug in a recent commit 22b2a7acaused, in some environments, significant startup delays and/or runtime
stalls because getaddrinfo(3) performed blocking DNS lookups when
parsing common names of X509 certificate subjects. Squid parses CNs when
loading configured and validating received certificates. Other side
effects may have included Squid-generated certificates having wrong
alternative subject names and/or wrong certificate validation results.
Negative names and context-disassociated boolean constants strike again!
Fortunately, associated problematic Ip::Address::lookupHostIP() will be
replaced when the existing Ip::Address::Parse() TODO is addressed.