-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Always use SNI for TLS enabled Pulsar Java broker client. #8117
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
/pulsarbot run-failure-checks |
3 similar comments
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
c329518
to
ed4acbb
Compare
/pulsarbot run-failure-checks |
5 similar comments
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
@rdhabalia Thanks. Finally got all checks to pass. |
### Motivation Improve previous PR #8117 (Always use SNI for TLS enabled Java client) ### Modifications - Use `ChannelFutures.toCompletableFuture` instead of private static utility method. - When TLS is not enabled, use 'original' code that invokes `Bootstrap.connect(InetSocketAddress)`; it is only when TLS is enabled we need custom setup code to properly set SNI headers. - Add documentation and argument checks to `PulsarChannelInitializer.initTls`
Co-authored-by: Rolf Arne Corneliussen <rolf.arne.corneliussen@addsecure.com>
### Motivation Improve previous PR apache#8117 (Always use SNI for TLS enabled Java client) ### Modifications - Use `ChannelFutures.toCompletableFuture` instead of private static utility method. - When TLS is not enabled, use 'original' code that invokes `Bootstrap.connect(InetSocketAddress)`; it is only when TLS is enabled we need custom setup code to properly set SNI headers. - Add documentation and argument checks to `PulsarChannelInitializer.initTls`
Move this change to 2.6.2, because the #8177 depends on this pr. |
Co-authored-by: Rolf Arne Corneliussen <rolf.arne.corneliussen@addsecure.com> (cherry picked from commit f2933f7)
### Motivation Improve previous PR #8117 (Always use SNI for TLS enabled Java client) ### Modifications - Use `ChannelFutures.toCompletableFuture` instead of private static utility method. - When TLS is not enabled, use 'original' code that invokes `Bootstrap.connect(InetSocketAddress)`; it is only when TLS is enabled we need custom setup code to properly set SNI headers. - Add documentation and argument checks to `PulsarChannelInitializer.initTls` (cherry picked from commit 1af5c8e)
Co-authored-by: Rolf Arne Corneliussen <rolf.arne.corneliussen@addsecure.com>
### Motivation Improve previous PR apache#8117 (Always use SNI for TLS enabled Java client) ### Modifications - Use `ChannelFutures.toCompletableFuture` instead of private static utility method. - When TLS is not enabled, use 'original' code that invokes `Bootstrap.connect(InetSocketAddress)`; it is only when TLS is enabled we need custom setup code to properly set SNI headers. - Add documentation and argument checks to `PulsarChannelInitializer.initTls`
Motivation
The Java Pulsar client does not currently set the SNI header when it creates TLS connections using the binary protocol to brokers (except when using proxyUrl with SNI routing).
If the client always set the SNI header, it can enable ingress routing using reverse proxies like HAProxy, possibly in combination with external advertised addresses (PIP-61).
Modifications
The main modification is to always create
SslEngine
s with advisory peer information (peer host and port).org.apache.pulsar.client.impl.PulsarChannelInitializer
modified to set up the SslHandler after the Netty channel is registered. A new methodCompletableFuture<Channel> initTls(Channel ch, InetSocketAddress sniHost)
was added to explicitly specify the remote peer.org.apache.pulsar.client.impl.ConnectionPool
modified to always invokePulsarChannelInitializer.initTls
with a remote peer if TLS is enabled.Added method
public SSLEngine createSSLEngine(String peerHost, int peerPort)
toorg.apache.pulsar.common.util.keystoretls.KeyStoreSSLContext
so SNI header can be set irrespective of using OpenSSL or internal Java TLS.Verifying this change
org.apache.pulsar.client.api.TlsSniTest
to verity that using an IP-address in the brokerServiceUrl does not cause problems.