From 7090d1331c327f9093ef260f9d0e3eef1a3d6715 Mon Sep 17 00:00:00 2001 From: Brendt Lucas Date: Wed, 20 Jan 2016 21:52:50 +0000 Subject: [PATCH] Clear disabled SSL protocols before enabling provided SSL protocols Motivation: Attempts to enable SSL protocols which are currently disabled fail when using the OpenSslEngine. Related to https://github.com/netty/netty/issues/4736 Modifications: Clear out all options that have disabled SSL protocols before attempting to enable any SSL protocol. Result: setEnabledProtocols works as expected. --- .../io/netty/handler/ssl/OpenSslEngine.java | 18 ++++++--- .../netty/handler/ssl/JdkSslEngineTest.java | 22 ++++++----- .../netty/handler/ssl/OpenSslEngineTest.java | 7 ++++ .../io/netty/handler/ssl/SSLEngineTest.java | 37 +++++++++++++++++++ pom.xml | 2 +- 5 files changed, 71 insertions(+), 15 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java index 91469ab0c1..1190030dc6 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java @@ -1076,21 +1076,29 @@ public final class OpenSslEngine extends SSLEngine { // Enable all and then disable what we not want SSL.setOptions(ssl, SSL.SSL_OP_ALL); + // Clear out options which disable protocols + SSL.clearOptions(ssl, SSL.SSL_OP_NO_SSLv2 | SSL.SSL_OP_NO_SSLv3 | SSL.SSL_OP_NO_TLSv1 | + SSL.SSL_OP_NO_TLSv1_1 | SSL.SSL_OP_NO_TLSv1_2); + + int opts = 0; if (!sslv2) { - SSL.setOptions(ssl, SSL.SSL_OP_NO_SSLv2); + opts |= SSL.SSL_OP_NO_SSLv2; } if (!sslv3) { - SSL.setOptions(ssl, SSL.SSL_OP_NO_SSLv3); + opts |= SSL.SSL_OP_NO_SSLv3; } if (!tlsv1) { - SSL.setOptions(ssl, SSL.SSL_OP_NO_TLSv1); + opts |= SSL.SSL_OP_NO_TLSv1; } if (!tlsv1_1) { - SSL.setOptions(ssl, SSL.SSL_OP_NO_TLSv1_1); + opts |= SSL.SSL_OP_NO_TLSv1_1; } if (!tlsv1_2) { - SSL.setOptions(ssl, SSL.SSL_OP_NO_TLSv1_2); + opts |= SSL.SSL_OP_NO_TLSv1_2; } + + // Disable protocols we do not want + SSL.setOptions(ssl, opts); } else { throw new IllegalStateException("failed to enable protocols: " + Arrays.asList(protocols)); } diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java index 6143348ae8..5efb21fff6 100644 --- a/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java @@ -15,25 +15,24 @@ */ package io.netty.handler.ssl; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeNoException; -import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelector; -import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelectorFactory; import io.netty.handler.ssl.ApplicationProtocolConfig.Protocol; import io.netty.handler.ssl.ApplicationProtocolConfig.SelectedListenerFailureBehavior; import io.netty.handler.ssl.ApplicationProtocolConfig.SelectorFailureBehavior; +import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelector; +import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelectorFactory; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; import io.netty.handler.ssl.util.SelfSignedCertificate; +import org.junit.Test; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLHandshakeException; import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLHandshakeException; - -import org.junit.Test; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeNoException; public class JdkSslEngineTest extends SSLEngineTest { private static final String PREFERRED_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http2"; @@ -263,6 +262,11 @@ public class JdkSslEngineTest extends SSLEngineTest { } } + @Test + public void testEnablingAnAlreadyDisabledSslProtocol() throws Exception { + testEnablingAnAlreadyDisabledSslProtocol(new String[]{}, new String[]{PROTOCOL_TLS_V1_2}); + } + private void runTest() throws Exception { runTest(PREFERRED_APPLICATION_LEVEL_PROTOCOL); } 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 18c0b1e3ac..a0701d0ca9 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java @@ -60,6 +60,13 @@ public class OpenSslEngineTest extends SSLEngineTest { runTest(PREFERRED_APPLICATION_LEVEL_PROTOCOL); } + @Test + public void testEnablingAnAlreadyDisabledSslProtocol() throws Exception { + assumeTrue(OpenSsl.isAvailable()); + testEnablingAnAlreadyDisabledSslProtocol(new String[]{PROTOCOL_SSL_V2_HELLO}, + new String[]{PROTOCOL_SSL_V2_HELLO, PROTOCOL_TLS_V1_2}); + } + @Override public void testMutualAuthSameCerts() throws Exception { assumeTrue(OpenSsl.isAvailable()); diff --git a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java index b6b5febaec..d6459208ce 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java @@ -59,6 +59,9 @@ import static org.mockito.Mockito.verify; public abstract class SSLEngineTest { + protected static final String PROTOCOL_TLS_V1_2 = "TLSv1.2"; + protected static final String PROTOCOL_SSL_V2_HELLO = "SSLv2Hello"; + @Mock protected MessageReceiver serverReceiver; @Mock @@ -329,6 +332,40 @@ public abstract class SSLEngineTest { assertFalse(session.isValid()); } + protected void testEnablingAnAlreadyDisabledSslProtocol(String[] protocols1, String[] protocols2) throws Exception { + SSLEngine sslEngine = null; + try { + File serverKeyFile = new File(getClass().getResource("test_unencrypted.pem").getFile()); + File serverCrtFile = new File(getClass().getResource("test.crt").getFile()); + SslContext sslContext = SslContextBuilder.forServer(serverCrtFile, serverKeyFile) + .sslProvider(sslProvider()) + .build(); + + sslEngine = sslContext.newEngine(UnpooledByteBufAllocator.DEFAULT); + + // Disable all protocols + sslEngine.setEnabledProtocols(new String[]{}); + + // The only protocol that should be enabled is SSLv2Hello + String[] enabledProtocols = sslEngine.getEnabledProtocols(); + assertEquals(protocols1.length, enabledProtocols.length); + assertArrayEquals(protocols1, enabledProtocols); + + // Enable a protocol that is currently disabled + sslEngine.setEnabledProtocols(new String[]{PROTOCOL_TLS_V1_2}); + + // The protocol that was just enabled should be returned + enabledProtocols = sslEngine.getEnabledProtocols(); + assertEquals(protocols2.length, enabledProtocols.length); + assertArrayEquals(protocols2, enabledProtocols); + } finally { + if (sslEngine != null) { + sslEngine.closeInbound(); + sslEngine.closeOutbound(); + } + } + } + private static void handshake(SSLEngine clientEngine, SSLEngine serverEngine) throws SSLException { int netBufferSize = 17 * 1024; ByteBuffer cTOs = ByteBuffer.allocateDirect(netBufferSize); diff --git a/pom.xml b/pom.xml index c8c131060b..7f906a2720 100644 --- a/pom.xml +++ b/pom.xml @@ -298,7 +298,7 @@ ${project.groupId} netty-tcnative - 1.1.33.Fork10 + 1.1.33.Fork11 ${tcnative.classifier} compile true