From 16e290796d0fa14a5a80b68e13e3ac871da32bdb Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 13 Aug 2019 10:26:13 +0200 Subject: [PATCH] Always wrap X509ExtendedTrustManager when using OpenSSL and JDK < 11 (#9443) Motivation: When using OpenSSL and JDK < 11 is used we need to wrap the user provided X509ExtendedTrustManager to be able to support TLS1.3. We had a check in place that first tried to see if wrapping is needed at all which could lead to missleading calls of the user provided trustmanager. We should remove these calls and just always wrap if needed. Modifications: Always wrap if OpenSSL + JDK < 11 and TLS1.3 is supported Result: Less missleading calls to user provided trustmanager --- ...OpenSslTlsv13X509ExtendedTrustManager.java | 266 +----------------- .../ReferenceCountedOpenSslClientContext.java | 2 +- .../ReferenceCountedOpenSslServerContext.java | 2 +- 3 files changed, 5 insertions(+), 265 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslTlsv13X509ExtendedTrustManager.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslTlsv13X509ExtendedTrustManager.java index 4f177309d2..8760e60595 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslTlsv13X509ExtendedTrustManager.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslTlsv13X509ExtendedTrustManager.java @@ -15,19 +15,14 @@ */ package io.netty.handler.ssl; -import io.netty.util.internal.EmptyArrays; import io.netty.util.internal.PlatformDependent; import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLEngineResult; -import javax.net.ssl.SSLEngineResult.HandshakeStatus; -import javax.net.ssl.SSLException; 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.nio.ByteBuffer; import java.security.Principal; import java.security.cert.Certificate; import java.security.cert.CertificateException; @@ -48,22 +43,9 @@ final class OpenSslTlsv13X509ExtendedTrustManager extends X509ExtendedTrustManag this.tm = tm; } - static X509ExtendedTrustManager wrap(X509ExtendedTrustManager tm, boolean client) { - if (PlatformDependent.javaVersion() < 11) { - try { - X509Certificate[] certs = { OpenSsl.selfSignedCertificate() }; - if (client) { - tm.checkServerTrusted(certs, "RSA", new DummySSLEngine(true)); - } else { - tm.checkClientTrusted(certs, "RSA", new DummySSLEngine(false)); - } - } catch (IllegalArgumentException e) { - // If this happened we failed because our protocol version was not known by the implementation. - // See http://mail.openjdk.java.net/pipermail/security-dev/2018-September/018242.html. - return new OpenSslTlsv13X509ExtendedTrustManager(tm); - } catch (Throwable ignore) { - // Just assume we do not need to wrap. - } + static X509ExtendedTrustManager wrap(X509ExtendedTrustManager tm) { + if (PlatformDependent.javaVersion() < 11 && OpenSsl.isTlsv13Supported()) { + return new OpenSslTlsv13X509ExtendedTrustManager(tm); } return tm; } @@ -253,246 +235,4 @@ final class OpenSslTlsv13X509ExtendedTrustManager extends X509ExtendedTrustManag public X509Certificate[] getAcceptedIssuers() { return tm.getAcceptedIssuers(); } - - private static final class DummySSLEngine extends SSLEngine { - - private final boolean client; - - DummySSLEngine(boolean client) { - this.client = client; - } - - @Override - public SSLSession getHandshakeSession() { - return new SSLSession() { - @Override - public byte[] getId() { - return EmptyArrays.EMPTY_BYTES; - } - - @Override - public SSLSessionContext getSessionContext() { - return null; - } - - @Override - public long getCreationTime() { - return 0; - } - - @Override - public long getLastAccessedTime() { - return 0; - } - - @Override - public void invalidate() { - // NOOP - } - - @Override - public boolean isValid() { - return false; - } - - @Override - public void putValue(String s, Object o) { - // NOOP - } - - @Override - public Object getValue(String s) { - return null; - } - - @Override - public void removeValue(String s) { - // NOOP - } - - @Override - public String[] getValueNames() { - return EmptyArrays.EMPTY_STRINGS; - } - - @Override - public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException { - return EmptyArrays.EMPTY_CERTIFICATES; - } - - @Override - public Certificate[] getLocalCertificates() { - return EmptyArrays.EMPTY_CERTIFICATES; - } - - @Override - public javax.security.cert.X509Certificate[] getPeerCertificateChain() - throws SSLPeerUnverifiedException { - return EmptyArrays.EMPTY_JAVAX_X509_CERTIFICATES; - } - - @Override - public Principal getPeerPrincipal() throws SSLPeerUnverifiedException { - return null; - } - - @Override - public Principal getLocalPrincipal() { - return null; - } - - @Override - public String getCipherSuite() { - return null; - } - - @Override - public String getProtocol() { - return SslUtils.PROTOCOL_TLS_V1_3; - } - - @Override - public String getPeerHost() { - return null; - } - - @Override - public int getPeerPort() { - return 0; - } - - @Override - public int getPacketBufferSize() { - return 0; - } - - @Override - public int getApplicationBufferSize() { - return 0; - } - }; - } - - @Override - public SSLEngineResult wrap(ByteBuffer[] byteBuffers, int i, int i1, ByteBuffer byteBuffer) - throws SSLException { - throw new UnsupportedOperationException(); - } - - @Override - public SSLEngineResult unwrap(ByteBuffer byteBuffer, ByteBuffer[] byteBuffers, int i, int i1) - throws SSLException { - throw new UnsupportedOperationException(); - } - - @Override - public Runnable getDelegatedTask() { - return null; - } - - @Override - public void closeInbound() throws SSLException { - // NOOP - } - - @Override - public boolean isInboundDone() { - return true; - } - - @Override - public void closeOutbound() { - // NOOP - } - - @Override - public boolean isOutboundDone() { - return true; - } - - @Override - public String[] getSupportedCipherSuites() { - return EmptyArrays.EMPTY_STRINGS; - } - - @Override - public String[] getEnabledCipherSuites() { - return EmptyArrays.EMPTY_STRINGS; - } - - @Override - public void setEnabledCipherSuites(String[] strings) { - // NOOP - } - - @Override - public String[] getSupportedProtocols() { - return new String[] { SslUtils.PROTOCOL_TLS_V1_3 }; - } - - @Override - public String[] getEnabledProtocols() { - return new String[] { SslUtils.PROTOCOL_TLS_V1_3 }; - } - - @Override - public void setEnabledProtocols(String[] strings) { - // NOOP - } - - @Override - public SSLSession getSession() { - return getHandshakeSession(); - } - - @Override - public void beginHandshake() throws SSLException { - // NOOP - } - - @Override - public HandshakeStatus getHandshakeStatus() { - return HandshakeStatus.NEED_TASK; - } - - @Override - public void setUseClientMode(boolean b) { - // NOOP - } - - @Override - public boolean getUseClientMode() { - return client; - } - - @Override - public void setNeedClientAuth(boolean b) { - // NOOP - } - - @Override - public boolean getNeedClientAuth() { - return false; - } - - @Override - public void setWantClientAuth(boolean b) { - // NOOP - } - - @Override - public boolean getWantClientAuth() { - return false; - } - - @Override - public void setEnableSessionCreation(boolean b) { - // NOOP - } - - @Override - public boolean getEnableSessionCreation() { - return false; - } - } } 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 b3e37f629f..11ee5d7605 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java @@ -239,7 +239,7 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted ExtendedTrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509ExtendedTrustManager manager) { super(engineMap); - this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(manager, true); + this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(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 3f822f1bba..6bb099ca40 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java @@ -233,7 +233,7 @@ public final class ReferenceCountedOpenSslServerContext extends ReferenceCounted ExtendedTrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509ExtendedTrustManager manager) { super(engineMap); - this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(manager, false); + this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(manager); } @Override