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());
+  }
+}

Reply via email to