[Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service #14569

nodece · 2022-03-05T17:35:21Z

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

The root cause is the broker service use CACert with OpenSSL SSL provider, which doesn't support these cipher, we need to switch to JDK provider, so we need to add a config to the support this.

Modifications

Update the tlsProvider description
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?

doc-complete

Update tlsProvider description in website.

nodece · 2022-03-06T08:14:10Z

/pulsarbot rerun-failure-checks

nodece · 2022-03-07T04:23:22Z

/pulsarbot rerun-failure-checks

codelipenghui · 2022-03-07T06:29:44Z

pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ServiceChannelInitializer.java

            if (tlsEnabledWithKeyStore) {
                serverSSLContextAutoRefreshBuilder = new NettySSLContextAutoRefreshBuilder(
-                        serviceConfig.getTlsProvider(),
+                        serviceConfig.getServiceSslProvider(),


Why not to use tlsProvider?

And the webservice, websocket also use the tlsProvider as sslProviderString for JettySslContextFactoryWithAutoRefresh

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.

There looks like I need to compatible the tlsProvider when using KeyStore?

Anonymitaet · 2022-03-11T00:04:47Z

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

nodece · 2022-03-14T02:59:15Z

@lhotari Could you review this PR?

conf/proxy.conf

momo-jun · 2022-03-14T09:37:37Z

conf/broker.conf

 brokerServicePortTls=

+# Specify the ssl provider for the broker service:
+# When using the CACert, available values are one of OpenSSL and JDK.


Suggested change

# When using the CACert, available values are one of OpenSSL and JDK.

# When using TLS authentication with CACert, the valid value is either OpenSSL or JDK.

Thanks @momo-jun, should be resolved now.

conf/broker.conf

momo-jun

@nodece I left some comments regarding the annotations in the configurations.
BTW, if it's going to replace tlsprovider', does it make sense to use tls` to name the new configs?

lhotari · 2022-04-08T06:08:34Z

@nodece one comment about these ciphers:

TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384
TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256

These are considered unsecure legacy ciphers which aren't supported in Netty with BoringSSL / netty-tcnative.
More details here: netty/netty#9775 (comment)

I believe it's the same reason why Conscrypt doesn't support the CBC mode ciphers.

nodece · 2022-04-08T06:29:34Z

Thanks @lhotari, but we should allow the users to set these ciphers, although these are not safe.

Anonymitaet · 2022-04-11T01:44:44Z

conf/broker.conf

 # authentication.
 tlsRequireTrustedClientCertOnConnect=false

+# Specify the TLS provider for the broker service:


@momo-jun need to add explanations to https://pulsar.apache.org/docs/en/next/reference-configuration/?

Yep, to both broker/standalone and proxy sections. Will add the docs after the 2.10 release.

…14569) Signed-off-by: Zixuan Liu <nodeces@gmail.com> (cherry picked from commit 9b2ba05)

codelipenghui · 2022-04-19T10:02:49Z

@nodece I'm not able to cherry-pick to branch-2.9 directly, could you please help push a PR to branch-2.9?

…pache#14569) Signed-off-by: Zixuan Liu <nodeces@gmail.com>

…pache#14569) Signed-off-by: Zixuan Liu <nodeces@gmail.com> (cherry picked from commit 9b2ba05) Signed-off-by: Zixuan Liu <nodeces@gmail.com>

…tocols (apache#14569)" This reverts commit 72aae15.

github-actions · 2022-09-16T07:40:47Z

@nodece Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.

github-actions bot assigned nodece Mar 5, 2022

github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Mar 5, 2022

nodece force-pushed the security_provider_ciphers_protocols branch from 0a4df96 to 0e3b754 Compare March 6, 2022 02:28

nodece changed the title ~~[Broker] Add ssl provider, ciphers and protocols support for broker service and proxy service~~ [Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service Mar 6, 2022

nodece force-pushed the security_provider_ciphers_protocols branch from 0e3b754 to 0100505 Compare March 6, 2022 02:38

nodece force-pushed the security_provider_ciphers_protocols branch 2 times, most recently from d28f8ef to b400d0c Compare March 7, 2022 02:58

codelipenghui added this to the 2.11.0 milestone Mar 7, 2022

codelipenghui added release/2.10.1 release/2.8.4 release/2.9.3 labels Mar 7, 2022

codelipenghui reviewed Mar 7, 2022

View reviewed changes

momo-jun reviewed Mar 14, 2022

View reviewed changes

conf/proxy.conf Outdated Show resolved Hide resolved

momo-jun reviewed Mar 14, 2022

View reviewed changes

conf/broker.conf Outdated Show resolved Hide resolved

momo-jun reviewed Mar 14, 2022

View reviewed changes

codelipenghui requested review from 315157973, aloyszhang, eolivelli, gaoran10, hangc0276, hezhangjian, lhotari, merlimat, rdhabalia and zymap March 14, 2022 13:42

Anonymitaet reviewed Apr 11, 2022

View reviewed changes

codelipenghui pushed a commit that referenced this pull request Apr 19, 2022

[feat][broker] Full-support set ssl provider, ciphers and protocols (#…

8d315e0

…14569) Signed-off-by: Zixuan Liu <nodeces@gmail.com> (cherry picked from commit 9b2ba05)

momo-jun mentioned this pull request Apr 19, 2022

[feature][doc] Add configs for full-support tls provider and managed cursor compression #15210

Merged

1 task

codelipenghui added cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.10 and removed cherry-picked/branch-2.9 Archived: 2.9 is end of life labels Apr 19, 2022

Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022

[feat][broker] Full-support set ssl provider, ciphers and protocols (a…

629353b

…pache#14569) Signed-off-by: Zixuan Liu <nodeces@gmail.com>

nodece mentioned this pull request Apr 20, 2022

[branch-2.9][broker] Full-support set ssl provider, ciphers and protocols #15226

Merged

1 task

Anonymitaet added doc-complete Your PR changes impact docs and the related docs have been already added. and removed doc-required Your PR changes impact docs and you will update later. labels Apr 25, 2022

mattisonchao added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label May 24, 2022

nicoloboschi added a commit to datastax/pulsar that referenced this pull request Jun 1, 2022

Revert "[feat][broker] Full-support set ssl provider, ciphers and pro…

6d58162

…tocols (apache#14569)" This reverts commit 72aae15.

Technoboy- mentioned this pull request Sep 14, 2022

[improve][broker] Add a constructor method for NettyServerSslContextBuilder. #17649

Merged

1 task

BewareMyPower removed cherry-picked/branch-2.8 Archived: 2.8 is end of life release/2.8.4 labels Sep 14, 2022

github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Sep 14, 2022

momo-jun removed the doc-required Your PR changes impact docs and you will update later. label Sep 15, 2022

github-actions bot added doc-required Your PR changes impact docs and you will update later. doc-label-missing and removed doc-complete Your PR changes impact docs and the related docs have been already added. doc-required Your PR changes impact docs and you will update later. labels Sep 15, 2022

github-actions bot added doc-complete Your PR changes impact docs and the related docs have been already added. and removed doc-label-missing labels Sep 16, 2022

	# When using the CACert, available values are one of OpenSSL and JDK.
	# When using TLS authentication with CACert, the valid value is either OpenSSL or JDK.

[Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service #14569

[Broker] Full-support ssl provider, ciphers and protocols for broker service and proxy service #14569

Uh oh!

Conversation

nodece commented Mar 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Documentation

Uh oh!

nodece commented Mar 6, 2022

Uh oh!

nodece commented Mar 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Anonymitaet commented Mar 11, 2022

Uh oh!

nodece commented Mar 14, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

momo-jun left a comment

Choose a reason for hiding this comment

Uh oh!

lhotari commented Apr 8, 2022

Uh oh!

nodece commented Apr 8, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codelipenghui commented Apr 19, 2022

Uh oh!

github-actions bot commented Sep 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

nodece commented Mar 5, 2022 •

edited

Loading