Skip to content

Commit

Permalink
Always wrap X509ExtendedTrustManager when using OpenSSL and JDK < 11 (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
normanmaurer committed Aug 13, 2019
1 parent 8d9cea2 commit d302121
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 265 deletions.
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
}
}
Expand Up @@ -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
Expand Down
Expand Up @@ -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
Expand Down

0 comments on commit d302121

Please sign in to comment.