Do the same extended checks as the JDK when a X509TrustManager is used with the OpenSSL provider. (#8307)
Motivation: When a X509TrustManager is used while configure the SslContext the JDK automatically does some extra checks during validation of provided certs by the remote peer. We should do the same when our native implementation is used. Modification: - Automatically wrap a X509TrustManager and so do the same validations as the JDK does. - Add unit tests. Result: More consistent behaviour. Fixes https://github.com/netty/netty/issues/6664.
This commit is contained in:
Norman Maurer
GitHub
parent
2d7cb47edd
commit
a208f6dc7c
@ -404,6 +404,10 @@ public final class PlatformDependent {
|
||||
"sun.misc.Unsafe or java.nio.DirectByteBuffer.<init>(long, int) not available");
|
||||
}
|
||||
|
||||
public static Object getObject(Object object, long fieldOffset) {
|
||||
return PlatformDependent0.getObject(object, fieldOffset);
|
||||
}
|
||||
|
||||
public static int getInt(Object object, long fieldOffset) {
|
||||
return PlatformDependent0.getInt(object, fieldOffset);
|
||||
}
|
||||
|
||||
@ -0,0 +1,190 @@
|
||||
/*
|
||||
* 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:
|
||||
*
|
||||
* http://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.EmptyArrays;
|
||||
import io.netty.util.internal.PlatformDependent;
|
||||
import io.netty.util.internal.logging.InternalLogger;
|
||||
import io.netty.util.internal.logging.InternalLoggerFactory;
|
||||
|
||||
import javax.net.ssl.SSLContext;
|
||||
import javax.net.ssl.TrustManager;
|
||||
import javax.net.ssl.X509ExtendedTrustManager;
|
||||
import javax.net.ssl.X509TrustManager;
|
||||
import java.lang.reflect.Field;
|
||||
import java.security.AccessController;
|
||||
import java.security.KeyManagementException;
|
||||
import java.security.NoSuchAlgorithmException;
|
||||
import java.security.PrivilegedAction;
|
||||
import java.security.cert.CertificateException;
|
||||
import java.security.cert.X509Certificate;
|
||||
|
||||
/**
|
||||
* Utility which allows to wrap {@link X509TrustManager} implementations with the internal implementation used by
|
||||
* {@code SSLContextImpl} that provides extended verification.
|
||||
*
|
||||
* This is really a "hack" until there is an official API as requested on the in
|
||||
* <a href="https://bugs.openjdk.java.net/projects/JDK/issues/JDK-8210843">JDK-8210843</a>.
|
||||
*/
|
||||
final class OpenSslX509TrustManagerWrapper {
|
||||
private static final InternalLogger LOGGER = InternalLoggerFactory
|
||||
.getInstance(OpenSslX509TrustManagerWrapper.class);
|
||||
private static final TrustManagerWrapper WRAPPER;
|
||||
|
||||
static {
|
||||
// By default we will not do any wrapping but just return the passed in manager.
|
||||
TrustManagerWrapper wrapper = new TrustManagerWrapper() {
|
||||
@Override
|
||||
public X509TrustManager wrapIfNeeded(X509TrustManager manager) {
|
||||
return manager;
|
||||
}
|
||||
};
|
||||
|
||||
Throwable cause = null;
|
||||
Throwable unsafeCause = PlatformDependent.getUnsafeUnavailabilityCause();
|
||||
if (unsafeCause == null) {
|
||||
SSLContext context;
|
||||
try {
|
||||
context = newSSLContext();
|
||||
// Now init with an array that only holds a X509TrustManager. This should be wrapped into an
|
||||
// AbstractTrustManagerWrapper which will delegate the TrustManager itself but also do extra
|
||||
// validations.
|
||||
//
|
||||
// See:
|
||||
// - http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/
|
||||
// cadea780bc76/src/share/classes/sun/security/ssl/SSLContextImpl.java#l127
|
||||
context.init(null, new TrustManager[] {
|
||||
new X509TrustManager() {
|
||||
@Override
|
||||
public void checkClientTrusted(X509Certificate[] x509Certificates, String s)
|
||||
throws CertificateException {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void checkServerTrusted(X509Certificate[] x509Certificates, String s)
|
||||
throws CertificateException {
|
||||
}
|
||||
|
||||
@Override
|
||||
public X509Certificate[] getAcceptedIssuers() {
|
||||
return EmptyArrays.EMPTY_X509_CERTIFICATES;
|
||||
}
|
||||
}
|
||||
}, null);
|
||||
} catch (Throwable error) {
|
||||
context = null;
|
||||
cause = error;
|
||||
}
|
||||
if (cause != null) {
|
||||
LOGGER.debug("Unable to access wrapped TrustManager", cause);
|
||||
} else {
|
||||
final SSLContext finalContext = context;
|
||||
Object maybeWrapper = AccessController.doPrivileged(new PrivilegedAction<Object>() {
|
||||
@Override
|
||||
public Object run() {
|
||||
try {
|
||||
Field contextSpiField = SSLContext.class.getDeclaredField("contextSpi");
|
||||
final long spiOffset = PlatformDependent.objectFieldOffset(contextSpiField);
|
||||
Object spi = PlatformDependent.getObject(finalContext, spiOffset);
|
||||
if (spi != null) {
|
||||
Class<?> clazz = spi.getClass();
|
||||
|
||||
// Let's cycle through the whole hierarchy until we find what we are looking for or
|
||||
// there is nothing left in which case we will not wrap at all.
|
||||
do {
|
||||
try {
|
||||
Field trustManagerField = clazz.getDeclaredField("trustManager");
|
||||
final long tmOffset = PlatformDependent.objectFieldOffset(trustManagerField);
|
||||
Object trustManager = PlatformDependent.getObject(spi, tmOffset);
|
||||
if (trustManager instanceof X509ExtendedTrustManager) {
|
||||
return new UnsafeTrustManagerWrapper(spiOffset, tmOffset);
|
||||
}
|
||||
} catch (NoSuchFieldException ignore) {
|
||||
// try next
|
||||
}
|
||||
clazz = clazz.getSuperclass();
|
||||
} while (clazz != null);
|
||||
}
|
||||
throw new NoSuchFieldException();
|
||||
} catch (NoSuchFieldException e) {
|
||||
return e;
|
||||
} catch (SecurityException e) {
|
||||
return e;
|
||||
}
|
||||
}
|
||||
});
|
||||
if (maybeWrapper instanceof Throwable) {
|
||||
LOGGER.debug("Unable to access wrapped TrustManager", (Throwable) maybeWrapper);
|
||||
} else {
|
||||
wrapper = (TrustManagerWrapper) maybeWrapper;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
LOGGER.debug("Unable to access wrapped TrustManager", cause);
|
||||
}
|
||||
WRAPPER = wrapper;
|
||||
}
|
||||
|
||||
private OpenSslX509TrustManagerWrapper() { }
|
||||
|
||||
static X509TrustManager wrapIfNeeded(X509TrustManager trustManager) {
|
||||
return WRAPPER.wrapIfNeeded(trustManager);
|
||||
}
|
||||
|
||||
private interface TrustManagerWrapper {
|
||||
X509TrustManager wrapIfNeeded(X509TrustManager manager);
|
||||
}
|
||||
|
||||
private static SSLContext newSSLContext() throws NoSuchAlgorithmException {
|
||||
return SSLContext.getInstance("TLS");
|
||||
}
|
||||
|
||||
private static final class UnsafeTrustManagerWrapper implements TrustManagerWrapper {
|
||||
private final long spiOffset;
|
||||
private final long tmOffset;
|
||||
|
||||
UnsafeTrustManagerWrapper(long spiOffset, long tmOffset) {
|
||||
this.spiOffset = spiOffset;
|
||||
this.tmOffset = tmOffset;
|
||||
}
|
||||
|
||||
@Override
|
||||
public X509TrustManager wrapIfNeeded(X509TrustManager manager) {
|
||||
if (!(manager instanceof X509ExtendedTrustManager)) {
|
||||
try {
|
||||
SSLContext ctx = newSSLContext();
|
||||
ctx.init(null, new TrustManager[] { manager }, null);
|
||||
Object spi = PlatformDependent.getObject(ctx, spiOffset);
|
||||
if (spi != null) {
|
||||
Object tm = PlatformDependent.getObject(spi, tmOffset);
|
||||
if (tm instanceof X509ExtendedTrustManager) {
|
||||
return (X509TrustManager) tm;
|
||||
}
|
||||
}
|
||||
} catch (NoSuchAlgorithmException e) {
|
||||
// This should never happen as we did the same in the static
|
||||
// before.
|
||||
PlatformDependent.throwException(e);
|
||||
} catch (KeyManagementException e) {
|
||||
// This should never happen as we did the same in the static
|
||||
// before.
|
||||
PlatformDependent.throwException(e);
|
||||
}
|
||||
}
|
||||
return manager;
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -508,7 +508,7 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
|
||||
protected static X509TrustManager chooseTrustManager(TrustManager[] managers) {
|
||||
for (TrustManager m : managers) {
|
||||
if (m instanceof X509TrustManager) {
|
||||
return (X509TrustManager) m;
|
||||
return OpenSslX509TrustManagerWrapper.wrapIfNeeded((X509TrustManager) m);
|
||||
}
|
||||
}
|
||||
throw new IllegalStateException("no X509TrustManager found");
|
||||
|
||||
@ -35,6 +35,7 @@ import io.netty.channel.socket.nio.NioServerSocketChannel;
|
||||
import io.netty.channel.socket.nio.NioSocketChannel;
|
||||
import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
|
||||
import io.netty.handler.ssl.util.SelfSignedCertificate;
|
||||
import io.netty.handler.ssl.util.SimpleTrustManagerFactory;
|
||||
import io.netty.util.CharsetUtil;
|
||||
import io.netty.util.NetUtil;
|
||||
import io.netty.util.ReferenceCountUtil;
|
||||
@ -58,7 +59,9 @@ import java.io.InputStream;
|
||||
import java.net.InetSocketAddress;
|
||||
import java.nio.ByteBuffer;
|
||||
import java.nio.channels.ClosedChannelException;
|
||||
import java.security.InvalidAlgorithmParameterException;
|
||||
import java.security.KeyStore;
|
||||
import java.security.KeyStoreException;
|
||||
import java.security.Provider;
|
||||
import java.security.cert.Certificate;
|
||||
import java.security.cert.CertificateException;
|
||||
@ -71,6 +74,7 @@ import java.util.concurrent.ExecutionException;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import javax.net.ssl.KeyManagerFactory;
|
||||
import javax.net.ssl.ManagerFactoryParameters;
|
||||
import javax.net.ssl.SNIHostName;
|
||||
import javax.net.ssl.SSLEngine;
|
||||
import javax.net.ssl.SSLEngineResult;
|
||||
@ -78,6 +82,10 @@ import javax.net.ssl.SSLException;
|
||||
import javax.net.ssl.SSLHandshakeException;
|
||||
import javax.net.ssl.SSLParameters;
|
||||
import javax.net.ssl.SSLSession;
|
||||
import javax.net.ssl.TrustManager;
|
||||
import javax.net.ssl.TrustManagerFactory;
|
||||
import javax.net.ssl.TrustManagerFactorySpi;
|
||||
import javax.net.ssl.X509TrustManager;
|
||||
import javax.security.cert.X509Certificate;
|
||||
|
||||
import static io.netty.handler.ssl.SslUtils.PROTOCOL_SSL_V2;
|
||||
@ -2391,7 +2399,7 @@ public abstract class SSLEngineTest {
|
||||
Set<String> supported = new HashSet<String>(Arrays.asList(server.getSupportedProtocols()));
|
||||
if (supported.contains(protocol)) {
|
||||
server.setEnabledProtocols(server.getSupportedProtocols());
|
||||
Assert.assertEquals(supported, new HashSet<String>(Arrays.asList(server.getSupportedProtocols())));
|
||||
assertEquals(supported, new HashSet<String>(Arrays.asList(server.getSupportedProtocols())));
|
||||
|
||||
for (String disabled : disabledProtocols) {
|
||||
supported.remove(disabled);
|
||||
@ -2401,7 +2409,7 @@ public abstract class SSLEngineTest {
|
||||
return;
|
||||
}
|
||||
server.setEnabledProtocols(supported.toArray(new String[0]));
|
||||
Assert.assertEquals(supported, new HashSet<String>(Arrays.asList(server.getEnabledProtocols())));
|
||||
assertEquals(supported, new HashSet<String>(Arrays.asList(server.getEnabledProtocols())));
|
||||
server.setEnabledProtocols(server.getSupportedProtocols());
|
||||
}
|
||||
} finally {
|
||||
@ -2411,6 +2419,72 @@ public abstract class SSLEngineTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testUsingX509TrustManagerVerifiesHostname() throws Exception {
|
||||
SelfSignedCertificate cert = new SelfSignedCertificate();
|
||||
clientSslCtx = SslContextBuilder
|
||||
.forClient()
|
||||
.trustManager(new TrustManagerFactory(new TrustManagerFactorySpi() {
|
||||
@Override
|
||||
protected void engineInit(KeyStore keyStore) {
|
||||
// NOOP
|
||||
}
|
||||
@Override
|
||||
protected TrustManager[] engineGetTrustManagers() {
|
||||
// Provide a custom trust manager, this manager trust all certificates
|
||||
return new TrustManager[] {
|
||||
new X509TrustManager() {
|
||||
@Override
|
||||
public void checkClientTrusted(
|
||||
java.security.cert.X509Certificate[] x509Certificates, String s) {
|
||||
// NOOP
|
||||
}
|
||||
|
||||
@Override
|
||||
public void checkServerTrusted(
|
||||
java.security.cert.X509Certificate[] x509Certificates, String s) {
|
||||
// NOOP
|
||||
}
|
||||
|
||||
@Override
|
||||
public java.security.cert.X509Certificate[] getAcceptedIssuers() {
|
||||
return EmptyArrays.EMPTY_X509_CERTIFICATES;
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void engineInit(ManagerFactoryParameters managerFactoryParameters) {
|
||||
}
|
||||
}, null, TrustManagerFactory.getDefaultAlgorithm()) {
|
||||
})
|
||||
.sslProvider(sslClientProvider())
|
||||
.build();
|
||||
|
||||
SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT, "netty.io", 1234));
|
||||
SSLParameters sslParameters = client.getSSLParameters();
|
||||
sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
|
||||
client.setSSLParameters(sslParameters);
|
||||
|
||||
serverSslCtx = SslContextBuilder
|
||||
.forServer(cert.certificate(), cert.privateKey())
|
||||
.sslProvider(sslServerProvider())
|
||||
.build();
|
||||
|
||||
SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT));
|
||||
try {
|
||||
handshake(client, server);
|
||||
fail();
|
||||
} catch (SSLException expected) {
|
||||
// expected as the hostname not matches.
|
||||
} finally {
|
||||
cleanupClientSslEngine(client);
|
||||
cleanupServerSslEngine(server);
|
||||
cert.delete();
|
||||
}
|
||||
}
|
||||
|
||||
protected SSLEngine wrapEngine(SSLEngine engine) {
|
||||
return engine;
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user