HADOOP-19876. SSL protocol config is not applied to Jetty when set to default value

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java

@@ -706,34 +706,32 @@ private Timer makeConfigurationChangeMonitor(long reloadInterval,
     private void setEnabledProtocols(SslContextFactory sslContextFactory) {
       String enabledProtocols = conf.get(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY,
           SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT);
-      if (!enabledProtocols.equals(SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT)) {
-        // Jetty 9.2.4.v20141103 and above excludes certain protocols by
-        // default. Remove the user enabled protocols from the exclude list,
-        // and add them into the include list.
-        String[] jettyExcludedProtocols =
-            sslContextFactory.getExcludeProtocols();
-        String[] enabledProtocolsArray =
-            StringUtils.getTrimmedStrings(enabledProtocols);
-        List<String> enabledProtocolsList =
-            Arrays.asList(enabledProtocolsArray);
-
-        List<String> resetExcludedProtocols = new ArrayList<>();
-        for (String jettyExcludedProtocol: jettyExcludedProtocols) {
-          if (!enabledProtocolsList.contains(jettyExcludedProtocol)) {
-            resetExcludedProtocols.add(jettyExcludedProtocol);
-          } else {
-            LOG.debug("Removed {} from exclude protocol list",
-                jettyExcludedProtocol);
-          }
+      // Jetty 9.2.4.v20141103 and above excludes certain protocols by
+      // default. Remove the user enabled protocols from the exclude list,
+      // and add them into the include list.
+      String[] jettyExcludedProtocols =
+          sslContextFactory.getExcludeProtocols();
+      String[] enabledProtocolsArray =
+          StringUtils.getTrimmedStrings(enabledProtocols);
+      List<String> enabledProtocolsList =
+          Arrays.asList(enabledProtocolsArray);
+
+      List<String> resetExcludedProtocols = new ArrayList<>();
+      for (String jettyExcludedProtocol: jettyExcludedProtocols) {
+        if (!enabledProtocolsList.contains(jettyExcludedProtocol)) {
+          resetExcludedProtocols.add(jettyExcludedProtocol);
+        } else {
+          LOG.debug("Removed {} from exclude protocol list",
+              jettyExcludedProtocol);
         }
+      }
 
-        sslContextFactory.setExcludeProtocols(
-            resetExcludedProtocols.toArray(new String[0]));
-        LOG.info("Reset exclude protocol list: {}", resetExcludedProtocols);
+      sslContextFactory.setExcludeProtocols(
+          resetExcludedProtocols.toArray(new String[0]));
+      LOG.info("Reset exclude protocol list: {}", resetExcludedProtocols);
 
-        sslContextFactory.setIncludeProtocols(enabledProtocolsArray);
-        LOG.info("Enabled protocols: {}", enabledProtocols);
-      }
+      sslContextFactory.setIncludeProtocols(enabledProtocolsArray);
+      LOG.info("Enabled protocols: {}", enabledProtocols);
     }
   }
 

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestSSLHttpServer.java

@@ -136,8 +136,10 @@ public static void setup() throws Exception {
 
   private static void setupServer(Configuration conf, Configuration sslConf)
       throws IOException, URISyntaxException {
-    conf.set(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY, INCLUDED_PROTOCOLS);
-    sslConf.set(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY, INCLUDED_PROTOCOLS);
+    String protocols = Shell.isJavaVersionAtLeast(11)
+        ? INCLUDED_PROTOCOLS_JDK11 : INCLUDED_PROTOCOLS;
+    conf.set(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY, protocols);
+    sslConf.set(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY, protocols);
     server = new HttpServer2.Builder().setName("test")
         .addEndpoint(new URI("https://localhost")).setConf(conf)
         .keyPassword(

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestSSLHttpServerConfigs.java

@@ -22,11 +22,15 @@
 import java.io.File;
 import java.io.IOException;
 import java.net.URI;
+import java.util.Arrays;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.security.ssl.KeyStoreTestUtil;
 import org.apache.hadoop.security.ssl.SSLFactory;
 import org.apache.hadoop.test.GenericTestUtils;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.server.SslConnectionFactory;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -39,6 +43,9 @@
 import static org.apache.hadoop.security.ssl.KeyStoreTestUtil.CLIENT_KEY_STORE_PASSWORD_DEFAULT;
 import static org.apache.hadoop.security.ssl.KeyStoreTestUtil.SERVER_KEY_STORE_PASSWORD_DEFAULT;
 import static org.apache.hadoop.security.ssl.KeyStoreTestUtil.TRUST_STORE_PASSWORD_DEFAULT;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.fail;
 
 /**
@@ -117,6 +124,28 @@ private HttpServer2 setupServer(String keyStoreKeyPassword,
     return server;
   }
 
+  private void assertServerAppliesEnabledProtocol(String protocol)
+      throws Exception {
+    HttpServer2 server = setupServer(SERVER_PWD, SERVER_PWD, TRUST_STORE_PWD);
+    try {
+      ServerConnector listener = server.getListeners().get(0);
+      SslConnectionFactory connectionFactory =
+          listener.getConnectionFactory(SslConnectionFactory.class);
+      assertNotNull(connectionFactory,
+          "Expected HTTPS listener with an SSL connection factory");
+      SslContextFactory sslContextFactory =
+          connectionFactory.getSslContextFactory();
+      assertArrayEquals(new String[] {protocol},
+          sslContextFactory.getIncludeProtocols());
+      assertFalse(Arrays.asList(sslContextFactory.getExcludeProtocols())
+              .contains(protocol),
+          "Configured enabled protocol should be removed from excluded "
+              + "protocols");
+    } finally {
+      server.stop();
+    }
+  }
+
   /**
    * Test if HttpServer2 start succeeds in validating KeyStore/ TrustStore
    * using the given passowords.
@@ -275,4 +304,31 @@ public void testKeyStoreSetupWithoutPassword() throws Exception {
           e.getCause());
     }
   }
+
+  @Test
+  @Timeout(value = 120)
+  public void testDefaultEnabledProtocolIsAppliedWhenConfigUnset()
+      throws Exception {
+    setupKeyStores(SERVER_PWD, CLIENT_PWD, TRUST_STORE_PWD);
+    conf.unset(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY);
+    assertServerAppliesEnabledProtocol(SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT);
+  }
+
+  @Test
+  @Timeout(value = 120)
+  public void testDefaultEnabledProtocolIsAppliedWhenConfigExplicitlySet()
+      throws Exception {
+    setupKeyStores(SERVER_PWD, CLIENT_PWD, TRUST_STORE_PWD);
+    conf.set(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY,
+        SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT);
+    assertServerAppliesEnabledProtocol(SSLFactory.SSL_ENABLED_PROTOCOLS_DEFAULT);
+  }
+
+  @Test
+  @Timeout(value = 120)
+  public void testNonDefaultEnabledProtocolIsApplied() throws Exception {
+    setupKeyStores(SERVER_PWD, CLIENT_PWD, TRUST_STORE_PWD);
+    conf.set(SSLFactory.SSL_ENABLED_PROTOCOLS_KEY, "TLSv1.3");
+    assertServerAppliesEnabledProtocol("TLSv1.3");
+  }
 }