updates column visibility to use accumulo access library (#3746)

Updates ColumnVisibility and VisibilityEvaluator to use the Accumulo Access control library :

https://github.com/apache/accumulo-access


Co-authored-by: Dave Marion <dlmarion@apache.org>

assemble/pom.xml

@@ -181,6 +181,11 @@
       <artifactId>jakarta.xml.bind-api</artifactId>
       <optional>true</optional>
     </dependency>
+    <dependency>
+      <groupId>org.apache.accumulo</groupId>
+      <artifactId>accumulo-access</artifactId>
+      <optional>true</optional>
+    </dependency>
     <dependency>
       <groupId>org.apache.accumulo</groupId>
       <artifactId>accumulo-compaction-coordinator</artifactId>

core/pom.xml

@@ -81,6 +81,10 @@
       <groupId>io.opentelemetry</groupId>
       <artifactId>opentelemetry-context</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.apache.accumulo</groupId>
+      <artifactId>accumulo-access</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.apache.accumulo</groupId>
       <artifactId>accumulo-start</artifactId>
@@ -272,6 +276,8 @@
                 <allow>javax[.]security[.]auth[.]DestroyFailedException</allow>
                 <!-- allow questionable Hadoop exceptions for mapreduce -->
                 <allow>org[.]apache[.]hadoop[.]mapred[.](FileAlreadyExistsException|InvalidJobConfException)</allow>
+                <!-- allow the following types from the visibility API -->
+                <allow>org[.]apache[.]accumulo[.]access[.].*</allow>
               </allows>
             </configuration>
           </execution>

core/src/main/java/org/apache/accumulo/core/client/summary/summarizers/AuthorizationSummarizer.java

@@ -24,14 +24,13 @@
 import java.util.Set;
 import java.util.function.Consumer;
 
+import org.apache.accumulo.access.AccessExpression;
 import org.apache.accumulo.core.client.admin.TableOperations;
 import org.apache.accumulo.core.client.summary.CountingSummarizer;
 import org.apache.accumulo.core.data.ArrayByteSequence;
 import org.apache.accumulo.core.data.ByteSequence;
 import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.data.Value;
-import org.apache.accumulo.core.security.ColumnVisibility;
-import org.apache.accumulo.core.security.ColumnVisibility.Node;
 
 /**
  * Counts unique authorizations in column visibility labels. Leverages super class to defend against
@@ -82,7 +81,10 @@ public void convert(Key k, Value v, Consumer<ByteSequence> consumer) {
       if (vis.length() > 0) {
         Set<ByteSequence> auths = cache.get(vis);
         if (auths == null) {
-          auths = findAuths(vis);
+          auths = new HashSet<>();
+          for (String auth : AccessExpression.of(vis.toArray()).getAuthorizations().asSet()) {
+            auths.add(new ArrayByteSequence(auth));
+          }
           cache.put(new ArrayByteSequence(vis), auths);
         }
 
@@ -91,33 +93,5 @@ public void convert(Key k, Value v, Consumer<ByteSequence> consumer) {
         }
       }
     }
-
-    private Set<ByteSequence> findAuths(ByteSequence vis) {
-      HashSet<ByteSequence> auths = new HashSet<>();
-      byte[] expression = vis.toArray();
-      Node root = new ColumnVisibility(expression).getParseTree();
-
-      findAuths(root, expression, auths);
-
-      return auths;
-    }
-
-    private void findAuths(Node node, byte[] expression, HashSet<ByteSequence> auths) {
-      switch (node.getType()) {
-        case AND:
-        case OR:
-          for (Node child : node.getChildren()) {
-            findAuths(child, expression, auths);
-          }
-          break;
-        case TERM:
-          auths.add(node.getTerm(expression));
-          break;
-        case EMPTY:
-          break;
-        default:
-          throw new IllegalArgumentException("Unknown node type " + node.getType());
-      }
-    }
   }
 }

core/src/main/java/org/apache/accumulo/core/clientImpl/ConditionalWriterImpl.java

@@ -42,6 +42,8 @@
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.accumulo.access.AccessEvaluator;
+import org.apache.accumulo.access.IllegalAccessExpressionException;
 import org.apache.accumulo.core.Constants;
 import org.apache.accumulo.core.client.AccumuloException;
 import org.apache.accumulo.core.client.AccumuloSecurityException;
@@ -69,13 +71,9 @@
 import org.apache.accumulo.core.rpc.ThriftUtil;
 import org.apache.accumulo.core.rpc.clients.ThriftClientTypes;
 import org.apache.accumulo.core.security.Authorizations;
-import org.apache.accumulo.core.security.ColumnVisibility;
-import org.apache.accumulo.core.security.VisibilityEvaluator;
-import org.apache.accumulo.core.security.VisibilityParseException;
 import org.apache.accumulo.core.tabletingest.thrift.TabletIngestClientService;
 import org.apache.accumulo.core.tabletserver.thrift.NoSuchScanIDException;
 import org.apache.accumulo.core.trace.TraceUtil;
-import org.apache.accumulo.core.util.BadArgumentException;
 import org.apache.accumulo.core.util.ByteBufferUtil;
 import org.apache.accumulo.core.util.threads.ThreadPools;
 import org.apache.accumulo.core.util.threads.Threads;
@@ -97,14 +95,14 @@ class ConditionalWriterImpl implements ConditionalWriter {
 
   private static final int MAX_SLEEP = 30000;
 
-  private Authorizations auths;
-  private VisibilityEvaluator ve;
-  private Map<Text,Boolean> cache = Collections.synchronizedMap(new LRUMap<>(1000));
+  private final Authorizations auths;
+  private final AccessEvaluator accessEvaluator;
+  private final Map<Text,Boolean> cache = Collections.synchronizedMap(new LRUMap<>(1000));
   private final ClientContext context;
-  private TabletLocator locator;
+  private final TabletLocator locator;
   private final TableId tableId;
   private final String tableName;
-  private long timeout;
+  private final long timeout;
   private final Durability durability;
   private final String classLoaderContext;
 
@@ -374,7 +372,7 @@ private TabletServerMutations<QCMutation> dequeue(String location) {
       ConditionalWriterConfig config) {
     this.context = context;
     this.auths = config.getAuthorizations();
-    this.ve = new VisibilityEvaluator(config.getAuthorizations());
+    this.accessEvaluator = AccessEvaluator.of(config.getAuthorizations().toAccessAuthorizations());
     this.threadPool = context.threadPools().createScheduledExecutorService(
         config.getMaxWriteThreads(), this.getClass().getSimpleName());
     this.locator = new SyncingTabletLocator(context, tableId);
@@ -808,21 +806,24 @@ private List<TCondition> convertConditions(ConditionalMutation cm,
   }
 
   private boolean isVisible(ByteSequence cv) {
-    Text testVis = new Text(cv.toArray());
-    if (testVis.getLength() == 0) {
+
+    if (cv.length() == 0) {
       return true;
     }
 
+    byte[] arrayVis = cv.toArray();
+    Text testVis = new Text(arrayVis);
+
     Boolean b = cache.get(testVis);
     if (b != null) {
       return b;
     }
 
     try {
-      boolean bb = ve.evaluate(new ColumnVisibility(testVis));
+      boolean bb = accessEvaluator.canAccess(arrayVis);
       cache.put(new Text(testVis), bb);
       return bb;
-    } catch (VisibilityParseException | BadArgumentException e) {
+    } catch (IllegalAccessExpressionException e) {
       return false;
     }
   }

core/src/main/java/org/apache/accumulo/core/data/constraints/VisibilityConstraint.java

@@ -24,12 +24,11 @@
 import java.util.HashSet;
 import java.util.List;
 
+import org.apache.accumulo.access.AccessEvaluator;
+import org.apache.accumulo.access.IllegalAccessExpressionException;
+import org.apache.accumulo.core.data.ArrayByteSequence;
 import org.apache.accumulo.core.data.ColumnUpdate;
 import org.apache.accumulo.core.data.Mutation;
-import org.apache.accumulo.core.security.ColumnVisibility;
-import org.apache.accumulo.core.security.VisibilityEvaluator;
-import org.apache.accumulo.core.security.VisibilityParseException;
-import org.apache.accumulo.core.util.BadArgumentException;
 
 /**
  * A constraint that checks the visibility of columns against the actor's authorizations. Violation
@@ -64,7 +63,7 @@ public List<Short> check(Environment env, Mutation mutation) {
       ok = new HashSet<>();
     }
 
-    VisibilityEvaluator ve = null;
+    AccessEvaluator ve = null;
 
     for (ColumnUpdate update : updates) {
 
@@ -78,14 +77,15 @@ public List<Short> check(Environment env, Mutation mutation) {
         try {
 
           if (ve == null) {
-            ve = new VisibilityEvaluator(env.getAuthorizationsContainer());
+            var authContainer = env.getAuthorizationsContainer();
+            ve = AccessEvaluator.of(auth -> authContainer.contains(new ArrayByteSequence(auth)));
           }
 
-          if (!ve.evaluate(new ColumnVisibility(cv))) {
+          if (!ve.canAccess(cv)) {
             return Collections.singletonList((short) 2);
           }
 
-        } catch (BadArgumentException | VisibilityParseException bae) {
+        } catch (IllegalAccessExpressionException iaee) {
           return Collections.singletonList((short) 1);
         }
 

core/src/main/java/org/apache/accumulo/core/iterators/user/TransformingIterator.java

@@ -30,6 +30,9 @@
 import java.util.Map.Entry;
 import java.util.NoSuchElementException;
 
+import org.apache.accumulo.access.AccessEvaluator;
+import org.apache.accumulo.access.AccessExpression;
+import org.apache.accumulo.access.IllegalAccessExpressionException;
 import org.apache.accumulo.core.client.IteratorSetting;
 import org.apache.accumulo.core.conf.ConfigurationTypeHelper;
 import org.apache.accumulo.core.data.ByteSequence;
@@ -43,10 +46,6 @@
 import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
 import org.apache.accumulo.core.iterators.WrappingIterator;
 import org.apache.accumulo.core.security.Authorizations;
-import org.apache.accumulo.core.security.ColumnVisibility;
-import org.apache.accumulo.core.security.VisibilityEvaluator;
-import org.apache.accumulo.core.security.VisibilityParseException;
-import org.apache.accumulo.core.util.BadArgumentException;
 import org.apache.accumulo.core.util.Pair;
 import org.apache.commons.collections4.map.LRUMap;
 import org.apache.hadoop.io.Text;
@@ -103,7 +102,7 @@ public abstract class TransformingIterator extends WrappingIterator implements O
   protected Collection<ByteSequence> seekColumnFamilies;
   protected boolean seekColumnFamiliesInclusive;
 
-  private VisibilityEvaluator ve = null;
+  private AccessEvaluator ve = null;
   private LRUMap<ByteSequence,Boolean> visibleCache = null;
   private LRUMap<ByteSequence,Boolean> parsedVisibilitiesCache = null;
   private long maxBufferSize;
@@ -118,7 +117,7 @@ public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> op
     if (scanning) {
       String auths = options.get(AUTH_OPT);
       if (auths != null && !auths.isEmpty()) {
-        ve = new VisibilityEvaluator(new Authorizations(auths.getBytes(UTF_8)));
+        ve = AccessEvaluator.of(new Authorizations(auths.getBytes(UTF_8)).toAccessAuthorizations());
         visibleCache = new LRUMap<>(100);
       }
     }
@@ -409,13 +408,12 @@ protected boolean canSee(Key key) {
     // Ensure that the visibility (which could have been transformed) parses. Must always do this
     // check, even if visibility is not evaluated.
     ByteSequence visibility = key.getColumnVisibilityData();
-    ColumnVisibility colVis = null;
     Boolean parsed = parsedVisibilitiesCache.get(visibility);
     if (parsed == null) {
       try {
-        colVis = new ColumnVisibility(visibility.toArray());
+        AccessExpression.validate(visibility.toArray());
         parsedVisibilitiesCache.put(visibility, Boolean.TRUE);
-      } catch (BadArgumentException e) {
+      } catch (IllegalAccessExpressionException e) {
         log.error("Parse error after transformation : {}", visibility);
         parsedVisibilitiesCache.put(visibility, Boolean.FALSE);
         if (scanning) {
@@ -441,12 +439,9 @@ protected boolean canSee(Key key) {
     visible = visibleCache.get(visibility);
     if (visible == null) {
       try {
-        if (colVis == null) {
-          colVis = new ColumnVisibility(visibility.toArray());
-        }
-        visible = ve.evaluate(colVis);
+        visible = ve.canAccess(visibility.toArray());
         visibleCache.put(visibility, visible);
-      } catch (VisibilityParseException | BadArgumentException e) {
+      } catch (IllegalAccessExpressionException e) {
         log.error("Parse Error", e);
         visible = Boolean.FALSE;
       }

core/src/main/java/org/apache/accumulo/core/iterators/user/VisibilityFilter.java

@@ -23,6 +23,9 @@
 import java.io.IOException;
 import java.util.Map;
 
+import org.apache.accumulo.access.AccessEvaluator;
+import org.apache.accumulo.access.AccessExpression;
+import org.apache.accumulo.access.IllegalAccessExpressionException;
 import org.apache.accumulo.core.client.IteratorSetting;
 import org.apache.accumulo.core.data.ByteSequence;
 import org.apache.accumulo.core.data.Key;
@@ -32,10 +35,6 @@
 import org.apache.accumulo.core.iterators.OptionDescriber;
 import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
 import org.apache.accumulo.core.security.Authorizations;
-import org.apache.accumulo.core.security.ColumnVisibility;
-import org.apache.accumulo.core.security.VisibilityEvaluator;
-import org.apache.accumulo.core.security.VisibilityParseException;
-import org.apache.accumulo.core.util.BadArgumentException;
 import org.apache.commons.collections4.map.LRUMap;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -45,7 +44,7 @@
  */
 public class VisibilityFilter extends Filter implements OptionDescriber {
 
-  protected VisibilityEvaluator ve;
+  private AccessEvaluator accessEvaluator;
   protected Map<ByteSequence,Boolean> cache;
 
   private static final Logger log = LoggerFactory.getLogger(VisibilityFilter.class);
@@ -66,7 +65,8 @@ public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> op
       String auths = options.get(AUTHS);
       Authorizations authObj = auths == null || auths.isEmpty() ? new Authorizations()
           : new Authorizations(auths.getBytes(UTF_8));
-      this.ve = new VisibilityEvaluator(authObj);
+
+      this.accessEvaluator = AccessEvaluator.of(authObj.toAccessAuthorizations());
     }
     this.cache = new LRUMap<>(1000);
   }
@@ -80,10 +80,10 @@ public boolean accept(Key k, Value v) {
         return b;
       }
       try {
-        new ColumnVisibility(testVis.toArray());
+        AccessExpression.validate(testVis.toArray());
         cache.put(testVis, true);
         return true;
-      } catch (BadArgumentException e) {
+      } catch (IllegalAccessExpressionException e) {
         cache.put(testVis, false);
         return false;
       }
@@ -98,10 +98,10 @@ public boolean accept(Key k, Value v) {
       }
 
       try {
-        boolean bb = ve.evaluate(new ColumnVisibility(testVis.toArray()));
+        boolean bb = accessEvaluator.canAccess(testVis.toArray());
         cache.put(testVis, bb);
         return bb;
-      } catch (VisibilityParseException | BadArgumentException e) {
+      } catch (IllegalAccessExpressionException e) {
         log.error("Parse Error", e);
         return false;
       }