Revert "Enable ZK access control by default"

2 files changed, 22 insertions, 27 deletions

diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
index 0a1bfdae3a3..1c60587b6e4 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/RestrictedServerCnxnFactory.java
@@ -11,7 +11,6 @@ import java.nio.channels.SocketChannel;
 import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -30,37 +29,33 @@ public class RestrictedServerCnxnFactory extends NIOServerCnxnFactory {
     
     @Override
     protected NIOServerCnxn createConnection(SocketChannel socket, SelectionKey selection) throws IOException {
-        ImmutableSet<String> allowedZooKeeperClients = findAllowedZooKeeperClients();
-        if (allowedZooKeeperClients.isEmpty()) return super.createConnection(socket, selection);
-        
         String remoteHost = ((InetSocketAddress)socket.getRemoteAddress()).getHostName();
-        if ( ! remoteHost.equals("localhost") && ! allowedZooKeeperClients.contains(remoteHost)) {
+
+        Optional<ImmutableSet<String>> allowedZooKeeperClients = ZooKeeperServer.getAllowedClientHostnames();
+        if ( ! allowedZooKeeperClients.isPresent()) {
+            log.fine("Allowing connection to ZooKeeper from " + remoteHost + ", as allowed zooKeeper clients is not set");
+            return super.createConnection(socket, selection); // client checking is not activated
+        }
+
+        if ( ! remoteHost.equals("localhost") && ! allowedZooKeeperClients.get().contains(remoteHost)) {
             String errorMessage = "Rejecting connection to ZooKeeper from " + remoteHost +
-                                  ": This cluster only allow connection from hosts in: " + allowedZooKeeperClients;
-            log.info(errorMessage);
-            throw new IllegalArgumentException(errorMessage); // log and throw as this exception will be suppressed by zk
+                                  ": This cluster only allow connection from hosts in: " + allowedZooKeeperClients.get();
+            if ("true".equals(System.getenv("vespa_zkfacade__restrict"))) {
+                log.info(errorMessage);
+                throw new IllegalArgumentException(errorMessage);
+            }
+            else {
+                log.fine("Would reject if activated: " + errorMessage);
+            }
         }
-        log.fine(() -> "Allowing connection to ZooKeeper from " + remoteHost + ", as it is in " + allowedZooKeeperClients);
+        log.fine("Allowing connection to ZooKeeper from " + remoteHost + ", as it is in " + allowedZooKeeperClients.get());
         return super.createConnection(socket, selection);
     }
 
-    /** Returns the allowed client host names. If the list is empty any host is allowed. */
-    private ImmutableSet<String> findAllowedZooKeeperClients() {
-        // Environment has precedence. Note that this allows setting restrict to "" to turn off client restriction
-        String environmentAllowedZooKeeperClients = System.getenv("vespa_zkfacade__restrict");
-        if (environmentAllowedZooKeeperClients != null) 
-            return ImmutableSet.copyOf(toHostnameSet(environmentAllowedZooKeeperClients));
-
-        // No environment setting -> use static field
-        return ZooKeeperServer.getAllowedClientHostnames();
-    }
-
     private Set<String> toHostnameSet(String commaSeparatedString) {
         Set<String> hostnames = new HashSet<>();
-        for (String hostname : commaSeparatedString.split(",")) {
-            if ( ! hostname.trim().isEmpty())
-                hostnames.add(hostname.trim());
-        }
+        for (String hostname : commaSeparatedString.split(","))
+            hostnames.add(hostname.trim());
         return hostnames;
     }
 
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
index 4eed2173fb5..f6219877f5e 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperServer.java
@@ -28,7 +28,7 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable {
      * This belongs logically to the server instance but must be static to make it accessible
      * from RestrictedServerCnxnFactory, which is created by ZK through reflection.
      */
-    private static volatile ImmutableSet<String> allowedClientHostnames = ImmutableSet.of();
+    private static volatile Optional<ImmutableSet<String>> allowedClientHostnames = Optional.empty();
 
     private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(ZooKeeperServer.class.getName());
     private static final String ZOOKEEPER_JMX_LOG4J_DISABLE = "zookeeper.jmx.log4j.disable";
@@ -56,11 +56,11 @@ public class ZooKeeperServer extends AbstractComponent implements Runnable {
     
     /** Restrict access to this ZooKeeper server to the given client hosts */
     public static void setAllowedClientHostnames(Collection<String> hostnames) {
-        allowedClientHostnames = ImmutableSet.copyOf(hostnames);
+        allowedClientHostnames = Optional.of(ImmutableSet.copyOf(hostnames));
     }
     
     /** Returns the hosts which are allowed to access this ZooKeeper server, or empty to allow access from anywhere */
-    public static ImmutableSet<String> getAllowedClientHostnames() { return allowedClientHostnames; }
+    public static Optional<ImmutableSet<String>> getAllowedClientHostnames() { return allowedClientHostnames; }
     
     private void writeConfigToDisk(ZookeeperServerConfig config) {
        String cfg = transformConfigToString(config);