jhuan31 closed pull request #768: ZOOKEEPER-3239: Adding EnsembleAuthProvider
to verify the ensemble name
URL: https://github.com/apache/zookeeper/pull/768
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DumbWatcher.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DumbWatcher.java
index f384d7c5c7..4a9771f0a4 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DumbWatcher.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DumbWatcher.java
@@ -55,7 +55,7 @@ public void process(WatchedEvent event) { }
int getSessionTimeout() { return 0; }
@Override
- void close() { }
+ public void close() { }
@Override
public void sendResponse(ReplyHeader h, Record r, String tag) throws
IOException { }
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java
index 8e145cbeb1..da40a26ef3 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxn.java
@@ -101,7 +101,7 @@ public void decrOutstandingAndCheckThrottle(ReplyHeader h) {
}
}
- abstract void close();
+ public abstract void close();
public void sendResponse(ReplyHeader h, Record r, String tag) throws
IOException {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
index c5d82deebb..6a0b45f50f 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
@@ -67,7 +67,22 @@
SNAP_COUNT(new SimpleCounter("snap_count")),
COMMIT_COUNT(new SimpleCounter("commit_count")),
CONNECTION_REQUEST_COUNT(new SimpleCounter("connection_request_count")),
- BYTES_RECEIVED_COUNT(new SimpleCounter("bytes_received_count"));
+ BYTES_RECEIVED_COUNT(new SimpleCounter("bytes_received_count")),
+
+ /*
+ * Number of successful matches of expected ensemble name in
EnsembleAuthenticationProvider.
+ */
+ ENSEMBLE_AUTH_SUCCESS(new SimpleCounter("ensemble_auth_success")),
+
+ /*
+ * Number of unsuccessful matches of expected ensemble name in
EnsembleAuthenticationProvider.
+ */
+ ENSEMBLE_AUTH_FAIL(new SimpleCounter("ensemble_auth_fail")),
+
+ /*
+ * Number of client auth requests with no ensemble set in
EnsembleAuthenticationProvider.
+ */
+ ENSEMBLE_AUTH_SKIP(new SimpleCounter("ensemble_auth_skip"));
private final Metric metric;
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 20ab023ec5..0670359359 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
@@ -1136,6 +1136,8 @@ public void processPacket(ServerCnxn cnxn, ByteBuffer
incomingBuffer) throws IOE
Code authReturn = KeeperException.Code.AUTHFAILED;
if(ap != null) {
try {
+ // handleAuthentication may close the connection, to allow
the client to choose
+ // a different server to connect to.
authReturn = ap.handleAuthentication(new
ServerAuthenticationProvider.ServerObjs(this, cnxn), authPacket.getAuth());
} catch(RuntimeException e) {
LOG.warn("Caught runtime exception from
AuthenticationProvider: " + scheme + " due to " + e);
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java
new file mode 100644
index 0000000000..18ab41deb8
--- /dev/null
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/EnsembleAuthenticationProvider.java
@@ -0,0 +1,137 @@
+/**
+ * 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.auth;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.jmx.MBeanRegistry;
+import org.apache.zookeeper.server.ServerCnxn;
+import org.apache.zookeeper.server.ServerMetrics;
+import org.apache.zookeeper.server.ZooKeeperServer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.zookeeper.jmx.ZKMBeanInfo;
+
+import javax.management.JMException;
+
+/**
+ * this is not a true AuthenticationProvider in the strict sense. it does
+ * handle add auth requests, but rather than authenticate the client, it checks
+ * to make sure that the ensemble name the client intends to connect to
+ * matches the name that the server thinks it belongs to. if the name does not
match,
+ * this provider will close the connection.
+ */
+/* TODO: this is ready to be an MBean, but it's unclear how to wire it up */
+public class EnsembleAuthenticationProvider implements AuthenticationProvider {
+ private static final Logger LOG = LoggerFactory
+ .getLogger(EnsembleAuthenticationProvider.class);
+
+ public static final String ENSEMBLE_PROPERTY =
"zookeeper.ensembleAuthName";
+ private static final int MIN_LOGGING_INTERVAL_MS = 1000;
+ private Set<String> ensembleNames;
+
+ public EnsembleAuthenticationProvider() {
+ String namesCSV = System.getProperty(ENSEMBLE_PROPERTY);
+ if (namesCSV != null) {
+ LOG.info("Set expected ensemble names to {}", namesCSV);
+ setEnsembleNames(namesCSV);
+ }
+ }
+
+ public void setEnsembleNames(String namesCSV) {
+ ensembleNames = new HashSet<String>();
+ for (String name: namesCSV.split(",")) {
+ ensembleNames.add(name.trim());
+ }
+ }
+
+ /* provider methods */
+ @Override
+ public String getScheme() {
+ return "ensemble";
+ }
+
+ /**
+ * if things go bad, we don't want to freak out with the logging, so track
+ * the last time we logged something here.
+ */
+ private long lastFailureLogged;
+
+ @Override
+ public KeeperException.Code
+ handleAuthentication(ServerCnxn cnxn, byte[] authData)
+ {
+ if (authData == null || authData.length == 0) {
+ ServerMetrics.ENSEMBLE_AUTH_SKIP.add(1);
+ return KeeperException.Code.OK;
+ }
+
+ String receivedEnsembleName = new String(authData);
+
+ if (ensembleNames == null) {
+ ServerMetrics.ENSEMBLE_AUTH_SKIP.add(1);
+ return KeeperException.Code.OK;
+ }
+
+ if (ensembleNames.contains(receivedEnsembleName)) {
+ ServerMetrics.ENSEMBLE_AUTH_SUCCESS.add(1);
+ return KeeperException.Code.OK;
+ }
+
+ long currentTime = System.currentTimeMillis();
+ if (lastFailureLogged + MIN_LOGGING_INTERVAL_MS < currentTime) {
+ String id =
cnxn.getRemoteSocketAddress().getAddress().getHostAddress();
+ LOG.warn("Unexpected ensemble name: ensemble name: {} client ip:
{}", receivedEnsembleName, id);
+ lastFailureLogged = currentTime;
+ }
+ /*
+ * we are doing a close here rather than returning some other error
+ * since we want the client to choose another server to connect to. if
+ * we return an error, the client will get a fatal auth error and
+ * shutdown.
+ */
+ ServerMetrics.ENSEMBLE_AUTH_FAIL.add(1);
+ cnxn.close();
+ return KeeperException.Code.BADARGUMENTS;
+ }
+
+ /*
+ * since we aren't a true provider we return false for everything so that
+ * it isn't used in ACLs.
+ */
+ @Override
+ public boolean matches(String id, String aclExpr) {
+ return false;
+ }
+
+ @Override
+ public boolean isAuthenticated() {
+ return false;
+ }
+
+ @Override
+ public boolean isValid(String id) {
+ return false;
+ }
+}
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/ProviderRegistry.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/ProviderRegistry.java
index ed69f92f53..9b4d1d0018 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/ProviderRegistry.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/ProviderRegistry.java
@@ -44,8 +44,6 @@ public static void reset() {
public static void initialize() {
synchronized (ProviderRegistry.class) {
- if (initialized)
- return;
IPAuthenticationProvider ipp = new IPAuthenticationProvider();
DigestAuthenticationProvider digp = new
DigestAuthenticationProvider();
authenticationProviders.put(ipp.getScheme(), ipp);
@@ -80,6 +78,10 @@ public static AuthenticationProvider getProvider(String
scheme) {
return authenticationProviders.get(scheme);
}
+ public static void removeProvider(String scheme) {
+ authenticationProviders.remove(scheme);
+ }
+
public static String listProviders() {
StringBuilder sb = new StringBuilder();
for(String s: authenticationProviders.keySet()) {
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/MockServerCnxn.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/MockServerCnxn.java
index 20cf36dc88..1ca3c548ba 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/MockServerCnxn.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/MockServerCnxn.java
@@ -39,7 +39,7 @@ int getSessionTimeout() {
}
@Override
- void close() {
+ public void close() {
}
@Override
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/EnsembleAuthTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/EnsembleAuthTest.java
new file mode 100644
index 0000000000..6724573dce
--- /dev/null
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/EnsembleAuthTest.java
@@ -0,0 +1,121 @@
+/**
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.test;
+
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.server.auth.EnsembleAuthenticationProvider;
+import org.apache.zookeeper.server.auth.ProviderRegistry;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class EnsembleAuthTest extends ClientBase {
+
+ @Before
+ public void setUp() throws Exception {
+ System.setProperty("zookeeper.authProvider.1",
"org.apache.zookeeper.server.auth.EnsembleAuthenticationProvider");
+ super.setUp();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ super.tearDown();
+ System.clearProperty("zookeeper.authProvider.1");
+ System.clearProperty(EnsembleAuthenticationProvider.ENSEMBLE_PROPERTY);
+ ProviderRegistry.removeProvider("ensemble");
+ }
+
+
+ @Test
+ public void noAuth() throws Exception {
+ resetEnsembleAuth(null, false);
+ connectToEnsemble(null);
+ }
+
+ @Test
+ public void emptyAuth() throws Exception {
+ resetEnsembleAuth(null, true);
+ connectToEnsemble("foo");
+ }
+
+ @Test
+ public void skipAuth() throws Exception {
+ resetEnsembleAuth("woo", true);
+ connectToEnsemble(null);
+ }
+
+ @Test
+ public void passAuth() throws Exception {
+ resetEnsembleAuth("woo", true);
+ connectToEnsemble("woo");
+ }
+
+ @Test
+ public void passAuthCSV() throws Exception {
+ resetEnsembleAuth(" foo,bar, baz ", true);
+
+ connectToEnsemble("foo");
+ connectToEnsemble("bar");
+ connectToEnsemble("baz");
+ }
+
+ @Test(expected = KeeperException.ConnectionLossException.class)
+ public void failAuth() throws Exception {
+ resetEnsembleAuth("woo", true);
+ connectToEnsemble("goo");
+ }
+
+ @Test(expected = KeeperException.AuthFailedException.class)
+ public void removeEnsembleAuthProvider() throws Exception {
+ resetEnsembleAuth(null, false);
+ connectToEnsemble("goo");
+ }
+
+
+ private void connectToEnsemble(final String auth) throws IOException,
InterruptedException, KeeperException {
+ try (ZooKeeper zk = createClient()) {
+ // pass auth check
+ if (auth != null) {
+ zk.addAuthInfo("ensemble", auth.getBytes());
+ }
+ zk.getData("/", false, null);
+ }
+ }
+
+ private void resetEnsembleAuth(final String auth, final boolean useAuth)
throws Exception {
+ stopServer();
+ if (auth == null) {
+
System.clearProperty(EnsembleAuthenticationProvider.ENSEMBLE_PROPERTY);
+ } else {
+
System.setProperty(EnsembleAuthenticationProvider.ENSEMBLE_PROPERTY, auth);
+ }
+ if (useAuth) {
+ System.setProperty("zookeeper.authProvider.1",
+
"org.apache.zookeeper.server.auth.EnsembleAuthenticationProvider");
+ } else {
+ System.clearProperty("zookeeper.authProvider.1");
+ }
+ ProviderRegistry.removeProvider("ensemble");
+ ProviderRegistry.initialize();
+ startServer();
+ }
+}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
