From d30212134d81228a05ce84b58b6db059615539b7 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 00c6886e9ad..838254cfe37 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 @@ private OpenSslTlsv13X509ExtendedTrustManager(X509ExtendedTrustManager tm) { 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 @@ public void checkServerTrusted(X509Certificate[] x509Certificates, String s) thr 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 df99c2ff9f0..c063fc398ce 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 @@ private static final class ExtendedTrustManagerVerifyCallback extends AbstractCe 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 3e72790bf9e..6790be70552 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java @@ -237,7 +237,7 @@ private static final class ExtendedTrustManagerVerifyCallback extends AbstractCe ExtendedTrustManagerVerifyCallback(OpenSslEngineMap engineMap, X509ExtendedTrustManager manager) { super(engineMap); - this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(manager, false); + this.manager = OpenSslTlsv13X509ExtendedTrustManager.wrap(manager); } @Override