From 325cc84a2ebbf4f5e362ec9ca09202b4647bd100 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 23 Feb 2017 09:42:00 +0100 Subject: [PATCH] Throw if SSLParameters contains settings that are not supported by ReferenceCountedOpenSslEngine Motivation: We not support all SSLParameters settings so we should better throw if a user try to use them. Modifications: - Check for unsupported parameters - Add unit test Result: Less surprising behavior. --- .../ssl/ReferenceCountedOpenSslEngine.java | 46 ++++++++----- .../netty/handler/ssl/OpenSslEngineTest.java | 69 +++++++++++++++++++ pom.xml | 1 + 3 files changed, 100 insertions(+), 16 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java index e3687640da..aeff67eab3 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java @@ -34,14 +34,17 @@ import io.netty.util.internal.logging.InternalLoggerFactory; import java.nio.ByteBuffer; import java.nio.ReadOnlyBufferException; +import java.security.AlgorithmConstraints; import java.security.Principal; import java.security.cert.Certificate; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; +import javax.net.ssl.SNIMatcher; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLException; @@ -1555,10 +1558,34 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc @Override public final synchronized void setSSLParameters(SSLParameters sslParameters) { - super.setSSLParameters(sslParameters); - int version = PlatformDependent.javaVersion(); if (version >= 7) { + if (sslParameters.getAlgorithmConstraints() != null) { + throw new IllegalArgumentException("AlgorithmConstraints are not supported."); + } + + if (version >= 8) { + Collection matchers = sslParameters.getSNIMatchers(); + if (matchers != null && !matchers.isEmpty()) { + throw new IllegalArgumentException("SNIMatchers are not supported."); + } + + if (!isDestroyed()) { + if (clientMode) { + final List sniHostNames = Java8SslParametersUtils.getSniHostNames(sslParameters); + for (String name: sniHostNames) { + SSL.setTlsExtHostName(ssl, name); + } + this.sniHostNames = sniHostNames; + } + if (Java8SslParametersUtils.getUseCipherSuitesOrder(sslParameters)) { + SSL.setOptions(ssl, SSL.SSL_OP_CIPHER_SERVER_PREFERENCE); + } else { + SSL.clearOptions(ssl, SSL.SSL_OP_CIPHER_SERVER_PREFERENCE); + } + } + } + final String endPointIdentificationAlgorithm = sslParameters.getEndpointIdentificationAlgorithm(); final boolean endPointVerificationEnabled = endPointIdentificationAlgorithm != null && !endPointIdentificationAlgorithm.isEmpty(); @@ -1572,21 +1599,8 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc this.endPointIdentificationAlgorithm = endPointIdentificationAlgorithm; algorithmConstraints = sslParameters.getAlgorithmConstraints(); - if (version >= 8 && !isDestroyed()) { - if (clientMode) { - final List sniHostNames = Java8SslParametersUtils.getSniHostNames(sslParameters); - for (String name: sniHostNames) { - SSL.setTlsExtHostName(ssl, name); - } - this.sniHostNames = sniHostNames; - } - if (Java8SslParametersUtils.getUseCipherSuitesOrder(sslParameters)) { - SSL.setOptions(ssl, SSL.SSL_OP_CIPHER_SERVER_PREFERENCE); - } else { - SSL.clearOptions(ssl, SSL.SSL_OP_CIPHER_SERVER_PREFERENCE); - } - } } + super.setSSLParameters(sslParameters); } private boolean isDestroyed() { diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java index 5dfa7b6792..d232d08eac 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java @@ -27,11 +27,20 @@ import org.junit.BeforeClass; import org.junit.Test; import java.nio.ByteBuffer; +import java.security.AlgorithmConstraints; +import java.security.AlgorithmParameters; +import java.security.CryptoPrimitive; +import java.security.Key; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Set; +import javax.net.ssl.SNIMatcher; +import javax.net.ssl.SNIServerName; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLException; +import javax.net.ssl.SSLParameters; import static io.netty.internal.tcnative.SSL.SSL_CVERIFY_IGNORED; import static org.junit.Assert.assertEquals; @@ -557,6 +566,66 @@ public class OpenSslEngineTest extends SSLEngineTest { assertFalse(src.hasRemaining()); } + @Test(expected = IllegalArgumentException.class) + public void testSNIMatchersThrows() throws Exception { + assumeTrue(PlatformDependent.javaVersion() >= 8); + SelfSignedCertificate ssc = new SelfSignedCertificate(); + serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) + .sslProvider(sslServerProvider()) + .build(); + + SSLEngine engine = serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT); + try { + SSLParameters parameters = new SSLParameters(); + SNIMatcher matcher = new SNIMatcher(0) { + @Override + public boolean matches(SNIServerName sniServerName) { + return false; + } + }; + parameters.setSNIMatchers(Collections.singleton(matcher)); + engine.setSSLParameters(parameters); + } finally { + cleanupServerSslEngine(engine); + ssc.delete(); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testAlgorithmConstraintsThrows() throws Exception { + SelfSignedCertificate ssc = new SelfSignedCertificate(); + serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) + .sslProvider(sslServerProvider()) + .build(); + + SSLEngine engine = serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT); + try { + SSLParameters parameters = new SSLParameters(); + parameters.setAlgorithmConstraints(new AlgorithmConstraints() { + @Override + public boolean permits( + Set primitives, String algorithm, AlgorithmParameters parameters) { + return false; + } + + @Override + public boolean permits(Set primitives, Key key) { + return false; + } + + @Override + public boolean permits( + Set primitives, String algorithm, Key key, AlgorithmParameters parameters) { + return false; + } + }); + engine.setSSLParameters(parameters); + } finally { + cleanupServerSslEngine(engine); + ssc.delete(); + } + } + @Override protected SslProvider sslClientProvider() { return SslProvider.OPENSSL; diff --git a/pom.xml b/pom.xml index e103a975f2..cd3eab0a39 100644 --- a/pom.xml +++ b/pom.xml @@ -702,6 +702,7 @@ javax.net.ssl.SSLParameters javax.net.ssl.SNIServerName javax.net.ssl.SNIHostName + javax.net.ssl.SNIMatcher java.security.AlgorithmConstraints java.security.cert.CertificateRevokedException java.security.cert.CertPathValidatorException