Seeing javax.net.ssl.SSLHandshakeException when enabling Conscrypt

[WeiranFang]

What version of gRPC-Java are you using?

1.26.0

What is your environment?

GCE VM ( Debian 4.9.189-3 (2019-09-02))
openjdk version "1.8.0_232"

What did you expect to see?

Enabling Conscrypt should not break gRPC call from a GCE VM (over CFE).

What did you see instead?

Client is running on GCE VM, and after I enabled Conscrypt using "Security.insertProviderAt(Conscrypt.newProvider(), 1)", I saw the following error:

Channel Pipeline: [SslHandler#0, ProtocolNegotiators$ClientTlsHandler#0, WriteBufferingAndExceptionHandler#0, DefaultChannelPipeline$TailContext#0]
        at io.grpc.Status.asRuntimeException(Status.java:533)
        at io.grpc.stub.ClientCalls$BlockingResponseStream.hasNext(ClientCalls.java:606)
        at io.grpc.gcs.GrpcClient.makeMediaRequest(GrpcClient.java:110)
        at io.grpc.gcs.GrpcClient.startCalls(GrpcClient.java:87)
        at io.grpc.gcs.TestMain.main(TestMain.java:40)
Caused by: javax.net.ssl.SSLHandshakeException: General OpenSslEngine problem
        at io.grpc.netty.shaded.io.netty.handler.ssl.ReferenceCountedOpenSslEngine.handshakeException(ReferenceCountedOpenSslEngine.java:1728)
        at io.grpc.netty.shaded.io.netty.handler.ssl.ReferenceCountedOpenSslEngine.wrap(ReferenceCountedOpenSslEngine.java:770)
        at javax.net.ssl.SSLEngine.wrap(SSLEngine.java:509)
        at io.grpc.netty.shaded.io.netty.handler.ssl.SslHandler.wrap(SslHandler.java:1043)
        at io.grpc.netty.shaded.io.netty.handler.ssl.SslHandler.wrapNonAppData(SslHandler.java:934)
        at io.grpc.netty.shaded.io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1392)
        at io.grpc.netty.shaded.io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1224)
        at io.grpc.netty.shaded.io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1271)
        at io.grpc.netty.shaded.io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:505)
        at io.grpc.netty.shaded.io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:444)
        at io.grpc.netty.shaded.io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:283)
        at io.grpc.netty.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:374)
        at io.grpc.netty.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:360)
        at io.grpc.netty.shaded.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:352)
        at io.grpc.netty.shaded.io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1422)
        at io.grpc.netty.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:374)
        at io.grpc.netty.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:360)
        at io.grpc.netty.shaded.io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:931)
        at io.grpc.netty.shaded.io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:792)
        at io.grpc.netty.shaded.io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:483)
        at io.grpc.netty.shaded.io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:383)
        at io.grpc.netty.shaded.io.netty.util.concurrent.SingleThreadEventExecutor$6.run(SingleThreadEventExecutor.java:1044)
        at io.grpc.netty.shaded.io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
        at io.grpc.netty.shaded.io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.lang.Thread.run(Thread.java:748)
Caused by: java.security.cert.CertificateException: No subjectAltNames on the certificate match
        at org.conscrypt.TrustManagerImpl.checkTrusted(TrustManagerImpl.java:408)
        at org.conscrypt.TrustManagerImpl.getTrustedChainForServer(TrustManagerImpl.java:356)
        at org.conscrypt.TrustManagerImpl.checkServerTrusted(TrustManagerImpl.java:369)
        at io.grpc.netty.shaded.io.netty.handler.ssl.OpenSslTlsv13X509ExtendedTrustManager.checkServerTrusted(OpenSslTlsv13X509ExtendedTrustManager.java:221)
        at io.grpc.netty.shaded.io.netty.handler.ssl.ReferenceCountedOpenSslClientContext$ExtendedTrustManagerVerifyCallback.verify(ReferenceCountedOpenSslClientContext.java:248)
        at io.grpc.netty.shaded.io.netty.handler.ssl.ReferenceCountedOpenSslContext$AbstractCertificateVerifier.verify(ReferenceCountedOpenSslContext.java:699)
        at io.grpc.netty.shaded.io.netty.internal.tcnative.SSL.readFromSSL(Native Method)
        at io.grpc.netty.shaded.io.netty.handler.ssl.ReferenceCountedOpenSslEngine.readPlaintextData(ReferenceCountedOpenSslEngine.java:589)
        at io.grpc.netty.shaded.io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1172)
        at io.grpc.netty.shaded.io.netty.handler.ssl.ReferenceCountedOpenSslEngine.unwrap(ReferenceCountedOpenSslEngine.java:1289)
        at io.grpc.netty.shaded.io.netty.handler.ssl.SslHandler$SslEngineType$1.unwrap(SslHandler.java:199)
        at io.grpc.netty.shaded.io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1329)

Steps to reproduce the bug

[ejona86]

This seems similar to google/conscrypt#656 . That seems horribly broken in Conscrypt. It is unclear why this wasn't an issue in the past.

Which Conscrypt version did you test with?

[WeiranFang]

Which Conscrypt version did you test with?

It's the default version (2.2.1) from grpc-alts

[veblush]

This seems to be related with other bug preventing Dataproc from upgrading Conscrypt to 2.2.1. Since google/conscrypt#656 was closed as sort-of work as intended, do we need to add other logic to meet the Conscrypt's requirement? I guess this change was introduced from 2.1 with this PR.

[ejona86]

It is a clear regression. Conscrypt will need to be fixed in some way.

[medb]

+@prbprbprb who seems to support Conscrypt currently.

May you confirm that this is in fact a Conscrypt bug?

[prbprbprb]

I'm not sure I'd class it as a bug, but maybe it's a surprising default. :)

If you don't set a ConscryptHostnameVerifier then Conscrypt will fall back to one which calls the platform's current default HostnameVerifier, which on GCE / openjdk8 is a deny-all one. That's because the default policy on openjdk is that HTTPSUrlConnection only calls its HostnameVerifier if the certificate for the connection doesn't match the expected hostname (at least that's what the javadoc says). Note that in these circumstances the text in the thrown exception is misleading, it will never have attempted to compare subjectAltNames.

This behaviour is different to Android where the platform default HostnameVerifier is derived from the okhttp one and does do (rigorous) hostname verification.

The "fix" is to set either a default ConscryptHostnameVerifier or HostnameVerifier before making any TLS connections. Note that this is a security-critical component so do not just plug in an allow-all verifier or you are opening yourself up to impersonation attacks. You should be able to use either the okhttp one (although it's Kotlin nowadays) or the AOSP one pretty much unchanged.

I think the takeaway here is that on non-Android platforms, Conscrypt should install a better default verifier for fewer surprises all round. I'll file an FR for that.

[prbprbprb]

Ah, I see @flooey already documented this behaviour in google/conscrypt#665 (I reviewed it and forgot).

I think it's feasible to implement a different default in a way that doesn't break Android backward compatibility, but we (Conscrypt) would want to be very careful about setting default security policies on other platforms, because coders never look at this stuff unless it breaks. ;)

[ejona86]

@prbprbprb, it seems a pretty clear bug.

Note: I didn't say anything about HTTPSUrlConnection. I don't really care about it. We aren't using it.

The "fix" is to set either a default ConscryptHostnameVerifier or HostnameVerifier before making any TLS connections.

HostnameVerifier shouldn't be discussed at all. That is a legacy HTTPSUrlConnection thing. In the design of that legacy API the client (e.g., grpc or HTTPSUrlConnection) had to manually call the API when using SSLSocket or SSLEngine. By default there was no hostname verification. The preferred, modern API is to use SSLParameters.setEndpointIdentificationAlgorithm(), which grpc is using.

Implementing a HostnameVerifier is a pain and, as you mentioned, security-sensitive. Applications should not be writing their own or copying an implementation that must be tracked separately for security enhancements and fixes.

[ejona86]

Also, this is a clear regression, and is meaning gRPC users are unable to upgrade to newer Conscrypt versions.

[ejona86]

I just realized in this issue we aren't using Conscrypt[1]. ReferenceCountedOpenSslEngine is only used with tcnative.

It still isn't clear why we see this particular error, but it may be related to #6315

1. What I mean is that the TLS provider is tcnative. Conscrypt is clearly in the stacktrace, but there may be ways to avoid using it. The issue here is directly related to Security.insertProviderAt()

[voidzcy]

From my current observation, probably both Conscrypt's HostnameVerifier issue and Conscrypt+tc_native compatibility issue (#6315) are involved. I tried with removing the runtime dependency of tc_native, but would still get No subjectAltNames on the certificate match error (with only Conscrypt shown in stacktrace), while this cert works fine with tc_native.

[voidzcy]

@WeiranFang @veblush

Replacing Security.insertProviderAt(Conscrypt.newProvider(), 1) with Security.insertProviderAt(Conscrypt.newProviderBuilder().provideTrustManager(false).build(), 1) should work around this issue.

This disables Conscrypt's TrustManager, whose HostNameVerifier denies all as discussed above.

Also, you need to be careful when both Conscrypt and tcnative present. GrpcSslContexts prefers tcnative, so the actual TLS provider being used will be tcnative instead of Conscrypt even though Conscrypt provider is inserted to the first place.

@ejona86
How do grpc-alts users use Conscrypt as TLS provider?

[ejona86]

grpc-alts will use Conscrypt if it is in the classpath and fall back to the system-default provider if Conscrypt is not in the classpath.

[veblush]

Dataproc (Internal Hadoop) modifies java.security file to have org.conscrypt.OpenSSLProvider at the first place when creating a new VM on GCP. From the code, Conscrypt enables includeTrustManager for openJDK but disables it for Android so we can use provideTrustManager(false) for our benchmark client but not enough for Dataproc's case.

[voidzcy]

What's the requirement for Dataproc's case?

[veblush]

When Dataproc creates VMs for users, it customizes a bunch of things including Conscrypt configuration and this is not only for gRPC but for all libraries that Hadoop uses such as the GCS HTTP client which can benefit from Conscrypt installed as a default security provider. Since gRPC hasn't went well with Conscrypt 2.1 and later, it's stuck with Conscrypt 1.9. For Dataproc, having a solution where the latest gRPC and Conscrypt work together would be ideal.

[ejona86]

Dataproc (Internal Hadoop) modifies java.security file to have org.conscrypt.OpenSSLProvider at the first place when creating a new VM on GCP

This discussion should be moved to Conscrypt. There is nothing we can do about it in gRPC. One possible fix would be to change the default on OpenJDK, because it is a silly default if it requires configuring the HostNameVerifier. google/conscrypt#834 . Another option is Conscrypt could observe another system property for configuring its default and Dataproc could set that property.

One unanswered question is why is Conscrypt 1.9 fine, whereas 2+ broken. It appears that is because google/conscrypt@f32607b was a regression because it "Change to provide the TrustManager by default on OpenJDK."

[voidzcy]

Closing. Fixed by #7342.