zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From h...@apache.org
Subject [zookeeper] branch master updated: ZOOKEEPER-3561: Generalize target authentication scheme for ZooKeeper authentication enforcement.
Date Wed, 21 Oct 2020 02:41:50 GMT
This is an automated email from the ASF dual-hosted git repository.

hanm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 1af3dcc  ZOOKEEPER-3561: Generalize target authentication scheme for ZooKeeper authentication
enforcement.
1af3dcc is described below

commit 1af3dcc633d4829864da74ca6b030428448fcc16
Author: Mohammad Arshad <arshad@apache.org>
AuthorDate: Tue Oct 20 19:41:38 2020 -0700

    ZOOKEEPER-3561: Generalize target authentication scheme for ZooKeeper authentication enforcement.
    
    Added enforce.auth.enabled and enforce.auth.scheme to enforce any authentication scheme.
    
    Author: Mohammad Arshad <arshad@apache.org>
    
    Reviewers: Michael Han <hanm@apache.org>, Damien Diederen <dd@crosstwine.com>,
Enrico Olivelli <eolivelli@gmail.com>, Andor Molnár <andor@apache.org>
    
    Closes #1500 from arshadmohammad/ZOOKEEPER-3561-master
---
 .../zookeeper-client-c/include/zookeeper.h         |   2 +-
 .../zookeeper-client-c/tests/zkServer.sh           |   2 +-
 .../src/main/resources/markdown/zookeeperAdmin.md  |  34 ++-
 .../java/org/apache/zookeeper/KeeperException.java |   5 +-
 .../zookeeper/server/AuthenticationHelper.java     | 141 +++++++++++
 .../apache/zookeeper/server/ZooKeeperServer.java   |  29 +--
 .../zookeeper/EnforceAuthenticationTest.java       | 266 +++++++++++++++++++++
 .../server/quorum/QuorumPeerTestBase.java          |  24 +-
 .../java/org/apache/zookeeper/test/ClientBase.java |  14 +-
 .../test/SaslAuthRequiredFailNoSASLTest.java       |  16 +-
 10 files changed, 493 insertions(+), 40 deletions(-)

diff --git a/zookeeper-client/zookeeper-client-c/include/zookeeper.h b/zookeeper-client/zookeeper-client-c/include/zookeeper.h
index 8bd1d95..467a857 100644
--- a/zookeeper-client/zookeeper-client-c/include/zookeeper.h
+++ b/zookeeper-client/zookeeper-client-c/include/zookeeper.h
@@ -140,7 +140,7 @@ enum ZOO_ERRORS {
   ZEPHEMERALONLOCALSESSION = -120, /*!< Attempt to create ephemeral node on a local session
*/
   ZNOWATCHER = -121, /*!< The watcher couldn't be found */
   ZRECONFIGDISABLED = -123, /*!< Attempts to perform a reconfiguration operation when
reconfiguration feature is disabled */
-  ZSESSIONCLOSEDREQUIRESASLAUTH = -124, /*!< The session has been closed by server because
server requires client to do SASL authentication, but client is not configured with SASL authentication
or configuted with SASL but failed (i.e. wrong credential used.). */
+  ZSESSIONCLOSEDREQUIRESASLAUTH = -124, /*!< The session has been closed by server because
server requires client to do authentication via configured authentication scheme at server,
but client is not configured with required authentication scheme or configured but failed
(i.e. wrong credential used.). */
   ZTHROTTLEDOP = -127 /*!< Operation was throttled and not executed at all. please, retry!
*/
 
   /* when adding/changing values here also update zerror(int) to return correct error message
*/
diff --git a/zookeeper-client/zookeeper-client-c/tests/zkServer.sh b/zookeeper-client/zookeeper-client-c/tests/zkServer.sh
index 99b716a..f98b405 100755
--- a/zookeeper-client/zookeeper-client-c/tests/zkServer.sh
+++ b/zookeeper-client/zookeeper-client-c/tests/zkServer.sh
@@ -128,9 +128,9 @@ PROPERTIES="$EXTRA_JVM_ARGS -Dzookeeper.extendedTypesEnabled=true -Dznode.contai
 if [ "x$1" == "xstartRequireSASLAuth" ]
 then
     PROPERTIES="-Dzookeeper.sessionRequireClientSASLAuth=true $PROPERTIES"
+    PROPERTIES="$PROPERTIES -Dzookeeper.authProvider.1=org.apache.zookeeper.server.auth.SASLAuthenticationProvider"
     if [ "x$2" != "x" ]
     then
-        PROPERTIES="$PROPERTIES -Dzookeeper.authProvider.1=org.apache.zookeeper.server.auth.SASLAuthenticationProvider"
         PROPERTIES="$PROPERTIES -Djava.security.auth.login.config=$2"
     fi
     if [ "x$3" != "x" ]
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 2150c5a..a3a1c6c 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -1498,8 +1498,8 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
     If the credential is not in the list, the connection request will be refused.
     This prevents a client accidentally connecting to a wrong ensemble.
 
-* *zookeeper.sessionRequireClientSASLAuth* :
-    (Java system property only: **zookeeper.sessionRequireClientSASLAuth**)
+* *sessionRequireClientSASLAuth* :
+    (Java system property: **zookeeper.sessionRequireClientSASLAuth**)
     **New in 3.6.0:**
     When set to **true**, ZooKeeper server will only accept connections and requests from
clients
     that have authenticated with server via SASL. Clients that are not configured with SASL
@@ -1508,13 +1508,41 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
     in such case, both Java and C client will close the session with server thereafter,
     without further attempts on retrying to reconnect.
 
+    This configuration is short hand for **enforce.auth.enabled=true** and **enforce.auth.scheme=sasl**
+
     By default, this feature is disabled. Users who would like to opt-in can enable the feature
-    by setting **zookeeper.sessionRequireClientSASLAuth** to **true**.
+    by setting **sessionRequireClientSASLAuth** to **true**.
 
     This feature overrules the <emphasis role="bold">zookeeper.allowSaslFailedClients</emphasis>
option, so even if server is
     configured to allow clients that fail SASL authentication to login, client will not be
able to
     establish a session with server if this feature is enabled.
 
+* *enforce.auth.enabled* :
+    (Java system property : **zookeeper.enforce.auth.enabled**)
+    **New in 3.7.0:**
+    When set to **true**, ZooKeeper server will only accept connections and requests from
clients
+    that have authenticated with server via configured auth scheme. Authentication schemes
+    can be configured using property enforce.auth.schemes. Clients that are not
+    configured with the any of the auth scheme configured at server or configured but failed
authentication (i.e. with invalid credential)
+    will not be able to establish a session with server. A typed error code (-124) will be
delivered
+    in such case, both Java and C client will close the session with server thereafter,
+    without further attempts on retrying to reconnect.
+
+    By default, this feature is disabled. Users who would like to opt-in can enable the feature
+    by setting **enforce.auth.enabled** to **true**.
+
+    When **enforce.auth.enabled=true** and **enforce.auth.schemes=sasl** then 
+    <emphasis role="bold">zookeeper.allowSaslFailedClients</emphasis> configuration
is overruled. So even if server is
+    configured to allow clients that fail SASL authentication to login, client will not be
able to
+    establish a session with server if this feature is enabled with sasl as authentication
scheme.
+
+* *enforce.auth.schemes* :
+    (Java system property : **zookeeper.enforce.auth.schemes**)
+    **New in 3.7.0:**
+    Comma separated list of authentication schemes. Clients must be authenticated with at
least one
+    authentication scheme before doing any zookeeper operations. 
+    This property is used only when **enforce.auth.enabled** is to **true**.
+
 * *sslQuorum* :
     (Java system property: **zookeeper.sslQuorum**)
     **New in 3.5.5:**
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java b/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
index c8b33b7..9469d64 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
@@ -403,8 +403,9 @@ public abstract class KeeperException extends Exception {
         REQUESTTIMEOUT(-122),
         /** Attempts to perform a reconfiguration operation when reconfiguration feature
is disabled. */
         RECONFIGDISABLED(-123),
-        /** The session has been closed by server because server requires client to do SASL
authentication,
-         *  but client is not configured with SASL authentication or configuted with SASL
but failed
+        /** The session has been closed by server because server requires client to do authentication
+         *  with configured authentication scheme at the server, but client is not configured
with
+         *  required  authentication scheme or configured but authentication failed
          *  (i.e. wrong credential used.). */
         SESSIONCLOSEDREQUIRESASLAUTH(-124),
         /** Operation was throttled and not executed at all. This error code indicates that
zookeeper server
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/AuthenticationHelper.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/AuthenticationHelper.java
new file mode 100644
index 0000000..61aebb3
--- /dev/null
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/AuthenticationHelper.java
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.server;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.proto.ReplyHeader;
+import org.apache.zookeeper.server.auth.ProviderRegistry;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Contains helper methods to enforce authentication
+ */
+public class AuthenticationHelper {
+    private static final Logger LOG = LoggerFactory.getLogger(AuthenticationHelper.class);
+
+    public static final String ENFORCE_AUTH_ENABLED = "zookeeper.enforce.auth.enabled";
+    public static final String ENFORCE_AUTH_SCHEMES = "zookeeper.enforce.auth.schemes";
+    public static final String SESSION_REQUIRE_CLIENT_SASL_AUTH =
+        "zookeeper.sessionRequireClientSASLAuth";
+    public static final String SASL_AUTH_SCHEME = "sasl";
+
+    private boolean enforceAuthEnabled;
+    private List<String> enforceAuthSchemes = new ArrayList<>();
+    private boolean saslAuthRequired;
+
+    public AuthenticationHelper() {
+        initConfigurations();
+    }
+
+    private void initConfigurations() {
+        if (Boolean.parseBoolean(System.getProperty(SESSION_REQUIRE_CLIENT_SASL_AUTH, "false")))
{
+            enforceAuthEnabled = true;
+            enforceAuthSchemes.add(SASL_AUTH_SCHEME);
+        } else {
+            enforceAuthEnabled =
+                Boolean.parseBoolean(System.getProperty(ENFORCE_AUTH_ENABLED, "false"));
+            String enforceAuthSchemesProp = System.getProperty(ENFORCE_AUTH_SCHEMES);
+            if (enforceAuthSchemesProp != null) {
+                Arrays.stream(enforceAuthSchemesProp.split(",")).forEach(s -> {
+                    enforceAuthSchemes.add(s.trim());
+                });
+            }
+        }
+        LOG.info("{} = {}", ENFORCE_AUTH_ENABLED, enforceAuthEnabled);
+        LOG.info("{} = {}", ENFORCE_AUTH_SCHEMES, enforceAuthSchemes);
+        validateConfiguredProperties();
+        saslAuthRequired = enforceAuthEnabled && enforceAuthSchemes.contains(SASL_AUTH_SCHEME);
+    }
+
+    private void validateConfiguredProperties() {
+        if (enforceAuthEnabled) {
+            if (enforceAuthSchemes.isEmpty()) {
+                String msg =
+                    ENFORCE_AUTH_ENABLED + " is true " + ENFORCE_AUTH_SCHEMES + " must be
 "
+                        + "configured.";
+                LOG.error(msg);
+                throw new IllegalArgumentException(msg);
+            }
+            enforceAuthSchemes.forEach(scheme -> {
+                if (ProviderRegistry.getProvider(scheme) == null) {
+                    String msg = "Authentication scheme " + scheme + " is not available.";
+                    LOG.error(msg);
+                    throw new IllegalArgumentException(msg);
+                }
+            });
+        }
+    }
+
+    /**
+     * Checks if connection is authenticated or not.
+     *
+     * @param cnxn server connection
+     * @return boolean
+     */
+    private boolean isCnxnAuthenticated(ServerCnxn cnxn) {
+        for (Id id : cnxn.getAuthInfo()) {
+            if (enforceAuthSchemes.contains(id.getScheme())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    public boolean isEnforceAuthEnabled() {
+        return enforceAuthEnabled;
+    }
+
+    /**
+     * Returns true when authentication enforcement was success otherwise returns false
+     * also closes the connection
+     *
+     * @param connection server connection
+     * @param xid        current operation xid
+     * @return true when authentication enforcement is success otherwise false
+     */
+    public boolean enforceAuthentication(ServerCnxn connection, int xid) throws IOException
{
+        if (isEnforceAuthEnabled() && !isCnxnAuthenticated(connection)) {
+            //Un authenticated connection, lets inform user with response and then close
the session
+            LOG.error("Client authentication scheme(s) {} does not match with any of the
expected "
+                    + "authentication scheme {}, closing session.", getAuthSchemes(connection),
+                enforceAuthSchemes);
+            ReplyHeader replyHeader = new ReplyHeader(xid, 0,
+                KeeperException.Code.SESSIONCLOSEDREQUIRESASLAUTH.intValue());
+            connection.sendResponse(replyHeader, null, "response");
+            connection.sendCloseSession();
+            connection.disableRecv();
+            return false;
+        }
+        return true;
+    }
+
+    private List<String> getAuthSchemes(ServerCnxn connection) {
+        return connection.getAuthInfo().stream().map(Id::getScheme).collect(Collectors.toList());
+    }
+
+    public boolean isSaslAuthRequired() {
+        return saslAuthRequired;
+    }
+}
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
index b016724..00550be 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
@@ -107,9 +107,6 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
     static final boolean skipACL;
 
     public static final String ALLOW_SASL_FAILED_CLIENTS = "zookeeper.allowSaslFailedClients";
-    public static final String SESSION_REQUIRE_CLIENT_SASL_AUTH = "zookeeper.sessionRequireClientSASLAuth";
-    public static final String SASL_AUTH_SCHEME = "sasl";
-
     public static final String ZOOKEEPER_DIGEST_ENABLED = "zookeeper.digest.enabled";
     private static boolean digestEnabled;
 
@@ -284,6 +281,8 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
 
     private final AtomicInteger currentLargeRequestBytes = new AtomicInteger(0);
 
+    private AuthenticationHelper authHelper;
+
     void removeCnxn(ServerCnxn cnxn) {
         zkDb.removeCnxn(cnxn);
     }
@@ -298,6 +297,7 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
         listener = new ZooKeeperServerListenerImpl(this);
         serverStats = new ServerStats(this);
         this.requestPathMetricsCollector = new RequestPathMetricsCollector();
+        this.authHelper = new AuthenticationHelper();
     }
 
     /**
@@ -339,6 +339,8 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
 
         this.initLargeRequestThrottlingSettings();
 
+        this.authHelper = new AuthenticationHelper();
+
         LOG.info(
             "Created server with"
                 + " tickTime {}"
@@ -1623,11 +1625,10 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
         } else if (h.getType() == OpCode.sasl) {
             processSasl(incomingBuffer, cnxn, h);
         } else {
-            if (shouldRequireClientSaslAuth() && !hasCnxSASLAuthenticated(cnxn))
{
-                ReplyHeader replyHeader = new ReplyHeader(h.getXid(), 0, Code.SESSIONCLOSEDREQUIRESASLAUTH.intValue());
-                cnxn.sendResponse(replyHeader, null, "response");
-                cnxn.sendCloseSession();
-                cnxn.disableRecv();
+            if (!authHelper.enforceAuthentication(cnxn, h.getXid())) {
+                // Authentication enforcement is failed
+                // Already sent response to user about failure and closed the session, lets
return
+                return;
             } else {
                 Request si = new Request(cnxn, cnxn.getSessionId(), h.getXid(), h.getType(),
incomingBuffer, cnxn.getAuthInfo());
                 int length = incomingBuffer.limit();
@@ -1646,14 +1647,6 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
         return Boolean.getBoolean(ALLOW_SASL_FAILED_CLIENTS);
     }
 
-    private static boolean shouldRequireClientSaslAuth() {
-        return Boolean.getBoolean(SESSION_REQUIRE_CLIENT_SASL_AUTH);
-    }
-
-    private boolean hasCnxSASLAuthenticated(ServerCnxn cnxn) {
-        return cnxn.getAuthInfo().stream().anyMatch(id -> id.getScheme().equals(SASL_AUTH_SCHEME));
-    }
-
     private void processSasl(ByteBuffer incomingBuffer, ServerCnxn cnxn, RequestHeader requestHeader)
throws IOException {
         LOG.debug("Responding to client SASL token.");
         GetSASLRequest clientTokenRecord = new GetSASLRequest();
@@ -1679,11 +1672,11 @@ public class ZooKeeperServer implements SessionExpirer, ServerStats.Provider
{
                 }
             } catch (SaslException e) {
                 LOG.warn("Client {} failed to SASL authenticate: {}", cnxn.getRemoteSocketAddress(),
e);
-                if (shouldAllowSaslFailedClientsConnect() && !shouldRequireClientSaslAuth())
{
+                if (shouldAllowSaslFailedClientsConnect() && !authHelper.isSaslAuthRequired())
{
                     LOG.warn("Maintaining client connection despite SASL authentication failure.");
                 } else {
                     int error;
-                    if (shouldRequireClientSaslAuth()) {
+                    if (authHelper.isSaslAuthRequired()) {
                         LOG.warn(
                             "Closing client connection due to server requires client SASL
authenticaiton,"
                                 + "but client SASL authentication has failed, or client is
not configured with SASL "
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/EnforceAuthenticationTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/EnforceAuthenticationTest.java
new file mode 100644
index 0000000..ef66562
--- /dev/null
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/EnforceAuthenticationTest.java
@@ -0,0 +1,266 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper;
+
+import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.server.AuthenticationHelper;
+import org.apache.zookeeper.server.ServerConfig;
+import org.apache.zookeeper.server.ZooKeeperServerMain;
+import org.apache.zookeeper.server.quorum.QuorumPeerTestBase;
+import org.apache.zookeeper.test.ClientBase;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class EnforceAuthenticationTest extends QuorumPeerTestBase {
+    protected static final Logger LOG = LoggerFactory.getLogger(EnforceAuthenticationTest.class);
+    private Servers servers;
+    private int clientPort;
+
+    @Before
+    public void setUp() {
+        System.setProperty("zookeeper.admin.enableServer", "false");
+        System.setProperty("zookeeper.4lw.commands.whitelist", "*");
+        System.clearProperty(AuthenticationHelper.ENFORCE_AUTH_ENABLED);
+        System.clearProperty(AuthenticationHelper.ENFORCE_AUTH_SCHEMES);
+    }
+
+    @After
+    public void tearDown() throws InterruptedException {
+        if (servers != null) {
+            servers.shutDownAllServers();
+        }
+        System.clearProperty(AuthenticationHelper.ENFORCE_AUTH_ENABLED);
+        System.clearProperty(AuthenticationHelper.ENFORCE_AUTH_SCHEMES);
+    }
+
+    /**
+     * When AuthenticationHelper.ENFORCE_AUTH_ENABLED is not set or set to false, behaviour
should
+     * be same as the old ie. clients without authentication are allowed to do operations
+     */
+    @Test
+    public void testEnforceAuthenticationOldBehaviour() throws Exception {
+        Map<String, String> prop = new HashMap<>();
+        startServer(prop);
+        testEnforceAuthOldBehaviour(false);
+    }
+
+    @Test
+    public void testEnforceAuthenticationOldBehaviourWithNetty() throws Exception {
+        Map<String, String> prop = new HashMap<>();
+        //setting property false should give the same behaviour as when property is not set
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), "false");
+        prop.put("serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory");
+        startServer(prop);
+        testEnforceAuthOldBehaviour(true);
+    }
+
+    private void testEnforceAuthOldBehaviour(boolean netty) throws Exception {
+        ZKClientConfig config = new ZKClientConfig();
+        if (netty) {
+            config.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
+                "org.apache.zookeeper.ClientCnxnSocketNetty");
+        }
+        ZooKeeper client = ClientBase
+            .createZKClient("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT, CONNECTION_TIMEOUT,
+                config);
+        String path = "/defaultAuth" + System.currentTimeMillis();
+        String data = "someData";
+        client.create(path, data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+        byte[] data1 = client.getData(path, false, null);
+        assertEquals(data, new String(data1));
+        client.close();
+    }
+
+    /**
+     * Server start should fail when ZooKeeperServer.ENFORCE_AUTH_ENABLED is set to true
 but
+     * AuthenticationHelper.ENFORCE_AUTH_SCHEME is not configured
+     */
+    @Test
+    public void testServerStartShouldFailWhenEnforceAuthSchemeIsNotConfigured() throws Exception
{
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), "true");
+        testServerCannotStart(prop);
+    }
+
+    /**
+     * Server start should fail when AuthenticationHelper.ENFORCE_AUTH_ENABLED is set to
true,
+     * AuthenticationHelper.ENFORCE_AUTH_SCHEME is configured but authentication provider
is not
+     * configured.
+     */
+    @Test
+    public void testServerStartShouldFailWhenAuthProviderIsNotConfigured() throws Exception
{
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), "true");
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_SCHEMES), "sasl");
+        testServerCannotStart(prop);
+    }
+
+    private void testServerCannotStart(Map<String, String> prop)
+        throws Exception {
+        File confFile = getConfFile(prop);
+        ServerConfig config = new ServerConfig();
+        config.parse(confFile.toString());
+        ZooKeeperServerMain serverMain = new ZooKeeperServerMain();
+        try {
+            serverMain.runFromConfig(config);
+            fail("IllegalArgumentException is expected.");
+        } catch (IllegalArgumentException e) {
+            //do nothing
+        }
+    }
+
+    @Test
+    public void testEnforceAuthenticationNewBehaviour() throws Exception {
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), "true");
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_SCHEMES), "digest");
+        //digest auth provider is started by default, so no need to
+        //prop.put("authProvider.1", DigestAuthenticationProvider.class.getName());
+        startServer(prop);
+        testEnforceAuthNewBehaviour(false);
+    }
+
+    @Test
+    public void testEnforceAuthenticationNewBehaviourWithNetty() throws Exception {
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), "true");
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_SCHEMES), "digest");
+        prop.put("serverCnxnFactory", "org.apache.zookeeper.server.NettyServerCnxnFactory");
+        startServer(prop);
+        testEnforceAuthNewBehaviour(true);
+    }
+
+    /**
+     * Client operations are allowed only after the authentication is done
+     */
+    private void testEnforceAuthNewBehaviour(boolean netty) throws Exception {
+        ZKClientConfig config = new ZKClientConfig();
+        if (netty) {
+            config.setProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET,
+                "org.apache.zookeeper.ClientCnxnSocketNetty");
+        }
+        CountDownLatch countDownLatch = new CountDownLatch(1);
+        ZooKeeper client =
+            new ZooKeeper("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT, getWatcher(countDownLatch),
+                config);
+        countDownLatch.await();
+        String path = "/newAuth" + System.currentTimeMillis();
+        String data = "someData";
+
+        //try without authentication
+        try {
+            client.create(path, data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+            fail("SessionClosedRequireAuthException is expected.");
+        } catch (KeeperException.SessionClosedRequireAuthException e) {
+            //do nothing
+        }
+        client.close();
+        countDownLatch = new CountDownLatch(1);
+        client =
+            new ZooKeeper("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT, getWatcher(countDownLatch),
+                config);
+        countDownLatch.await();
+
+        // try operations after authentication
+        String idPassword = "user1:pass1";
+        client.addAuthInfo("digest", idPassword.getBytes());
+        client.create(path, data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+        byte[] data1 = client.getData(path, false, null);
+        assertEquals(data, new String(data1));
+        client.close();
+    }
+
+    @Test
+    public void testEnforceAuthenticationWithMultipleAuthSchemes() throws Exception {
+        Map<String, String> prop = new HashMap<>();
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_ENABLED), "true");
+        prop.put(removeZooKeeper(AuthenticationHelper.ENFORCE_AUTH_SCHEMES), "digest,ip");
+        startServer(prop);
+        ZKClientConfig config = new ZKClientConfig();
+        CountDownLatch countDownLatch = new CountDownLatch(1);
+        ZooKeeper client =
+            new ZooKeeper("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT, getWatcher(countDownLatch),
+                config);
+        countDownLatch.await();
+        // try operation without adding auth info, it should be success as ip auth info is
+        // added automatically by server
+        String path = "/newAuth" + System.currentTimeMillis();
+        String data = "someData";
+        client.create(path, data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+        byte[] data1 = client.getData(path, false, null);
+        assertEquals(data, new String(data1));
+        client.close();
+    }
+
+    private String removeZooKeeper(String prop) {
+        return prop.replace("zookeeper.", "");
+    }
+
+    private File getConfFile(Map<String, String> additionalProp) throws IOException
{
+        clientPort = PortAssignment.unique();
+        StringBuilder sb = new StringBuilder();
+        sb.append("standaloneEnabled=true" + "\n");
+        if (null != additionalProp) {
+            for (Map.Entry<String, String> entry : additionalProp.entrySet()) {
+                sb.append(entry.getKey());
+                sb.append("=");
+                sb.append(entry.getValue());
+                sb.append("\n");
+            }
+        }
+        String currentQuorumCfgSection = sb.toString();
+        return new MainThread(1, clientPort, currentQuorumCfgSection, false).getConfFile();
+    }
+
+    private void startServer(Map<String, String> additionalProp) throws Exception {
+        additionalProp.put("standaloneEnabled", "true");
+        servers = LaunchServers(1, additionalProp);
+        clientPort = servers.clientPorts[0];
+    }
+
+    private Watcher getWatcher(CountDownLatch countDownLatch) {
+        return event -> {
+            Event.EventType type = event.getType();
+            if (type == Event.EventType.None) {
+                Event.KeeperState state = event.getState();
+                if (state == Event.KeeperState.SyncConnected) {
+                    LOG.info("Event.KeeperState.SyncConnected");
+                    countDownLatch.countDown();
+                } else if (state == Event.KeeperState.Expired) {
+                    LOG.info("Event.KeeperState.Expired");
+                } else if (state == Event.KeeperState.Disconnected) {
+                    LOG.info("Event.KeeperState.Disconnected");
+                } else if (state == Event.KeeperState.AuthFailed) {
+                    LOG.info("Event.KeeperState.AuthFailed");
+                }
+            }
+        };
+    }
+}
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
index 953f4ab..2a116dc 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java
@@ -406,7 +406,7 @@ public class QuorumPeerTestBase extends ZKTestCase implements Watcher
{
 
         MainThread[] mt;
         ZooKeeper[] zk;
-        int[] clientPorts;
+        public int[] clientPorts;
 
         public void shutDownAllServers() throws InterruptedException {
             for (MainThread t : mt) {
@@ -470,23 +470,35 @@ public class QuorumPeerTestBase extends ZKTestCase implements Watcher
{
     }
 
     protected Servers LaunchServers(int numServers) throws IOException, InterruptedException
{
-        return LaunchServers(numServers, null);
+        return LaunchServers(numServers, (Integer) null);
+    }
+
+    protected Servers LaunchServers(int numServers, Map<String, String> otherConfigs)
+        throws IOException, InterruptedException {
+        return LaunchServers(numServers, 0, null, otherConfigs);
     }
 
     protected Servers LaunchServers(int numServers, Integer tickTime) throws IOException,
InterruptedException {
         return LaunchServers(numServers, 0, tickTime);
     }
 
+    protected Servers LaunchServers(int numServers, int numObservers, Integer tickTime)
+        throws IOException, InterruptedException {
+        return LaunchServers(numServers, numObservers, tickTime, new HashMap<>());
+    }
+
     /** * This is a helper function for launching a set of servers
      *
      * @param numServers the number of participant servers
-     * @param numObserver the number of observer servers
+     * @param numObservers the number of observer servers
      * @param tickTime A ticktime to pass to MainThread
+     * @param otherConfigs any zoo.cfg configuration
      * @return
      * @throws IOException
      * @throws InterruptedException
      */
-    protected Servers LaunchServers(int numServers, int numObservers, Integer tickTime) throws
IOException, InterruptedException {
+    protected Servers LaunchServers(int numServers, int numObservers, Integer tickTime,
+        Map<String, String> otherConfigs) throws IOException, InterruptedException
{
         int SERVER_COUNT = numServers + numObservers;
         QuorumPeerMainTest.Servers svrs = new QuorumPeerMainTest.Servers();
         svrs.clientPorts = new int[SERVER_COUNT];
@@ -504,9 +516,9 @@ public class QuorumPeerTestBase extends ZKTestCase implements Watcher
{
         svrs.zk = new ZooKeeper[SERVER_COUNT];
         for (int i = 0; i < SERVER_COUNT; i++) {
             if (tickTime != null) {
-                svrs.mt[i] = new MainThread(i, svrs.clientPorts[i], quorumCfgSection, new
HashMap<String, String>(), tickTime);
+                svrs.mt[i] = new MainThread(i, svrs.clientPorts[i], quorumCfgSection, otherConfigs,
tickTime);
             } else {
-                svrs.mt[i] = new MainThread(i, svrs.clientPorts[i], quorumCfgSection);
+                svrs.mt[i] = new MainThread(i, svrs.clientPorts[i], quorumCfgSection, otherConfigs);
             }
             svrs.mt[i].start();
             svrs.restartClient(i, this);
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientBase.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientBase.java
index 5904a37..c628e4f 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientBase.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ClientBase.java
@@ -51,6 +51,7 @@ import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.Watcher.Event.KeeperState;
 import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.client.ZKClientConfig;
 import org.apache.zookeeper.common.IOUtils;
 import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.common.X509Exception.SSLContextException;
@@ -351,6 +352,9 @@ public abstract class ClientBase extends ZKTestCase {
     }
 
     static File createTmpDir(File parentDir, boolean createInitFile) throws IOException {
+        if (!parentDir.exists()) {
+            parentDir.mkdir();
+        }
         File tmpFile = File.createTempFile("test", ".junit", parentDir);
         // don't delete tmpFile - this ensures we don't attempt to create
         // a tmpDir with a duplicate name
@@ -738,9 +742,15 @@ public abstract class ClientBase extends ZKTestCase {
         return createZKClient(cxnString, sessionTimeout, CONNECTION_TIMEOUT);
     }
 
-    public static ZooKeeper createZKClient(String cxnString, int sessionTimeout, long connectionTimeout)
throws IOException {
+    public static ZooKeeper createZKClient(String cxnString, int sessionTimeout,
+        long connectionTimeout) throws IOException {
+        return createZKClient(cxnString, sessionTimeout, connectionTimeout, new ZKClientConfig());
+    }
+
+    public static ZooKeeper createZKClient(String cxnString, int sessionTimeout,
+        long connectionTimeout, ZKClientConfig config) throws IOException {
         CountdownWatcher watcher = new CountdownWatcher();
-        ZooKeeper zk = new ZooKeeper(cxnString, sessionTimeout, watcher);
+        ZooKeeper zk = new ZooKeeper(cxnString, sessionTimeout, watcher, config);
         try {
             watcher.waitForConnected(connectionTimeout);
         } catch (InterruptedException | TimeoutException e) {
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthRequiredFailNoSASLTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthRequiredFailNoSASLTest.java
index 51dbbb5..f5b99b9 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthRequiredFailNoSASLTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SaslAuthRequiredFailNoSASLTest.java
@@ -24,20 +24,22 @@ import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.ZooDefs.Ids;
 import org.apache.zookeeper.ZooKeeper;
-import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
 public class SaslAuthRequiredFailNoSASLTest extends ClientBase {
 
-    @BeforeEach
-    public void setup() {
+    @BeforeAll
+    public static void setup() {
         System.setProperty(SaslTestUtil.requireSASLAuthProperty, "true");
+        System.setProperty(SaslTestUtil.authProviderProperty, SaslTestUtil.authProvider);
     }
 
-    @AfterEach
-    public void tearDown() throws Exception {
+    @AfterAll
+    public static void clearSetup() {
         System.clearProperty(SaslTestUtil.requireSASLAuthProperty);
+        System.clearProperty(SaslTestUtil.authProviderProperty);
     }
 
     @Test
@@ -46,7 +48,7 @@ public class SaslAuthRequiredFailNoSASLTest extends ClientBase {
         CountdownWatcher watcher = new CountdownWatcher();
         try {
             zk = createClient(watcher);
-            zk.create("/foo", null, Ids.CREATOR_ALL_ACL, CreateMode.PERSISTENT);
+            zk.create("/foo", null, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
             fail("Client is not configured with SASL authentication, so zk.create operation
should fail.");
         } catch (KeeperException e) {
             assertTrue(e.code() == KeeperException.Code.SESSIONCLOSEDREQUIRESASLAUTH);


Mime
View raw message