This is an automated email from the ASF dual-hosted git repository. edcoleman pushed a commit to branch 2.1 in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push: new 247af741f4 add option to dump Zk ACls with zoo-info-viewer (#3118) 247af741f4 is described below commit 247af741f428754c8f9dcbb832882899e6927b00 Author: EdColeman <d...@etcoleman.com> AuthorDate: Thu Jan 19 12:26:48 2023 -0500 add option to dump Zk ACls with zoo-info-viewer (#3118) Adds option to zoo-info-viewer utility to print ZooKeeper ACLs This closes https://github.com/apache/accumulo/pull/3118 During upgrades, if Accumulo does not have ZooKeeper permissions to write or create nodes, the upgrade will fail - or there will be operational issues. This utility helps operators by dumping all of the current ACLs in ZooKeeper and will flag an error if the permissions are insufficient. The original problem was highlighted in issue https://github.com/apache/accumulo/issues/2890 Addition checks have been added to the upgrade code, but this stand-alone utility can be used prior to attempting the upgrade. --- .../accumulo/core/fate/zookeeper/ZooReader.java | 1 - .../accumulo/server/conf/util/ZooInfoViewer.java | 104 +++++++++++++ .../accumulo/server/zookeeper/ZooAclUtil.java | 165 +++++++++++++++++++++ .../accumulo/server/zookeeper/ZooAclUtilTest.java | 87 +++++++++++ 4 files changed, 356 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooReader.java b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooReader.java index 2721e0fc9a..05de7af365 100644 --- a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooReader.java +++ b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooReader.java @@ -210,5 +210,4 @@ public class ZooReader { // non-transient issue should always be thrown and handled by the caller return false; } - } diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/util/ZooInfoViewer.java b/server/base/src/main/java/org/apache/accumulo/server/conf/util/ZooInfoViewer.java index a2c5a6a517..3bbfcfac79 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/conf/util/ZooInfoViewer.java +++ b/server/base/src/main/java/org/apache/accumulo/server/conf/util/ZooInfoViewer.java @@ -26,6 +26,9 @@ import static org.apache.accumulo.core.Constants.ZROOT; import static org.apache.accumulo.core.Constants.ZTABLES; import static org.apache.accumulo.core.Constants.ZTABLE_NAME; import static org.apache.accumulo.core.Constants.ZTABLE_NAMESPACE; +import static org.apache.accumulo.server.zookeeper.ZooAclUtil.checkWritableAuth; +import static org.apache.accumulo.server.zookeeper.ZooAclUtil.extractAuthName; +import static org.apache.accumulo.server.zookeeper.ZooAclUtil.translateZooPerm; import java.io.BufferedWriter; import java.io.FileOutputStream; @@ -38,6 +41,7 @@ import java.time.ZoneId; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; import java.util.ArrayList; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Set; @@ -64,8 +68,13 @@ import org.apache.accumulo.server.conf.store.TablePropKey; import org.apache.accumulo.server.conf.store.impl.PropStoreWatcher; import org.apache.accumulo.server.conf.store.impl.ReadyMonitor; import org.apache.accumulo.server.conf.store.impl.ZooPropStore; +import org.apache.accumulo.server.zookeeper.ZooAclUtil; import org.apache.accumulo.start.spi.KeywordExecutable; import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.ZKUtil; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.data.ACL; +import org.apache.zookeeper.data.Stat; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -81,6 +90,7 @@ public class ZooInfoViewer implements KeywordExecutable { private static final DateTimeFormatter tsFormat = DateTimeFormatter.ISO_OFFSET_DATE_TIME.withZone(ZoneId.from(ZoneOffset.UTC)); private static final Logger log = LoggerFactory.getLogger(ZooInfoViewer.class); + private final NullWatcher nullWatcher = new NullWatcher(new ReadyMonitor(ZooInfoViewer.class.getSimpleName(), 20_000L)); @@ -153,6 +163,9 @@ public class ZooInfoViewer implements KeywordExecutable { printProps(iid, zooReader, opts, writer); } + if (opts.printAcls) { + printAcls(iid, opts, writer); + } writer.println("-----------------------------------------------"); } } @@ -272,6 +285,90 @@ public class ZooInfoViewer implements KeywordExecutable { writer.println(); } + private void printAcls(final InstanceId iid, final Opts opts, final PrintWriter writer) { + + Map<String,List<ACL>> aclMap = new TreeMap<>(); + + writer.printf("Output format:\n"); + writer.printf("ACCUMULO_PERM:OTHER_PERM path user_acls...\n\n"); + + writer.printf("ZooKeeper acls for instance ID: %s\n\n", iid.canonical()); + + ZooKeeper zooKeeper = new ZooReaderWriter(opts.getSiteConfiguration()).getZooKeeper(); + + String instanceRoot = ZooUtil.getRoot(iid); + + final Stat stat = new Stat(); + + recursiveAclRead(zooKeeper, ZROOT + ZINSTANCES, stat, aclMap); + + recursiveAclRead(zooKeeper, instanceRoot, stat, aclMap); + + // print formatting + aclMap.forEach((path, acl) -> { + if (acl == null) { + writer.printf("ERROR_ACCUMULO_MISSING_SOME: '%s' : none\n", path); + } else { + // sort for consistent presentation + acl.sort(Comparator.comparing(a -> a.getId().getId())); + ZooAclUtil.ZkAccumuloAclStatus aclStatus = checkWritableAuth(acl); + + String authStatus; + if (aclStatus.accumuloHasFull()) { + authStatus = "ACCUMULO_OKAY"; + } else { + authStatus = "ERROR_ACCUMULO_MISSING_SOME"; + } + + String otherUpdate; + if (aclStatus.othersMayUpdate() || aclStatus.anyCanRead()) { + otherUpdate = "NOT_PRIVATE"; + } else { + otherUpdate = "PRIVATE"; + } + + writer.printf("%s:%s %s", authStatus, otherUpdate, path); + boolean addSeparator = false; + for (ACL a : acl) { + if (addSeparator) { + writer.printf(","); + } + writer.printf(" %s:%s", translateZooPerm(a.getPerms()), extractAuthName(a)); + addSeparator = true; + } + } + writer.println(""); + }); + writer.flush(); + } + + private void recursiveAclRead(final ZooKeeper zooKeeper, final String rootPath, final Stat stat, + final Map<String,List<ACL>> aclMap) { + try { + ZKUtil.visitSubTreeDFS(zooKeeper, rootPath, false, (rc, path, ctx, name) -> { + try { + final List<ACL> acls = zooKeeper.getACL(path, stat); + // List<ACL> acl = getAcls(childPath, dummy); + aclMap.put(path, acls); + + } catch (KeeperException.NoNodeException ex) { + throw new IllegalStateException("Node removed during processing", ex); + } catch (KeeperException ex) { + throw new IllegalStateException("ZooKeeper exception during processing", ex); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("interrupted during processing", ex); + } + }); + } catch (KeeperException ex) { + throw new IllegalStateException("ZooKeeper exception during processing", ex); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + throw new IllegalStateException("interrupted during processing", ex); + } + + } + /** * Read the instance names and instance ids from ZooKeeper. The storage structure in ZooKeeper is: * @@ -450,6 +547,13 @@ public class ZooInfoViewer implements KeywordExecutable { description = "Write the output to a file, if the file exists will not be overwritten.") public String outfile = ""; + @Parameter(names = {"--print-acls"}, + description = "print the current acls for all ZooKeeper nodes. The acls are evaluated in context of Accumulo " + + "operations. Context: ACCUMULO_OKAY | ERROR_ACCUMULO_MISSING_SOME - Accumulo requires cdwra for ZooKeeper " + + " nodes that it uses. PRIVATE | NOT_PRIVATE - other than configuration, most nodes are world read-able " + + "(NOT_PRIVATE) to permit client access") + public boolean printAcls = false; + @Parameter(names = {"--print-id-map"}, description = "print the namespace and table id, name mappings stored in ZooKeeper") public boolean printIdMap = false; diff --git a/server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooAclUtil.java b/server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooAclUtil.java new file mode 100644 index 0000000000..6b5bb73682 --- /dev/null +++ b/server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooAclUtil.java @@ -0,0 +1,165 @@ +/* + * 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 + * + * https://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.accumulo.server.zookeeper; + +import static java.util.Objects.requireNonNull; + +import java.util.List; + +import org.apache.zookeeper.ZooDefs; +import org.apache.zookeeper.data.ACL; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ZooAclUtil { + + private static final Logger log = LoggerFactory.getLogger(ZooAclUtil.class); + + /** + * Utility class - prevent instantiation. + */ + private ZooAclUtil() {} + + public static String extractAuthName(ACL acl) { + requireNonNull(acl, "provided ACL cannot be null"); + try { + return acl.getId().getId().trim().split(":")[0]; + } catch (Exception ex) { + log.debug("Invalid ACL passed, cannot parse id from '{}'", acl); + return ""; + } + } + + /** + * translate the ZooKeeper ACL perm bits into a string. The output order is cdrwa (when all perms + * are set) Copied from `org.apache.zookeeper.ZKUtil.getPermString()` added in more recent + * ZooKeeper versions. + */ + public static String translateZooPerm(final int perm) { + if (perm == ZooDefs.Perms.ALL) { + return "cdrwa"; + } + StringBuilder sb = new StringBuilder(); + if ((perm & ZooDefs.Perms.CREATE) != 0) { + sb.append("c"); + } + if ((perm & ZooDefs.Perms.DELETE) != 0) { + sb.append("d"); + } + if ((perm & ZooDefs.Perms.READ) != 0) { + sb.append("r"); + } + if ((perm & ZooDefs.Perms.WRITE) != 0) { + sb.append("w"); + } + if ((perm & ZooDefs.Perms.ADMIN) != 0) { + sb.append("a"); + } + if (sb.length() == 0) { + return "invalid"; + } + return sb.toString(); + } + + /** + * Process the ZooKeeper acls and return a structure that shows if the accumulo user has ALL + * (cdrwa) access and if any other user has anything other than read access. + * <ul> + * <li>Accumulo having access is expected - anything else needs checked + * <li>Anyone having more than read access is unexpected and should be checked. + * </ul> + * + * @param acls ZooKeeper ACLs for a node + * @return acl status with accumulo and other accesses. + */ + public static ZkAccumuloAclStatus checkWritableAuth(List<ACL> acls) { + + ZkAccumuloAclStatus result = new ZkAccumuloAclStatus(); + + for (ACL a : acls) { + String name = extractAuthName(a); + // check accumulo has or inherits all access + if ("accumulo".equals(name) || "anyone".equals(name)) { + if (a.getPerms() == ZooDefs.Perms.ALL) { + result.setAccumuloHasFull(); + } + } + // check if not accumulo and any perm bit other than read is set + if (!"accumulo".equals(name) && ((a.getPerms() & ~ZooDefs.Perms.READ) != 0)) { + result.setOthersMayUpdate(); + } + // check if not accumulo and any read perm is set + if (!"accumulo".equals(name) && ((a.getPerms() & ZooDefs.Perms.READ) != 0)) { + result.setAnyCanRead(); + } + } + return result; + } + + /** + * Wrapper for decoding ZooKeeper ACLs in context of Accumulo usages. The expected usage is this + * will be used by Accumulo utilities that print or check permissions of ZooKeeper nodes created / + * owned by Accumulo. + * <ul> + * <li>It is expected that the authenticated Accumulo session will have cdrwa (ALL ZooKeeper + * permissions). {@link #accumuloHasFull() accumuloHasFull()} should true. + * <li>Typically, no other users should have permissions to modify any node in ZooKeeper under the + * /accumulo path except for /accumulo/instances path {@link #othersMayUpdate() othersMayUpdate()} + * return false. + * <li>Most ZooKeeper nodes permit world read - a few are sensitive and are accessible only by the + * authenticated Accumulo sessions. {@link #anyCanRead() anyCanRead()} returns true. + * </ul> + */ + public static class ZkAccumuloAclStatus { + private boolean accumuloHasFull = false; + private boolean othersMayUpdate = false; + private boolean anyCanRead = false; + + public boolean accumuloHasFull() { + return accumuloHasFull; + } + + public void setAccumuloHasFull() { + this.accumuloHasFull = true; + } + + public boolean othersMayUpdate() { + return othersMayUpdate; + } + + public void setOthersMayUpdate() { + this.othersMayUpdate = true; + } + + public boolean anyCanRead() { + return anyCanRead; + } + + public void setAnyCanRead() { + this.anyCanRead = true; + } + + @Override + public String toString() { + return "ZkAccumuloAclStatus{accumuloHasFull=" + accumuloHasFull + ", anyMayUpdate=" + + othersMayUpdate + ", anyCanRead=" + anyCanRead + '}'; + } + } + +} diff --git a/server/base/src/test/java/org/apache/accumulo/server/zookeeper/ZooAclUtilTest.java b/server/base/src/test/java/org/apache/accumulo/server/zookeeper/ZooAclUtilTest.java new file mode 100644 index 0000000000..e02c1c3bf5 --- /dev/null +++ b/server/base/src/test/java/org/apache/accumulo/server/zookeeper/ZooAclUtilTest.java @@ -0,0 +1,87 @@ +/* + * 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 + * + * https://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.accumulo.server.zookeeper; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; + +import org.apache.zookeeper.ZooDefs; +import org.apache.zookeeper.data.ACL; +import org.apache.zookeeper.data.Id; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ZooAclUtilTest { + + private static final Logger log = LoggerFactory.getLogger(ZooAclUtilTest.class); + + @Test + public void extractAuthNameTest() { + assertThrows(NullPointerException.class, () -> ZooAclUtil.extractAuthName(null)); + assertEquals("", + ZooAclUtil.extractAuthName(new ACL(ZooDefs.Perms.ALL, new Id("digest", null)))); + assertEquals("accumulo", ZooAclUtil + .extractAuthName(new ACL(ZooDefs.Perms.ALL, new Id("digest", "accumulo:abcd123")))); + assertEquals("noauth", + ZooAclUtil.extractAuthName(new ACL(ZooDefs.Perms.ALL, new Id("digest", "noauth")))); + assertEquals("noscheme", + ZooAclUtil.extractAuthName(new ACL(ZooDefs.Perms.ALL, new Id("", "noscheme")))); + assertEquals("", ZooAclUtil.extractAuthName(new ACL(ZooDefs.Perms.ALL, new Id("digest", "")))); + } + + @Test + public void translateZooPermTest() { + assertEquals("invalid", ZooAclUtil.translateZooPerm(0)); + assertEquals("cdrwa", ZooAclUtil.translateZooPerm(ZooDefs.Perms.ALL)); + assertEquals("drw", ZooAclUtil + .translateZooPerm(ZooDefs.Perms.DELETE | ZooDefs.Perms.READ | ZooDefs.Perms.WRITE)); + assertEquals("ca", ZooAclUtil.translateZooPerm(ZooDefs.Perms.CREATE | ZooDefs.Perms.ADMIN)); + } + + @Test + public void checkWritableAuthTest() { + ACL a1 = new ACL(ZooDefs.Perms.ALL, new Id("digest", "accumulo:abcd123")); + ZooAclUtil.ZkAccumuloAclStatus status = ZooAclUtil.checkWritableAuth(List.of(a1)); + assertTrue(status.accumuloHasFull()); + assertFalse(status.othersMayUpdate()); + assertFalse(status.anyCanRead()); + + ACL a2 = new ACL(ZooDefs.Perms.ALL, new Id("digest", "someone:abcd123")); + status = ZooAclUtil.checkWritableAuth(List.of(a2)); + assertFalse(status.accumuloHasFull()); + assertTrue(status.othersMayUpdate()); + assertTrue(status.anyCanRead()); + + ACL a3 = new ACL(ZooDefs.Perms.READ, new Id("digest", "reader")); + status = ZooAclUtil.checkWritableAuth(List.of(a3)); + assertFalse(status.accumuloHasFull()); + assertFalse(status.othersMayUpdate()); + assertTrue(status.anyCanRead()); + + status = ZooAclUtil.checkWritableAuth(List.of(a1, a2, a3)); + assertTrue(status.accumuloHasFull()); + assertTrue(status.othersMayUpdate()); + assertTrue(status.anyCanRead()); + } +}