You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/03/05 17:35:37 UTC

[GitHub] [pulsar] nodece opened a new pull request #14569: [Broker] Add ssl provider, ciphers and protocols support for broker service and proxy service

nodece opened a new pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569


   ### Motivation
   
   The broker service and proxy service didn't full-support the ssl provider, ciphers and protocols config when using ca-cert. 
   
   We are getting an error when we try to use certain TLS ciphers in Pulsar Broker:
   TLS_DHE_RSA_WITH_AES_256_GCM_SHA384
   TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384
   TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
   TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
   
   ```
   2022-03-02T18:27:10,165 [pulsar-client-io-60-1] INFO  org.apache.pulsar.client.impl.ConnectionPool - [[id: 0xc25db0d7, L:/172.16.1.50:48122 - R:172.16.1.50/172.16.1.50:6651]] Connected to server
   2022-03-02T18:27:10,179 [pulsar-io-4-8] ERROR org.apache.pulsar.common.util.SslContextAutoRefreshBuilder - Exception while trying to refresh ssl Context failed to set cipher suite: [TLS_DHE_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384]
   javax.net.ssl.SSLException: failed to set cipher suite: [TLS_DHE_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384]
   	at io.netty.handler.ssl.ReferenceCountedOpenSslContext.<init>(ReferenceCountedOpenSslContext.java:315) ~[io.netty-netty-handler-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.handler.ssl.OpenSslContext.<init>(OpenSslContext.java:45) ~[io.netty-netty-handler-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:349) ~[io.netty-netty-handler-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:336) ~[io.netty-netty-handler-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.handler.ssl.SslContext.newServerContextInternal(SslContext.java:473) ~[io.netty-netty-handler-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.handler.ssl.SslContextBuilder.build(SslContextBuilder.java:606) ~[io.netty-netty-handler-4.1.72.Final.jar:4.1.72.Final]
   	at org.apache.pulsar.common.util.SecurityUtility.createNettySslContextForServer(SecurityUtility.java:300) ~[org.apache.pulsar-pulsar-common-2.8.2.jar:2.8.2]
   	at org.apache.pulsar.common.util.NettyServerSslContextBuilder.update(NettyServerSslContextBuilder.java:57) ~[org.apache.pulsar-pulsar-common-2.8.2.jar:2.8.2]
   	at org.apache.pulsar.common.util.NettyServerSslContextBuilder.update(NettyServerSslContextBuilder.java:31) ~[org.apache.pulsar-pulsar-common-2.8.2.jar:2.8.2]
   	at org.apache.pulsar.common.util.SslContextAutoRefreshBuilder.get(SslContextAutoRefreshBuilder.java:79) [org.apache.pulsar-pulsar-common-2.8.2.jar:2.8.2]
   	at org.apache.pulsar.broker.service.PulsarChannelInitializer.initChannel(PulsarChannelInitializer.java:115) [org.apache.pulsar-pulsar-broker-2.8.2.jar:2.8.2]
   	at org.apache.pulsar.broker.service.PulsarChannelInitializer.initChannel(PulsarChannelInitializer.java:43) [org.apache.pulsar-pulsar-broker-2.8.2.jar:2.8.2]
   	at io.netty.channel.ChannelInitializer.initChannel(ChannelInitializer.java:129) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.ChannelInitializer.handlerAdded(ChannelInitializer.java:112) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.AbstractChannelHandlerContext.callHandlerAdded(AbstractChannelHandlerContext.java:938) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.DefaultChannelPipeline.callHandlerAdded0(DefaultChannelPipeline.java:609) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.DefaultChannelPipeline.access$100(DefaultChannelPipeline.java:46) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.DefaultChannelPipeline$PendingHandlerAddedTask.execute(DefaultChannelPipeline.java:1463) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.DefaultChannelPipeline.callHandlerAddedForAllHandlers(DefaultChannelPipeline.java:1115) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.DefaultChannelPipeline.invokeHandlerAddedIfNeeded(DefaultChannelPipeline.java:650) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.AbstractChannel$AbstractUnsafe.register0(AbstractChannel.java:514) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.AbstractChannel$AbstractUnsafe.access$200(AbstractChannel.java:429) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.AbstractChannel$AbstractUnsafe$1.run(AbstractChannel.java:486) [io.netty-netty-transport-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:164) [io.netty-netty-common-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:469) [io.netty-netty-common-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:384) [io.netty-netty-transport-classes-epoll-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:986) [io.netty-netty-common-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) [io.netty-netty-common-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [io.netty-netty-common-4.1.72.Final.jar:4.1.72.Final]
   	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_302]
   Caused by: java.lang.IllegalArgumentException: unsupported cipher suite: TLS_DHE_RSA_WITH_AES_256_GCM_SHA384(DHE-RSA-AES256-GCM-SHA384)
   	at io.netty.handler.ssl.CipherSuiteConverter.convertToCipherStrings(CipherSuiteConverter.java:472) ~[io.netty-netty-handler-4.1.72.Final.jar:4.1.72.Final]
   	at io.netty.handler.ssl.ReferenceCountedOpenSslContext.<init>(ReferenceCountedOpenSslContext.java:301) ~[io.netty-netty-handler-4.1.72.Final.jar:4.1.72.Final]
   	... 29 more
   ```
   
   
   ### Modifications
   
   -  Add `brokerServiceSslProvider` to proxy and broker config
   - Fix full-support the ssl provider, ciphers and protocols in broker and proxy service
   
   ### Documentation
   
   Check the box below or label this PR directly (if you have committer privilege).
   
   Need to update docs? 
   
   - [x] `doc-required` 
     
   Add `brokerServiceSslProvider` to proxy and broker config.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r828013359



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarChannelInitializer.java
##########
@@ -92,19 +93,36 @@ public PulsarChannelInitializer(ClientConfigurationData conf, Supplier<ClientCnx
 
             sslContextSupplier = new ObjectCache<SslContext>(() -> {
                 try {
+                    SslProvider sslProvider = null;
+                    if (conf.getSslProvider() != null) {
+                        sslProvider = SslProvider.valueOf(conf.getSslProvider());
+                    }
+
                     // Set client certificate if available
                     AuthenticationDataProvider authData = conf.getAuthentication().getAuthData();
                     if (authData.hasDataForTls()) {
                         return authData.getTlsTrustStoreStream() == null
-                                ? SecurityUtility.createNettySslContextForClient(conf.isTlsAllowInsecureConnection(),
-                                        conf.getTlsTrustCertsFilePath(),
-                                        authData.getTlsCertificates(), authData.getTlsPrivateKey())
-                                : SecurityUtility.createNettySslContextForClient(conf.isTlsAllowInsecureConnection(),
-                                        authData.getTlsTrustStoreStream(),
-                                        authData.getTlsCertificates(), authData.getTlsPrivateKey());
+                                ? SecurityUtility.createNettySslContextForClient(
+                                sslProvider,
+                                conf.isTlsAllowInsecureConnection(),
+                                conf.getTlsTrustCertsFilePath(),
+                                authData.getTlsCertificates(),
+                                authData.getTlsPrivateKey(),
+                                conf.getTlsCiphers(),
+                                conf.getTlsProtocols())
+                                : SecurityUtility.createNettySslContextForClient(sslProvider,
+                                conf.isTlsAllowInsecureConnection(),
+                                authData.getTlsTrustStoreStream(),
+                                authData.getTlsCertificates(), authData.getTlsPrivateKey(),
+                                conf.getTlsCiphers(),

Review comment:
       Ok, I see.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1084215049


   /pulsarbot rerun-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] lhotari merged pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
lhotari merged pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] momo-jun commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
momo-jun commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r825741178



##########
File path: conf/proxy.conf
##########
@@ -65,6 +65,11 @@ servicePort=6650
 # The port to use to server binary Protobuf TLS requests
 servicePortTls=
 
+# Specify the ssl provider to use to server binary Protobuf TLS requests:

Review comment:
       ```suggestion
   # Specify the SSL provider to serve binary Protobuf TLS requests:
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1059916604


   /pulsarbot rerun-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1066289492


   @lhotari Could you review this PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1067513279


   > 
   
   Thanks for your review, good idea.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1067679215


   @momo-jun Thanks for your review, should be resolved now.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r820452429



##########
File path: pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ServiceChannelInitializer.java
##########
@@ -64,7 +65,7 @@ public ServiceChannelInitializer(ProxyService proxyService, ProxyConfiguration s
         if (enableTls) {
             if (tlsEnabledWithKeyStore) {
                 serverSSLContextAutoRefreshBuilder = new NettySSLContextAutoRefreshBuilder(
-                        serviceConfig.getTlsProvider(),
+                        serviceConfig.getServiceSslProvider(),

Review comment:
       We have to know the SSLContext provider and SSL provider:
   
   - The SSLContext provider is used to new SSLContext
   -  The SSL provider is used to handle the SSL(This is my understand). 
   
   The `tlsProvider` is the SSLContext provider, not the SSL provider. 
   
   Broker service:
   
   With KeyStore, we only need to set the SSLContext provider, the current implementation doesn't support setting the SSL provider.
   
   With CACert, we only need to set the SSL provider, the default should be OpenSSL, when the OpenSSL is not available, will use JDK.
   
   Web service:
   
   SSL context provider and SSL provider are the same.
   
   
   When both use the KeyStore, we can use `tlsProvider`. When use the CACERT, the web service and broker service cannot use the same provider, so we need to split these config.
   
   
   
   
   
   
   
   
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r827651324



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarChannelInitializer.java
##########
@@ -92,19 +93,36 @@ public PulsarChannelInitializer(ClientConfigurationData conf, Supplier<ClientCnx
 
             sslContextSupplier = new ObjectCache<SslContext>(() -> {
                 try {
+                    SslProvider sslProvider = null;
+                    if (conf.getSslProvider() != null) {
+                        sslProvider = SslProvider.valueOf(conf.getSslProvider());
+                    }
+
                     // Set client certificate if available
                     AuthenticationDataProvider authData = conf.getAuthentication().getAuthData();
                     if (authData.hasDataForTls()) {
                         return authData.getTlsTrustStoreStream() == null
-                                ? SecurityUtility.createNettySslContextForClient(conf.isTlsAllowInsecureConnection(),
-                                        conf.getTlsTrustCertsFilePath(),
-                                        authData.getTlsCertificates(), authData.getTlsPrivateKey())
-                                : SecurityUtility.createNettySslContextForClient(conf.isTlsAllowInsecureConnection(),
-                                        authData.getTlsTrustStoreStream(),
-                                        authData.getTlsCertificates(), authData.getTlsPrivateKey());
+                                ? SecurityUtility.createNettySslContextForClient(
+                                sslProvider,
+                                conf.isTlsAllowInsecureConnection(),
+                                conf.getTlsTrustCertsFilePath(),
+                                authData.getTlsCertificates(),
+                                authData.getTlsPrivateKey(),
+                                conf.getTlsCiphers(),
+                                conf.getTlsProtocols())
+                                : SecurityUtility.createNettySslContextForClient(sslProvider,
+                                conf.isTlsAllowInsecureConnection(),
+                                authData.getTlsTrustStoreStream(),
+                                authData.getTlsCertificates(), authData.getTlsPrivateKey(),
+                                conf.getTlsCiphers(),

Review comment:
       If the cipher is empty, we don't use it, so I think no breaking changes.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1068654689


   /pulsarbot rerun-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] momo-jun commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
momo-jun commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r825750469



##########
File path: conf/broker.conf
##########
@@ -35,6 +35,11 @@ brokerServicePort=6650
 # Broker data port for TLS - By default TLS is disabled
 brokerServicePortTls=
 
+# Specify the ssl provider for the broker service:
+# When using the CACert, available values are one of OpenSSL and JDK.

Review comment:
       ```suggestion
   # When using TLS authentication with CACert, the valid value is either OpenSSL or JDK.
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece edited a comment on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece edited a comment on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1067513279


   @momo-jun Thanks for your review, a good idea.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] momo-jun commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
momo-jun commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r825751072



##########
File path: conf/broker.conf
##########
@@ -35,6 +35,11 @@ brokerServicePort=6650
 # Broker data port for TLS - By default TLS is disabled
 brokerServicePortTls=
 
+# Specify the ssl provider for the broker service:
+# When using the CACert, available values are one of OpenSSL and JDK.
+# When using the KeyStore, available values like SunJSSE and Conscrypt.

Review comment:
       ```suggestion
   # When using TLS authentication with KeyStore, available values can be SunJSSE, Conscrypt and etc.
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r820454685



##########
File path: pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ServiceChannelInitializer.java
##########
@@ -64,7 +65,7 @@ public ServiceChannelInitializer(ProxyService proxyService, ProxyConfiguration s
         if (enableTls) {
             if (tlsEnabledWithKeyStore) {
                 serverSSLContextAutoRefreshBuilder = new NettySSLContextAutoRefreshBuilder(
-                        serviceConfig.getTlsProvider(),
+                        serviceConfig.getServiceSslProvider(),

Review comment:
       There looks like I need to compatible the `tlsProvider` when using KeyStore?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r827600199



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/PulsarChannelInitializer.java
##########
@@ -92,19 +93,36 @@ public PulsarChannelInitializer(ClientConfigurationData conf, Supplier<ClientCnx
 
             sslContextSupplier = new ObjectCache<SslContext>(() -> {
                 try {
+                    SslProvider sslProvider = null;
+                    if (conf.getSslProvider() != null) {
+                        sslProvider = SslProvider.valueOf(conf.getSslProvider());
+                    }
+
                     // Set client certificate if available
                     AuthenticationDataProvider authData = conf.getAuthentication().getAuthData();
                     if (authData.hasDataForTls()) {
                         return authData.getTlsTrustStoreStream() == null
-                                ? SecurityUtility.createNettySslContextForClient(conf.isTlsAllowInsecureConnection(),
-                                        conf.getTlsTrustCertsFilePath(),
-                                        authData.getTlsCertificates(), authData.getTlsPrivateKey())
-                                : SecurityUtility.createNettySslContextForClient(conf.isTlsAllowInsecureConnection(),
-                                        authData.getTlsTrustStoreStream(),
-                                        authData.getTlsCertificates(), authData.getTlsPrivateKey());
+                                ? SecurityUtility.createNettySslContextForClient(
+                                sslProvider,
+                                conf.isTlsAllowInsecureConnection(),
+                                conf.getTlsTrustCertsFilePath(),
+                                authData.getTlsCertificates(),
+                                authData.getTlsPrivateKey(),
+                                conf.getTlsCiphers(),
+                                conf.getTlsProtocols())
+                                : SecurityUtility.createNettySslContextForClient(sslProvider,
+                                conf.isTlsAllowInsecureConnection(),
+                                authData.getTlsTrustStoreStream(),
+                                authData.getTlsCertificates(), authData.getTlsPrivateKey(),
+                                conf.getTlsCiphers(),

Review comment:
       Does this will change the behavior? I noticed the tls ciphers is empty by default.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1068691453


   /pulsarbot rerun-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1060177962


   /pulsarbot rerun-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r820412517



##########
File path: pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ServiceChannelInitializer.java
##########
@@ -64,7 +65,7 @@ public ServiceChannelInitializer(ProxyService proxyService, ProxyConfiguration s
         if (enableTls) {
             if (tlsEnabledWithKeyStore) {
                 serverSSLContextAutoRefreshBuilder = new NettySSLContextAutoRefreshBuilder(
-                        serviceConfig.getTlsProvider(),
+                        serviceConfig.getServiceSslProvider(),

Review comment:
       Why not to use `tlsProvider`?

##########
File path: pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ServiceChannelInitializer.java
##########
@@ -64,7 +65,7 @@ public ServiceChannelInitializer(ProxyService proxyService, ProxyConfiguration s
         if (enableTls) {
             if (tlsEnabledWithKeyStore) {
                 serverSSLContextAutoRefreshBuilder = new NettySSLContextAutoRefreshBuilder(
-                        serviceConfig.getTlsProvider(),
+                        serviceConfig.getServiceSslProvider(),

Review comment:
       And the webservice, websocket also use the `tlsProvider` as `sslProviderString` for `JettySslContextFactoryWithAutoRefresh`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] Anonymitaet commented on pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#issuecomment-1064638286


   @momo-jun a soft reminder: this PR is labeled `w/ doc-required` and targeted for 2.11


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] nodece commented on a change in pull request #14569: [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service

Posted by GitBox <gi...@apache.org>.
nodece commented on a change in pull request #14569:
URL: https://github.com/apache/pulsar/pull/14569#discussion_r826678491



##########
File path: conf/proxy.conf
##########
@@ -65,6 +65,11 @@ servicePort=6650
 # The port to use to server binary Protobuf TLS requests
 servicePortTls=
 
+# Specify the ssl provider to use to server binary Protobuf TLS requests:

Review comment:
       Thanks @momo-jun, should be resolved now.

##########
File path: conf/broker.conf
##########
@@ -35,6 +35,11 @@ brokerServicePort=6650
 # Broker data port for TLS - By default TLS is disabled
 brokerServicePortTls=
 
+# Specify the ssl provider for the broker service:
+# When using the CACert, available values are one of OpenSSL and JDK.

Review comment:
       Thanks @momo-jun, should be resolved now.
   

##########
File path: conf/broker.conf
##########
@@ -35,6 +35,11 @@ brokerServicePort=6650
 # Broker data port for TLS - By default TLS is disabled
 brokerServicePortTls=
 
+# Specify the ssl provider for the broker service:
+# When using the CACert, available values are one of OpenSSL and JDK.
+# When using the KeyStore, available values like SunJSSE and Conscrypt.

Review comment:
       Thanks @momo-jun, should be resolved now.
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org