From fd475546693f0218f295d3d198c83e1cd5a90895 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 20 Aug 2021 11:41:52 +0200 Subject: [PATCH] Only suppert TLSv1.3 when JDK does support it as well (#11604) Motivation: We tightly integrate the TrustManger and KeyManager into our native SSL implementation which means that both of them need to support TLSv1.3 as protocol. This is not always the case and so can produce runtime exceptions. As TLSv1.3 support was backported to Java8 quite some time now we should be a bit more conservative and only enable TLSv1.3 for our native implementation if the JDK implementation supports it as well. This also allows us to remove some hacks we had in place to be able to support it before in Java8. Modifications: - Only enable TLSv1.3 support for our native SSL implementation when the JDK supports it as well - Remove OpenSslTlsv13X509ExtendedTrustManager as its not needed anymore Result: Fixes https://github.com/netty/netty/issues/11589 --- .../java/io/netty/handler/ssl/OpenSsl.java | 38 +-- ...OpenSslTlsv13X509ExtendedTrustManager.java | 240 ------------------ .../ReferenceCountedOpenSslClientContext.java | 2 +- .../ReferenceCountedOpenSslServerContext.java | 2 +- 4 files changed, 23 insertions(+), 259 deletions(-) delete mode 100644 handler/src/main/java/io/netty/handler/ssl/OpenSslTlsv13X509ExtendedTrustManager.java diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java b/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java index 0778031572..d6fd1814ce 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java @@ -206,25 +206,29 @@ public final class OpenSsl { long cert = 0; long key = 0; try { - try { - StringBuilder tlsv13Ciphers = new StringBuilder(); + // As we delegate to the KeyManager / TrustManager of the JDK we need to ensure it can actually + // handle TLSv13 as otherwise we may see runtime exceptions + if (SslProvider.isTlsv13Supported(SslProvider.JDK)) { + try { + StringBuilder tlsv13Ciphers = new StringBuilder(); - for (String cipher: TLSV13_CIPHERS) { - String converted = CipherSuiteConverter.toOpenSsl(cipher, IS_BORINGSSL); - if (converted != null) { - tlsv13Ciphers.append(converted).append(':'); + for (String cipher : TLSV13_CIPHERS) { + String converted = CipherSuiteConverter.toOpenSsl(cipher, IS_BORINGSSL); + if (converted != null) { + tlsv13Ciphers.append(converted).append(':'); + } + } + if (tlsv13Ciphers.length() == 0) { + tlsv13Supported = false; + } else { + tlsv13Ciphers.setLength(tlsv13Ciphers.length() - 1); + SSLContext.setCipherSuite(sslCtx, tlsv13Ciphers.toString(), true); + tlsv13Supported = true; } - } - if (tlsv13Ciphers.length() == 0) { - tlsv13Supported = false; - } else { - tlsv13Ciphers.setLength(tlsv13Ciphers.length() - 1); - SSLContext.setCipherSuite(sslCtx, tlsv13Ciphers.toString() , true); - tlsv13Supported = true; - } - } catch (Exception ignore) { - tlsv13Supported = false; + } catch (Exception ignore) { + tlsv13Supported = false; + } } SSLContext.setCipherSuite(sslCtx, "ALL", false); @@ -367,7 +371,7 @@ public final class OpenSsl { protocols.add(SslProtocols.TLS_v1_2); } - // This is only supported by java11 and later. + // This is only supported by java8u272 and later. if (tlsv13Supported && doesSupportProtocol(SSL.SSL_PROTOCOL_TLSV1_3, SSL.SSL_OP_NO_TLSv1_3)) { protocols.add(SslProtocols.TLS_v1_3); TLSV13_SUPPORTED = true; diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslTlsv13X509ExtendedTrustManager.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslTlsv13X509ExtendedTrustManager.java deleted file mode 100644 index 1b3bed6ba2..0000000000 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslTlsv13X509ExtendedTrustManager.java +++ /dev/null @@ -1,240 +0,0 @@ -/* - * Copyright 2018 The Netty Project - * - * The Netty Project licenses this file to you under the Apache License, - * version 2.0 (the "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at: - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. - */ -package io.netty.handler.ssl; - -import io.netty.util.internal.PlatformDependent; -import io.netty.util.internal.SuppressJava6Requirement; - -import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLPeerUnverifiedException; -import javax.net.ssl.SSLSession; -import javax.net.ssl.SSLSessionContext; -import javax.net.ssl.X509ExtendedTrustManager; -import java.net.Socket; -import java.security.Principal; -import java.security.cert.Certificate; -import java.security.cert.CertificateException; -import java.security.cert.X509Certificate; -import java.util.List; - -/** - * Provide a way to use {@code TLSv1.3} with Java versions prior to 11 by adding a - * = 7 && session instanceof ExtendedOpenSslSession) { - final ExtendedOpenSslSession extendedOpenSslSession = (ExtendedOpenSslSession) session; - return new ExtendedOpenSslSession(extendedOpenSslSession) { - @Override - public List getRequestedServerNames() { - return extendedOpenSslSession.getRequestedServerNames(); - } - - @Override - public String[] getPeerSupportedSignatureAlgorithms() { - return extendedOpenSslSession.getPeerSupportedSignatureAlgorithms(); - } - - @Override - public String getProtocol() { - return SslProtocols.TLS_v1_2; - } - }; - } else { - return new SSLSession() { - @Override - public byte[] getId() { - return session.getId(); - } - - @Override - public SSLSessionContext getSessionContext() { - return session.getSessionContext(); - } - - @Override - public long getCreationTime() { - return session.getCreationTime(); - } - - @Override - public long getLastAccessedTime() { - return session.getLastAccessedTime(); - } - - @Override - public void invalidate() { - session.invalidate(); - } - - @Override - public boolean isValid() { - return session.isValid(); - } - - @Override - public void putValue(String s, Object o) { - session.putValue(s, o); - } - - @Override - public Object getValue(String s) { - return session.getValue(s); - } - - @Override - public void removeValue(String s) { - session.removeValue(s); - } - - @Override - public String[] getValueNames() { - return session.getValueNames(); - } - - @Override - public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException { - return session.getPeerCertificates(); - } - - @Override - public Certificate[] getLocalCertificates() { - return session.getLocalCertificates(); - } - - @Override - public javax.security.cert.X509Certificate[] getPeerCertificateChain() - throws SSLPeerUnverifiedException { - return session.getPeerCertificateChain(); - } - - @Override - public Principal getPeerPrincipal() throws SSLPeerUnverifiedException { - return session.getPeerPrincipal(); - } - - @Override - public Principal getLocalPrincipal() { - return session.getLocalPrincipal(); - } - - @Override - public String getCipherSuite() { - return session.getCipherSuite(); - } - - @Override - public String getProtocol() { - return SslProtocols.TLS_v1_2; - } - - @Override - public String getPeerHost() { - return session.getPeerHost(); - } - - @Override - public int getPeerPort() { - return session.getPeerPort(); - } - - @Override - public int getPacketBufferSize() { - return session.getPacketBufferSize(); - } - - @Override - public int getApplicationBufferSize() { - return session.getApplicationBufferSize(); - } - }; - } - } - }; - } - return engine; - } - - @Override - public void checkClientTrusted(X509Certificate[] x509Certificates, final String s, SSLEngine sslEngine) - throws CertificateException { - tm.checkClientTrusted(x509Certificates, s, wrapEngine(sslEngine)); - } - - @Override - public void checkServerTrusted(X509Certificate[] x509Certificates, String s, SSLEngine sslEngine) - throws CertificateException { - tm.checkServerTrusted(x509Certificates, s, wrapEngine(sslEngine)); - } - - @Override - public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { - tm.checkClientTrusted(x509Certificates, s); - } - - @Override - public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { - tm.checkServerTrusted(x509Certificates, s); - } - - @Override - public X509Certificate[] getAcceptedIssuers() { - return tm.getAcceptedIssuers(); - } -} diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java index 317c558639..21bb540506 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java @@ -225,7 +225,7 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted ExtendedTrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509ExtendedTrustManager manager) { super(engineMap); - this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(manager); + this.manager = manager; } @Override diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java index f143d8a4c0..07e0a5de17 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java @@ -267,7 +267,7 @@ public final class ReferenceCountedOpenSslServerContext extends ReferenceCounted ExtendedTrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509ExtendedTrustManager manager) { super(engineMap); - this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(manager); + this.manager = manager; } @Override