You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by nk...@apache.org on 2021/03/25 10:46:05 UTC

[zookeeper] branch branch-3.6 updated: ZOOKEEPER-4259: Allow AdminServer to force https

This is an automated email from the ASF dual-hosted git repository.

nkalmar pushed a commit to branch branch-3.6
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/branch-3.6 by this push:
     new 13e02c5  ZOOKEEPER-4259: Allow AdminServer to force https
13e02c5 is described below

commit 13e02c5731df6785fba16c71b0f47177a5c40376
Author: Norbert Kalmar <nk...@apache.org>
AuthorDate: Thu Mar 25 09:48:24 2021 +0000

    ZOOKEEPER-4259: Allow AdminServer to force https
    
    Initial commit to allow adminServer to only serve HTTPS requests
    
    Author: Norbert Kalmar <nk...@apache.org>
    
    Reviewers: Mate Szalay-Beko <sy...@apache.org>, Enrico Olivelli <eo...@apache.org>
    
    Closes #1652 from nkalmar/ZOOKEEPER-2512
---
 .../src/main/resources/markdown/zookeeperAdmin.md  |  9 ++++++
 .../zookeeper/server/admin/JettyAdminServer.java   | 23 ++++++++++-----
 .../server/admin/JettyAdminServerTest.java         | 33 ++++++++++++++++++++++
 3 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 1b1b77e..c1c85e3 100644
--- 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
index 6845f23..bfb051b 100644
--- 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.HttpConnectionFactory;
 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 class JettyAdminServer implements AdminServer {
             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 class JettyAdminServer implements AdminServer {
         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 class JettyAdminServer implements AdminServer {
         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 class JettyAdminServer implements AdminServer {
                 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
index 4e77a6b..2042034 100644
--- 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 @@ package org.apache.zookeeper.server.admin;
 
 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 class JettyAdminServerTest extends ZKTestCase {
         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 class JettyAdminServerTest extends ZKTestCase {
             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.)