Use the correct alert depending on the CertificateException when using OpenSslEngine
Motivation: We tried to detect the correct alert to use depending on the CertificateException that is thrown by the TrustManager. This not worked all the time as depending on the TrustManager implementation it may also wrap a CertPathValidatorException. Modification: - Try to unwrap the CertificateException if needed and detect the right alert via the CertPathValidatorException. - Add unit to verify Result: Send the correct alert depending on the CertificateException when using OpenSslEngine.
This commit is contained in:
parent
e5070b7266
commit
9b7fb2f362
@ -35,6 +35,7 @@ import org.apache.tomcat.jni.SSLContext;
|
||||
import java.security.AccessController;
|
||||
import java.security.PrivateKey;
|
||||
import java.security.PrivilegedAction;
|
||||
import java.security.cert.CertPathValidatorException;
|
||||
import java.security.cert.Certificate;
|
||||
import java.security.cert.CertificateExpiredException;
|
||||
import java.security.cert.CertificateNotYetValidException;
|
||||
@ -605,7 +606,10 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
|
||||
e.initCause(cause);
|
||||
engine.handshakeException = e;
|
||||
|
||||
// Try to extract the correct error code that should be used.
|
||||
if (cause instanceof OpenSslCertificateException) {
|
||||
// This will never return a negative error code as its validated when constructing the
|
||||
// OpenSslCertificateException.
|
||||
return ((OpenSslCertificateException) cause).errorCode();
|
||||
}
|
||||
if (cause instanceof CertificateExpiredException) {
|
||||
@ -614,9 +618,34 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
|
||||
if (cause instanceof CertificateNotYetValidException) {
|
||||
return CertificateVerifier.X509_V_ERR_CERT_NOT_YET_VALID;
|
||||
}
|
||||
if (PlatformDependent.javaVersion() >= 7 && cause instanceof CertificateRevokedException) {
|
||||
return CertificateVerifier.X509_V_ERR_CERT_REVOKED;
|
||||
if (PlatformDependent.javaVersion() >= 7) {
|
||||
if (cause instanceof CertificateRevokedException) {
|
||||
return CertificateVerifier.X509_V_ERR_CERT_REVOKED;
|
||||
}
|
||||
|
||||
// The X509TrustManagerImpl uses a Validator which wraps a CertPathValidatorException into
|
||||
// an CertificateException. So we need to handle the wrapped CertPathValidatorException to be
|
||||
// able to send the correct alert.
|
||||
Throwable wrapped = cause.getCause();
|
||||
while (wrapped != null) {
|
||||
if (wrapped instanceof CertPathValidatorException) {
|
||||
CertPathValidatorException ex = (CertPathValidatorException) wrapped;
|
||||
CertPathValidatorException.Reason reason = ex.getReason();
|
||||
if (reason == CertPathValidatorException.BasicReason.EXPIRED) {
|
||||
return CertificateVerifier.X509_V_ERR_CERT_HAS_EXPIRED;
|
||||
}
|
||||
if (reason == CertPathValidatorException.BasicReason.NOT_YET_VALID) {
|
||||
return CertificateVerifier.X509_V_ERR_CERT_NOT_YET_VALID;
|
||||
}
|
||||
if (reason == CertPathValidatorException.BasicReason.REVOKED) {
|
||||
return CertificateVerifier.X509_V_ERR_CERT_REVOKED;
|
||||
}
|
||||
}
|
||||
wrapped = wrapped.getCause();
|
||||
}
|
||||
}
|
||||
|
||||
// Could not detect a specific error code to use, so fallback to a default code.
|
||||
return CertificateVerifier.X509_V_ERR_UNSPECIFIED;
|
||||
}
|
||||
}
|
||||
|
248
handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java
Normal file
248
handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java
Normal file
@ -0,0 +1,248 @@
|
||||
/*
|
||||
* Copyright 2016 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.bootstrap.Bootstrap;
|
||||
import io.netty.bootstrap.ServerBootstrap;
|
||||
import io.netty.channel.Channel;
|
||||
import io.netty.channel.ChannelHandlerContext;
|
||||
import io.netty.channel.ChannelInboundHandlerAdapter;
|
||||
import io.netty.channel.ChannelInitializer;
|
||||
import io.netty.channel.EventLoopGroup;
|
||||
import io.netty.channel.nio.NioEventLoopGroup;
|
||||
import io.netty.channel.socket.nio.NioServerSocketChannel;
|
||||
import io.netty.channel.socket.nio.NioSocketChannel;
|
||||
import io.netty.handler.logging.LogLevel;
|
||||
import io.netty.handler.logging.LoggingHandler;
|
||||
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.ReferenceCountUtil;
|
||||
import io.netty.util.concurrent.Promise;
|
||||
import io.netty.util.internal.EmptyArrays;
|
||||
import org.junit.Assume;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.junit.runners.Parameterized;
|
||||
import sun.security.validator.ValidatorException;
|
||||
|
||||
import javax.net.ssl.ManagerFactoryParameters;
|
||||
import javax.net.ssl.SSLException;
|
||||
import javax.net.ssl.TrustManager;
|
||||
import javax.net.ssl.X509TrustManager;
|
||||
import javax.security.auth.x500.X500Principal;
|
||||
import java.io.File;
|
||||
import java.security.KeyStore;
|
||||
import java.security.cert.CRLReason;
|
||||
import java.security.cert.CertPathValidatorException;
|
||||
import java.security.cert.CertificateException;
|
||||
import java.security.cert.CertificateExpiredException;
|
||||
import java.security.cert.CertificateNotYetValidException;
|
||||
import java.security.cert.CertificateRevokedException;
|
||||
import java.security.cert.Extension;
|
||||
import java.security.cert.X509Certificate;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.Date;
|
||||
import java.util.List;
|
||||
|
||||
|
||||
@RunWith(Parameterized.class)
|
||||
public class SslErrorTest {
|
||||
|
||||
@Parameterized.Parameters(name = "{index}: serverProvider = {0}, clientProvider = {1}, exception = {2}")
|
||||
public static Collection<Object[]> data() {
|
||||
List<SslProvider> serverProviders = new ArrayList<SslProvider>(2);
|
||||
List<SslProvider> clientProviders = new ArrayList<SslProvider>(3);
|
||||
|
||||
if (OpenSsl.isAvailable()) {
|
||||
serverProviders.add(SslProvider.OPENSSL);
|
||||
serverProviders.add(SslProvider.OPENSSL_REFCNT);
|
||||
clientProviders.add(SslProvider.OPENSSL);
|
||||
clientProviders.add(SslProvider.OPENSSL_REFCNT);
|
||||
}
|
||||
// We not test with SslProvider.JDK on the server side as the JDK implementation currently just send the same
|
||||
// alert all the time, sigh.....
|
||||
clientProviders.add(SslProvider.JDK);
|
||||
|
||||
List<CertificateException> exceptions = new ArrayList<CertificateException>(6);
|
||||
exceptions.add(new CertificateExpiredException());
|
||||
exceptions.add(new CertificateNotYetValidException());
|
||||
exceptions.add(new CertificateRevokedException(
|
||||
new Date(), CRLReason.AA_COMPROMISE, new X500Principal(""),
|
||||
Collections.<String, Extension>emptyMap()));
|
||||
|
||||
// Also use wrapped exceptions as this is what the JDK implementation of X509TrustManagerFactory is doing.
|
||||
exceptions.add(newCertificateException(CertPathValidatorException.BasicReason.EXPIRED));
|
||||
exceptions.add(newCertificateException(CertPathValidatorException.BasicReason.NOT_YET_VALID));
|
||||
exceptions.add(newCertificateException(CertPathValidatorException.BasicReason.REVOKED));
|
||||
|
||||
List<Object[]> params = new ArrayList<Object[]>();
|
||||
for (SslProvider serverProvider: serverProviders) {
|
||||
for (SslProvider clientProvider: clientProviders) {
|
||||
for (CertificateException exception: exceptions) {
|
||||
params.add(new Object[] { serverProvider, clientProvider, exception});
|
||||
}
|
||||
}
|
||||
}
|
||||
return params;
|
||||
}
|
||||
|
||||
private static CertificateException newCertificateException(CertPathValidatorException.Reason reason) {
|
||||
return new ValidatorException("Test exception",
|
||||
new CertPathValidatorException("x", null, null, -1, reason));
|
||||
}
|
||||
|
||||
private final SslProvider serverProvider;
|
||||
private final SslProvider clientProvider;
|
||||
private final CertificateException exception;
|
||||
|
||||
public SslErrorTest(SslProvider serverProvider, SslProvider clientProvider, CertificateException exception) {
|
||||
this.serverProvider = serverProvider;
|
||||
this.clientProvider = clientProvider;
|
||||
this.exception = exception;
|
||||
}
|
||||
|
||||
@Test(timeout = 10000L)
|
||||
public void testCorrectAlert() throws Exception {
|
||||
// As this only works correctly at the moment when OpenSslEngine is used on the server-side there is
|
||||
// no need to run it if there is no openssl is available at all.
|
||||
Assume.assumeTrue(OpenSsl.isAvailable());
|
||||
|
||||
SelfSignedCertificate ssc = new SelfSignedCertificate();
|
||||
final SslContext sslServerCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey())
|
||||
.sslProvider(serverProvider)
|
||||
.trustManager(new SimpleTrustManagerFactory() {
|
||||
@Override
|
||||
protected void engineInit(KeyStore keyStore) { }
|
||||
@Override
|
||||
protected void engineInit(ManagerFactoryParameters managerFactoryParameters) { }
|
||||
|
||||
@Override
|
||||
protected TrustManager[] engineGetTrustManagers() {
|
||||
return new TrustManager[] { new X509TrustManager() {
|
||||
|
||||
@Override
|
||||
public void checkClientTrusted(X509Certificate[] x509Certificates, String s)
|
||||
throws CertificateException {
|
||||
throw exception;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void checkServerTrusted(X509Certificate[] x509Certificates, String s)
|
||||
throws CertificateException {
|
||||
// NOOP
|
||||
}
|
||||
|
||||
@Override
|
||||
public X509Certificate[] getAcceptedIssuers() {
|
||||
return EmptyArrays.EMPTY_X509_CERTIFICATES;
|
||||
}
|
||||
} };
|
||||
}
|
||||
}).clientAuth(ClientAuth.REQUIRE).build();
|
||||
|
||||
final SslContext sslClientCtx = SslContextBuilder.forClient()
|
||||
.trustManager(InsecureTrustManagerFactory.INSTANCE)
|
||||
.keyManager(new File(getClass().getResource("test.crt").getFile()),
|
||||
new File(getClass().getResource("test_unencrypted.pem").getFile()))
|
||||
.sslProvider(clientProvider).build();
|
||||
|
||||
Channel serverChannel = null;
|
||||
Channel clientChannel = null;
|
||||
EventLoopGroup group = new NioEventLoopGroup();
|
||||
try {
|
||||
serverChannel = new ServerBootstrap().group(group)
|
||||
.channel(NioServerSocketChannel.class)
|
||||
.handler(new LoggingHandler(LogLevel.INFO))
|
||||
.childHandler(new ChannelInitializer<Channel>() {
|
||||
@Override
|
||||
protected void initChannel(Channel ch) throws Exception {
|
||||
ch.pipeline().addLast(sslServerCtx.newHandler(ch.alloc()));
|
||||
ch.pipeline().addLast(new ChannelInboundHandlerAdapter() {
|
||||
|
||||
@Override
|
||||
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
|
||||
ctx.close();
|
||||
}
|
||||
});
|
||||
}
|
||||
}).bind(0).sync().channel();
|
||||
|
||||
final Promise<Void> promise = group.next().newPromise();
|
||||
|
||||
clientChannel = new Bootstrap().group(group)
|
||||
.channel(NioSocketChannel.class)
|
||||
.handler(new ChannelInitializer<Channel>() {
|
||||
@Override
|
||||
protected void initChannel(Channel ch) throws Exception {
|
||||
ch.pipeline().addLast(sslClientCtx.newHandler(ch.alloc()));
|
||||
ch.pipeline().addLast(new ChannelInboundHandlerAdapter() {
|
||||
@Override
|
||||
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
|
||||
// Unwrap as its wrapped by a DecoderException
|
||||
Throwable unwrappedCause = cause.getCause();
|
||||
if (unwrappedCause instanceof SSLException) {
|
||||
if (exception instanceof ValidatorException) {
|
||||
CertPathValidatorException.Reason reason =
|
||||
((CertPathValidatorException) exception.getCause()).getReason();
|
||||
if (reason == CertPathValidatorException.BasicReason.EXPIRED) {
|
||||
verifyException(unwrappedCause, "expired", promise);
|
||||
} else if (reason == CertPathValidatorException.BasicReason.NOT_YET_VALID) {
|
||||
verifyException(unwrappedCause, "bad", promise);
|
||||
} else if (reason == CertPathValidatorException.BasicReason.REVOKED) {
|
||||
verifyException(unwrappedCause, "revoked", promise);
|
||||
}
|
||||
} else if (exception instanceof CertificateExpiredException) {
|
||||
verifyException(unwrappedCause, "expired", promise);
|
||||
} else if (exception instanceof CertificateNotYetValidException) {
|
||||
verifyException(unwrappedCause, "bad", promise);
|
||||
} else if (exception instanceof CertificateRevokedException) {
|
||||
verifyException(unwrappedCause, "revoked", promise);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
}).connect(serverChannel.localAddress()).syncUninterruptibly().channel();
|
||||
// Block until we received the correct exception
|
||||
promise.syncUninterruptibly();
|
||||
} finally {
|
||||
if (clientChannel != null) {
|
||||
clientChannel.close().syncUninterruptibly();
|
||||
}
|
||||
if (serverChannel != null) {
|
||||
serverChannel.close().syncUninterruptibly();
|
||||
}
|
||||
group.shutdownGracefully();
|
||||
|
||||
ReferenceCountUtil.release(sslServerCtx);
|
||||
ReferenceCountUtil.release(sslClientCtx);
|
||||
}
|
||||
}
|
||||
|
||||
// Its a bit hacky to verify against the message that is part of the exception but there is no other way
|
||||
// at the moment as there are no different exceptions for the different alerts.
|
||||
private static void verifyException(Throwable cause, String messagePart, Promise<Void> promise) {
|
||||
String message = cause.getMessage();
|
||||
if (message.contains(messagePart)) {
|
||||
promise.setSuccess(null);
|
||||
} else {
|
||||
promise.setFailure(new AssertionError("message not contains '" + messagePart + "': " + message));
|
||||
}
|
||||
}
|
||||
}
|
3
pom.xml
3
pom.xml
@ -723,6 +723,9 @@
|
||||
<ignore>javax.net.ssl.SSLParameters</ignore>
|
||||
<ignore>java.security.AlgorithmConstraints</ignore>
|
||||
<ignore>java.security.cert.CertificateRevokedException</ignore>
|
||||
<ignore>java.security.cert.CertPathValidatorException</ignore>
|
||||
<ignore>java.security.cert.CertPathValidatorException$Reason</ignore>
|
||||
<ignore>java.security.cert.CertPathValidatorException$BasicReason</ignore>
|
||||
|
||||
<ignore>java.util.concurrent.ConcurrentLinkedDeque</ignore>
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user