This is an automated email from the ASF dual-hosted git repository.
andor 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 ceebda949 ZOOKEEPER-2590: exists() should check read ACL permission
ceebda949 is described below
commit ceebda9493bd2e406973e1f4a7f77f9e0121ba16
Author: Andor Molnár <[email protected]>
AuthorDate: Mon Dec 4 13:30:16 2023 +0100
ZOOKEEPER-2590: exists() should check read ACL permission
ZOOKEEPER-2590:exists() should check read ACL permission
ZOOKEEPER-2590. Skip ACL check if znode is missing
Reviewers: eolivelli
Author: anmolnar
Closes #2093 from anmolnar/ZOOKEEPER-2590
---
.../zookeeper/server/FinalRequestProcessor.java | 11 ++-
.../java/org/apache/zookeeper/test/ACLTest.java | 93 ++++++++++++++++++++++
2 files changed, 103 insertions(+), 1 deletion(-)
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
index 1edde5d57..d3e20eca2 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
@@ -363,12 +363,21 @@ public class FinalRequestProcessor implements
RequestProcessor {
}
case OpCode.exists: {
lastOp = "EXIS";
- // TODO we need to figure out the security requirement for
this!
ExistsRequest existsRequest =
request.readRequestRecord(ExistsRequest::new);
path = existsRequest.getPath();
if (path.indexOf('\0') != -1) {
throw new KeeperException.BadArgumentsException();
}
+ DataNode n = zks.getZKDatabase().getNode(path);
+ if (n != null) {
+ zks.checkACL(
+ request.cnxn,
+ zks.getZKDatabase().aclForNode(n),
+ ZooDefs.Perms.READ,
+ request.authInfo,
+ path,
+ null);
+ }
Stat stat = zks.getZKDatabase().statNode(path,
existsRequest.getWatch() ? cnxn : null);
rsp = new ExistsResponse(stat);
requestPathMetricsCollector.registerRequest(request.type,
path);
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ACLTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ACLTest.java
index 2f0d408fc..965d99c4c 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ACLTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ACLTest.java
@@ -19,7 +19,10 @@
package org.apache.zookeeper.test;
import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import java.io.File;
@@ -27,16 +30,19 @@ import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.KeeperException.InvalidACLException;
import org.apache.zookeeper.PortAssignment;
import org.apache.zookeeper.WatchedEvent;
import org.apache.zookeeper.Watcher;
import org.apache.zookeeper.Watcher.Event.KeeperState;
import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.ZooDefs.Ids;
import org.apache.zookeeper.ZooKeeper;
import org.apache.zookeeper.data.ACL;
import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.data.Stat;
import org.apache.zookeeper.server.ServerCnxn;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.apache.zookeeper.server.SyncRequestProcessor;
@@ -302,4 +308,91 @@ public class ACLTest extends ZKTestCase implements Watcher
{
}
}
+ @Test
+ public void testExistACLCheck() throws Exception {
+ File tmpDir = ClientBase.createTmpDir();
+ ClientBase.setupTestEnv();
+ ZooKeeperServer zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
+ final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
+ ServerCnxnFactory f = ServerCnxnFactory.createFactory(PORT, -1);
+ f.startup(zks);
+ String path = "/testExistACLCheck";
+ String data = "/testExistACLCheck-data";
+ try {
+ LOG.info("starting up the zookeeper server .. waiting");
+ assertTrue(ClientBase.waitForServerUp(HOSTPORT,
CONNECTION_TIMEOUT), "waiting for server being up");
+ ZooKeeper zk = ClientBase.createZKClient(HOSTPORT);
+ try {
+ Stat stat = zk.exists(path, false);
+ assertNull(stat);
+ zk.create(path, data.getBytes(), Ids.OPEN_ACL_UNSAFE,
CreateMode.PERSISTENT);
+ stat = zk.exists(path, false);
+ assertNotNull(stat);
+ assertEquals(data.length(), stat.getDataLength());
+
+ zk.delete(path, -1);
+ ArrayList<ACL> acls = new ArrayList<>();
+ acls.add(new ACL(ZooDefs.Perms.WRITE, Ids.ANYONE_ID_UNSAFE));
+ zk.create(path, data.getBytes(), acls, CreateMode.PERSISTENT);
+ try {
+ stat = zk.exists(path, false);
+ fail("exists should throw NoAuthException when don't have
read permission");
+ } catch (KeeperException.NoAuthException e) {
+ //expected
+ }
+
+ zk.delete(path, -1);
+ acls = new ArrayList<>();
+ acls.add(new ACL(ZooDefs.Perms.READ, Ids.ANYONE_ID_UNSAFE));
+ zk.create(path, data.getBytes(), acls, CreateMode.PERSISTENT);
+ stat = zk.exists(path, false);
+ assertNotNull(stat);
+ assertEquals(data.length(), stat.getDataLength());
+ } finally {
+ zk.close();
+ }
+ } finally {
+ f.shutdown();
+ zks.shutdown();
+ assertTrue(ClientBase.waitForServerDown(HOSTPORT,
ClientBase.CONNECTION_TIMEOUT), "waiting for server down");
+ }
+ }
+
+ @Test
+ public void testExistACLCheckAtRootPath() throws Exception {
+ File tmpDir = ClientBase.createTmpDir();
+ ClientBase.setupTestEnv();
+ ZooKeeperServer zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
+ final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
+ ServerCnxnFactory f = ServerCnxnFactory.createFactory(PORT, -1);
+ f.startup(zks);
+ try {
+ LOG.info("starting up the zookeeper server .. waiting");
+ assertTrue(ClientBase.waitForServerUp(HOSTPORT,
CONNECTION_TIMEOUT), "waiting for server being up");
+ ZooKeeper zk = ClientBase.createZKClient(HOSTPORT);
+ try {
+ String data = "/testExistACLCheckAtRootPath-data";
+ zk.create("/a", data.getBytes(), Ids.OPEN_ACL_UNSAFE,
CreateMode.PERSISTENT);
+ ArrayList<ACL> acls = new ArrayList<>();
+ acls.add(new ACL(0, Ids.ANYONE_ID_UNSAFE));
+ zk.setACL("/", acls, -1);
+
+ Stat stat = zk.exists("/a", false);
+ assertNotNull(stat);
+ assertEquals(data.length(), stat.getDataLength());
+ try {
+ stat = zk.exists("/", false);
+ fail("exists should throw NoAuthException when removing
root path's ACL permission");
+ } catch (KeeperException.NoAuthException e) {
+ //expected
+ }
+ } finally {
+ zk.close();
+ }
+ } finally {
+ f.shutdown();
+ zks.shutdown();
+ assertTrue(ClientBase.waitForServerDown(HOSTPORT,
ClientBase.CONNECTION_TIMEOUT), "waiting for server down");
+ }
+ }
}
