From 620dad0c2602440f2c457a5cb2a4c7a29e786bda Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 27 Jul 2018 01:56:32 +0800 Subject: [PATCH] Allow to validate sni hostname with underscore (#8150) Motivation: We should allow to also validate sni hostname which contains for example underscore when using our native SSL impl. The JDK implementation does this as well. Modifications: - Construct the SNIHostName via byte[] and not String. - Add unit test Result: Fixes https://github.com/netty/netty/issues/8144. --- .../io/netty/handler/ssl/Java8SslUtils.java | 2 +- .../ssl/ReferenceCountedOpenSslEngine.java | 3 ++- .../ReferenceCountedOpenSslServerContext.java | 4 ++- .../netty/handler/ssl/Java8SslTestUtils.java | 5 ++-- .../netty/handler/ssl/OpenSslEngineTest.java | 26 ++++++++++++++++++- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/Java8SslUtils.java b/handler/src/main/java/io/netty/handler/ssl/Java8SslUtils.java index 583d4cf498..b40c121748 100644 --- a/handler/src/main/java/io/netty/handler/ssl/Java8SslUtils.java +++ b/handler/src/main/java/io/netty/handler/ssl/Java8SslUtils.java @@ -69,7 +69,7 @@ final class Java8SslUtils { } @SuppressWarnings("unchecked") - static boolean checkSniHostnameMatch(Collection matchers, String hostname) { + static boolean checkSniHostnameMatch(Collection matchers, byte[] hostname) { if (matchers != null && !matchers.isEmpty()) { SNIHostName name = new SNIHostName(hostname); Iterator matcherIt = (Iterator) matchers.iterator(); 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 b98e9858d7..8b14735766 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java @@ -20,6 +20,7 @@ import io.netty.buffer.ByteBufAllocator; import io.netty.internal.tcnative.Buffer; import io.netty.internal.tcnative.SSL; import io.netty.util.AbstractReferenceCounted; +import io.netty.util.CharsetUtil; import io.netty.util.ReferenceCounted; import io.netty.util.ResourceLeakDetector; import io.netty.util.ResourceLeakDetectorFactory; @@ -1817,7 +1818,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc return destroyed != 0; } - final boolean checkSniHostnameMatch(String hostname) { + final boolean checkSniHostnameMatch(byte[] hostname) { return Java8SslUtils.checkSniHostnameMatch(matchers, hostname); } 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 9f09393f01..b9c8cf60b8 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java @@ -19,6 +19,7 @@ import io.netty.buffer.ByteBufAllocator; import io.netty.internal.tcnative.SSL; import io.netty.internal.tcnative.SSLContext; import io.netty.internal.tcnative.SniHostNameMatcher; +import io.netty.util.CharsetUtil; import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -244,7 +245,8 @@ public final class ReferenceCountedOpenSslServerContext extends ReferenceCounted public boolean match(long ssl, String hostname) { ReferenceCountedOpenSslEngine engine = engineMap.get(ssl); if (engine != null) { - return engine.checkSniHostnameMatch(hostname); + // TODO: In the next release of tcnative we should pass the byte[] directly in and not use a String. + return engine.checkSniHostnameMatch(hostname.getBytes(CharsetUtil.UTF_8)); } logger.warn("No ReferenceCountedOpenSslEngine found for SSL pointer: {}", ssl); return false; diff --git a/handler/src/test/java/io/netty/handler/ssl/Java8SslTestUtils.java b/handler/src/test/java/io/netty/handler/ssl/Java8SslTestUtils.java index 32219ff155..50fb935d9d 100644 --- a/handler/src/test/java/io/netty/handler/ssl/Java8SslTestUtils.java +++ b/handler/src/test/java/io/netty/handler/ssl/Java8SslTestUtils.java @@ -23,17 +23,18 @@ import javax.net.ssl.SNIServerName; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; import java.security.Provider; +import java.util.Arrays; import java.util.Collections; final class Java8SslTestUtils { private Java8SslTestUtils() { } - static void setSNIMatcher(SSLParameters parameters) { + static void setSNIMatcher(SSLParameters parameters, final byte[] match) { SNIMatcher matcher = new SNIMatcher(0) { @Override public boolean matches(SNIServerName sniServerName) { - return false; + return Arrays.equals(match, sniServerName.getEncoded()); } }; parameters.setSNIMatchers(Collections.singleton(matcher)); 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 9be57e5468..99fb8fa82e 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java @@ -22,6 +22,8 @@ import io.netty.handler.ssl.ApplicationProtocolConfig.SelectorFailureBehavior; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; import io.netty.handler.ssl.util.SelfSignedCertificate; import io.netty.internal.tcnative.SSL; +import io.netty.util.CharsetUtil; +import io.netty.util.internal.EmptyArrays; import io.netty.util.internal.PlatformDependent; import org.junit.Assume; import org.junit.BeforeClass; @@ -986,7 +988,7 @@ public class OpenSslEngineTest extends SSLEngineTest { SSLEngine engine = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); try { SSLParameters parameters = new SSLParameters(); - Java8SslTestUtils.setSNIMatcher(parameters); + Java8SslTestUtils.setSNIMatcher(parameters, EmptyArrays.EMPTY_BYTES); engine.setSSLParameters(parameters); } finally { cleanupServerSslEngine(engine); @@ -994,6 +996,28 @@ public class OpenSslEngineTest extends SSLEngineTest { } } + @Test + public void testSNIMatchersWithSNINameWithUnderscore() throws Exception { + assumeTrue(PlatformDependent.javaVersion() >= 8); + byte[] name = "rb8hx3pww30y3tvw0mwy.v1_1".getBytes(CharsetUtil.UTF_8); + SelfSignedCertificate ssc = new SelfSignedCertificate(); + serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) + .sslProvider(sslServerProvider()) + .build(); + + SSLEngine engine = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); + try { + SSLParameters parameters = new SSLParameters(); + Java8SslTestUtils.setSNIMatcher(parameters, name); + engine.setSSLParameters(parameters); + assertTrue(unwrapEngine(engine).checkSniHostnameMatch(name)); + assertFalse(unwrapEngine(engine).checkSniHostnameMatch("other".getBytes(CharsetUtil.UTF_8))); + } finally { + cleanupServerSslEngine(engine); + ssc.delete(); + } + } + @Test(expected = IllegalArgumentException.class) public void testAlgorithmConstraintsThrows() throws Exception { SelfSignedCertificate ssc = new SelfSignedCertificate();