ZOOKEEPER-4259: Allow AdminServer to force https

Initial commit to allow adminServer to only serve HTTPS requests Author: Norbert Kalmar <[email protected]> Reviewers: Mate Szalay-Beko <[email protected]>, Enrico Olivelli <[email protected]> Closes #1652 from nkalmar/ZOOKEEPER-2512
apache · Mar 25, 2021 · 13e02c5 · 13e02c5
1 parent 8625c36
commit 13e02c5
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 7 deletions.
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -1809,6 +1809,15 @@ Both subsystems need to have sufficient amount of threads to achieve peak read t
 
 #### AdminServer configuration
 
+**New in 3.7.1:** The following
+options are used to configure the [AdminServer](#sc_adminserver).
+
+* *admin.forceHttps* :
+  (Java system property: **zookeeper.admin.forceHttps**)
+  Force AdminServer to use SSL, thus allowing only HTTPS traffic.
+  Defaults to disabled.
+  Overwrites **admin.portUnification** settings.
+
 **New in 3.6.0:** The following
 options are used to configure the [AdminServer](#sc_adminserver).
 

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/JettyAdminServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/JettyAdminServer.java
@@ -41,6 +41,7 @@
 import org.eclipse.jetty.server.SecureRequestCustomizer;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.server.SslConnectionFactory;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
 import org.eclipse.jetty.util.security.Constraint;
@@ -86,7 +87,8 @@ public JettyAdminServer() throws AdminServerException, IOException, GeneralSecur
             Integer.getInteger("zookeeper.admin.idleTimeout", DEFAULT_IDLE_TIMEOUT),
             System.getProperty("zookeeper.admin.commandURL", DEFAULT_COMMAND_URL),
             Integer.getInteger("zookeeper.admin.httpVersion", DEFAULT_HTTP_VERSION),
-            Boolean.getBoolean("zookeeper.admin.portUnification"));
+            Boolean.getBoolean("zookeeper.admin.portUnification"),
+            Boolean.getBoolean("zookeeper.admin.forceHttps"));
     }
 
     public JettyAdminServer(
@@ -95,7 +97,8 @@ public JettyAdminServer(
         int timeout,
         String commandUrl,
         int httpVersion,
-        boolean portUnification) throws IOException, GeneralSecurityException {
+        boolean portUnification,
+        boolean forceHttps) throws IOException, GeneralSecurityException {
 
         this.port = port;
         this.idleTimeout = timeout;
@@ -105,7 +108,7 @@ public JettyAdminServer(
         server = new Server();
         ServerConnector connector = null;
 
-        if (!portUnification) {
+        if (!portUnification && !forceHttps) {
             connector = new ServerConnector(server);
         } else {
             SecureRequestCustomizer customizer = new SecureRequestCustomizer();
@@ -141,10 +144,16 @@ public JettyAdminServer(
                 sslContextFactory.setTrustStore(trustStore);
                 sslContextFactory.setTrustStorePassword(certAuthPassword);
 
-                connector = new ServerConnector(
-                    server,
-                    new UnifiedConnectionFactory(sslContextFactory, HttpVersion.fromVersion(httpVersion).asString()),
-                    new HttpConnectionFactory(config));
+                if (forceHttps) {
+                    connector = new ServerConnector(server,
+                            new SslConnectionFactory(sslContextFactory, HttpVersion.fromVersion(httpVersion).asString()),
+                            new HttpConnectionFactory(config));
+                } else {
+                    connector = new ServerConnector(
+                            server,
+                            new UnifiedConnectionFactory(sslContextFactory, HttpVersion.fromVersion(httpVersion).asString()),
+                            new HttpConnectionFactory(config));
+                }
             }
         }
 

diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/JettyAdminServerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/JettyAdminServerTest.java
@@ -20,11 +20,13 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.net.HttpURLConnection;
+import java.net.SocketException;
 import java.net.URL;
 import java.security.GeneralSecurityException;
 import java.security.Security;
@@ -143,6 +145,7 @@ public void cleanUp() {
         System.clearProperty("zookeeper.ssl.quorum.trustStore.password");
         System.clearProperty("zookeeper.ssl.quorum.trustStore.type");
         System.clearProperty("zookeeper.admin.portUnification");
+        System.clearProperty("zookeeper.admin.forceHttps");
     }
 
     /**
@@ -240,6 +243,36 @@ public void testQuorum() throws Exception {
             ClientBase.waitForServerDown("127.0.0.1:" + CLIENT_PORT_QP2, ClientBase.CONNECTION_TIMEOUT));
     }
 
+    @Test
+    public void testForceHttpsPortUnificationEnabled() throws Exception {
+        testForceHttps(true);
+    }
+
+    @Test
+    public void testForceHttpsPortUnificationDisabled() throws Exception {
+        testForceHttps(false);
+    }
+
+    private void testForceHttps(boolean portUnification) throws Exception {
+        System.setProperty("zookeeper.admin.forceHttps", "true");
+        System.setProperty("zookeeper.admin.portUnification", String.valueOf(portUnification));
+        boolean httpsPassed = false;
+
+        JettyAdminServer server = new JettyAdminServer();
+        try {
+            server.start();
+            queryAdminServer(String.format(HTTPS_URL_FORMAT, jettyAdminPort), true);
+            httpsPassed = true;
+            queryAdminServer(String.format(URL_FORMAT, jettyAdminPort), false);
+            fail("http call should have failed since forceHttps=true");
+        } catch (SocketException se) {
+            //good
+        } finally {
+            server.shutdown();
+        }
+        assertTrue(httpsPassed);
+    }
+
     /**
      * Check that we can load the commands page of an AdminServer running at
      * localhost:port. (Note that this should work even if no zk server is set.)