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 #4736

Modifications:

Clear out all options that have disabled SSL protocols before attempting to enable any SSL protocol.

Result:

setEnabledProtocols works as expected.

handler/src/main/java/io/netty/handler/ssl/OpenSslEngine.java

@@ -1076,21 +1076,29 @@ public void setEnabledProtocols(String[] protocols) {
                 // 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));
             }

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 String select(List<String> protocols) {
         }
     }
 
+    @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);
     }

handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java

@@ -60,6 +60,13 @@ public void testAlpnCompatibleProtocolsDifferentClientOrder() throws Exception {
         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());

handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java

@@ -59,6 +59,9 @@
 
 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 void testSessionInvalidate() throws Exception {
         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);

pom.xml

@@ -284,7 +284,7 @@
       <dependency>
         <groupId>${project.groupId}</groupId>
         <artifactId>netty-tcnative</artifactId>
-        <version>1.1.33.Fork10</version>
+        <version>1.1.33.Fork11</version>
         <classifier>${tcnative.classifier}</classifier>
         <scope>compile</scope>
         <optional>true</optional>