Client connections created for TLS-based replication use SNI if possible #11458
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
yossigo
added
the
release-notes
yossigo
reviewed
Nov 2, 2022
There was a problem hiding this comment.
@CatboxParadox Thanks, this makes sense. Technically, this could be considered a breaking change in extreme cases but I think we can accept it as-is and not make it optional. @redis/core-team any other opinions?
yossigo
added
the
breaking-change
|
@yossigo for completeness, can you describe the case and how it'll break? |
|
@oranagra It's a breaking change because Redis did not include an SNI in the past and will include it now (when connecting to a hostname, not an IP). Forwarding the connections through a proxy may result in different routing and subsequent connectivity issues, depending on how the proxy is configured. |
zeekling
approved these changes
Nov 5, 2022
|
conceptually approved in a core team meeting for 7.2 |
yossigo
approved these changes
Nov 27, 2022
Mixficsol
pushed a commit
to Mixficsol/redis
that referenced
this pull request
Apr 12, 2023
When establishing an outgoing TLS connection using a hostname as a target, use TLS SNI extensions to include the hostname in use.
madolson
pushed a commit
to madolson/redis
that referenced
this pull request
Apr 19, 2023
When establishing an outgoing TLS connection using a hostname as a target, use TLS SNI extensions to include the hostname in use.
enjoy-binbin
pushed a commit
to enjoy-binbin/redis
that referenced
this pull request
Jul 31, 2023
When establishing an outgoing TLS connection using a hostname as a target, use TLS SNI extensions to include the hostname in use.
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.
It is not possible to reliably select the target server instance for TLS replication in case that the target sits behind a TLS (terminating or pass-through) proxy, as there's no SNI extension in the ClientHello message sent via the connection created for the sake of replication.
This is a simple attempt at setting the SNI to the value of the master server's address, in case that address is not actually an IP