This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new 1e347f8482 updates column visibility to use accumulo access library
(#3746)
1e347f8482 is described below
commit 1e347f84828a597d97f698e276be11c12d37f41d
Author: Keith Turner <[email protected]>
AuthorDate: Mon Jul 8 14:38:36 2024 -0700
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 <[email protected]>
---
assemble/pom.xml | 5 +
core/pom.xml | 6 +
.../summarizers/AuthorizationSummarizer.java | 36 +-----
.../core/clientImpl/ConditionalWriterImpl.java | 29 ++---
.../data/constraints/VisibilityConstraint.java | 16 +--
.../core/iterators/user/TransformingIterator.java | 23 ++--
.../core/iterators/user/VisibilityFilter.java | 20 +--
.../iteratorsImpl/system/VisibilityFilter.java | 19 ++-
.../accumulo/core/security/Authorizations.java | 20 +++
.../accumulo/core/security/ColumnVisibility.java | 85 ++++++++-----
.../core/security/VisibilityEvaluator.java | 134 +++------------------
.../core/security/VisibilityParseException.java | 11 ++
.../accumulo/core/util/BadArgumentException.java | 7 ++
.../data/constraints/VisibilityConstraintTest.java | 13 +-
.../core/security/ColumnVisibilityTest.java | 13 ++
.../core/security/VisibilityEvaluatorTest.java | 24 +---
pom.xml | 6 +
17 files changed, 206 insertions(+), 261 deletions(-)
diff --git a/assemble/pom.xml b/assemble/pom.xml
index 4e0bc83497..fbb5a9e698 100644
--- a/assemble/pom.xml
+++ b/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>
diff --git a/core/pom.xml b/core/pom.xml
index c064563a4e..ff2523748f 100644
--- a/core/pom.xml
+++ b/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>
diff --git
a/core/src/main/java/org/apache/accumulo/core/client/summary/summarizers/AuthorizationSummarizer.java
b/core/src/main/java/org/apache/accumulo/core/client/summary/summarizers/AuthorizationSummarizer.java
index 5660f665c3..77a92442e5 100644
---
a/core/src/main/java/org/apache/accumulo/core/client/summary/summarizers/AuthorizationSummarizer.java
+++
b/core/src/main/java/org/apache/accumulo/core/client/summary/summarizers/AuthorizationSummarizer.java
@@ -24,14 +24,13 @@ import java.util.Map;
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 class AuthorizationSummarizer extends
CountingSummarizer<ByteSequence> {
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 class AuthorizationSummarizer extends
CountingSummarizer<ByteSequence> {
}
}
}
-
- 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());
- }
- }
}
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/clientImpl/ConditionalWriterImpl.java
b/core/src/main/java/org/apache/accumulo/core/clientImpl/ConditionalWriterImpl.java
index 3137a19f00..4ddcdae552 100644
---
a/core/src/main/java/org/apache/accumulo/core/clientImpl/ConditionalWriterImpl.java
+++
b/core/src/main/java/org/apache/accumulo/core/clientImpl/ConditionalWriterImpl.java
@@ -42,6 +42,8 @@ import java.util.concurrent.ScheduledFuture;
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.lock.ServiceLock;
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 @@ class ConditionalWriterImpl implements ConditionalWriter {
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 @@ class ConditionalWriterImpl implements ConditionalWriter {
}
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;
}
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/data/constraints/VisibilityConstraint.java
b/core/src/main/java/org/apache/accumulo/core/data/constraints/VisibilityConstraint.java
index 62531c5fa0..26f55f109a 100644
---
a/core/src/main/java/org/apache/accumulo/core/data/constraints/VisibilityConstraint.java
+++
b/core/src/main/java/org/apache/accumulo/core/data/constraints/VisibilityConstraint.java
@@ -24,12 +24,11 @@ import java.util.Collections;
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 class VisibilityConstraint implements Constraint {
ok = new HashSet<>();
}
- VisibilityEvaluator ve = null;
+ AccessEvaluator ve = null;
for (ColumnUpdate update : updates) {
@@ -78,14 +77,15 @@ public class VisibilityConstraint implements Constraint {
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);
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/iterators/user/TransformingIterator.java
b/core/src/main/java/org/apache/accumulo/core/iterators/user/TransformingIterator.java
index be8d63330e..10dffb0083 100644
---
a/core/src/main/java/org/apache/accumulo/core/iterators/user/TransformingIterator.java
+++
b/core/src/main/java/org/apache/accumulo/core/iterators/user/TransformingIterator.java
@@ -30,6 +30,9 @@ import java.util.Map;
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.OptionDescriber;
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 abstract class TransformingIterator extends
WrappingIterator implements O
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 @@ public abstract class TransformingIterator extends
WrappingIterator implements O
// 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 @@ public abstract class TransformingIterator extends
WrappingIterator implements O
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;
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/iterators/user/VisibilityFilter.java
b/core/src/main/java/org/apache/accumulo/core/iterators/user/VisibilityFilter.java
index c91009bb6f..92fb75a72d 100644
---
a/core/src/main/java/org/apache/accumulo/core/iterators/user/VisibilityFilter.java
+++
b/core/src/main/java/org/apache/accumulo/core/iterators/user/VisibilityFilter.java
@@ -23,6 +23,9 @@ import static java.nio.charset.StandardCharsets.UTF_8;
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.IteratorEnvironment;
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 @@ import org.slf4j.LoggerFactory;
*/
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 class VisibilityFilter extends Filter implements
OptionDescriber {
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 class VisibilityFilter extends Filter implements
OptionDescriber {
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 class VisibilityFilter extends Filter implements
OptionDescriber {
}
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;
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/VisibilityFilter.java
b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/VisibilityFilter.java
index 0a0fd5f4aa..22acd782ea 100644
---
a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/VisibilityFilter.java
+++
b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/VisibilityFilter.java
@@ -18,6 +18,8 @@
*/
package org.apache.accumulo.core.iteratorsImpl.system;
+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.ByteSequence;
import org.apache.accumulo.core.data.Key;
@@ -26,10 +28,6 @@ import
org.apache.accumulo.core.iterators.IteratorEnvironment;
import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
import org.apache.accumulo.core.iterators.SynchronizedServerFilter;
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;
@@ -43,7 +41,7 @@ import org.slf4j.LoggerFactory;
* class.
*/
public class VisibilityFilter extends SynchronizedServerFilter {
- protected VisibilityEvaluator ve;
+ protected AccessEvaluator ve;
protected ByteSequence defaultVisibility;
protected LRUMap<ByteSequence,Boolean> cache;
protected Authorizations authorizations;
@@ -53,7 +51,7 @@ public class VisibilityFilter extends
SynchronizedServerFilter {
private VisibilityFilter(SortedKeyValueIterator<Key,Value> iterator,
Authorizations authorizations, byte[] defaultVisibility) {
super(iterator);
- this.ve = new VisibilityEvaluator(authorizations);
+ this.ve = AccessEvaluator.of(authorizations.toAccessAuthorizations());
this.authorizations = authorizations;
this.defaultVisibility = new ArrayByteSequence(defaultVisibility);
this.cache = new LRUMap<>(1000);
@@ -80,14 +78,11 @@ public class VisibilityFilter extends
SynchronizedServerFilter {
}
try {
- boolean bb = ve.evaluate(new ColumnVisibility(testVis.toArray()));
+ boolean bb = ve.canAccess(testVis.toArray());
cache.put(testVis, bb);
return bb;
- } catch (VisibilityParseException e) {
- log.error("VisibilityParseException with visibility of Key: {}", k, e);
- return false;
- } catch (BadArgumentException e) {
- log.error("BadArgumentException with visibility of Key: {}", k, e);
+ } catch (IllegalAccessExpressionException e) {
+ log.error("IllegalAccessExpressionException with visibility of Key: {}",
k, e);
return false;
}
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/security/Authorizations.java
b/core/src/main/java/org/apache/accumulo/core/security/Authorizations.java
index 1cfeebb8b9..363a0586f8 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/Authorizations.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/Authorizations.java
@@ -385,4 +385,24 @@ public class Authorizations implements Iterable<byte[]>,
Serializable, Authoriza
return sb.toString();
}
+
+ private static final org.apache.accumulo.access.Authorizations
EMPTY_ACCESS_AUTH =
+ org.apache.accumulo.access.Authorizations.of(Set.of());
+
+ /**
+ * Converts to an Accumulo Access Authorizations object.
+ *
+ * @since 3.1.0
+ */
+ public org.apache.accumulo.access.Authorizations toAccessAuthorizations() {
+ if (auths.isEmpty()) {
+ return EMPTY_ACCESS_AUTH;
+ } else {
+ Set<String> auths = new HashSet<>(authsList.size());
+ for (var auth : authsList) {
+ auths.add(new String(auth, UTF_8));
+ }
+ return org.apache.accumulo.access.Authorizations.of(auths);
+ }
+ }
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java
b/core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java
index 4b8784be02..338cf1396c 100644
--- a/core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java
+++ b/core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java
@@ -27,7 +27,10 @@ import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.TreeSet;
+import java.util.function.Supplier;
+import org.apache.accumulo.access.AccessExpression;
+import org.apache.accumulo.access.IllegalAccessExpressionException;
import org.apache.accumulo.core.data.ArrayByteSequence;
import org.apache.accumulo.core.data.ByteSequence;
import org.apache.accumulo.core.util.BadArgumentException;
@@ -35,6 +38,8 @@ import org.apache.accumulo.core.util.TextUtil;
import org.apache.hadoop.io.Text;
import org.apache.hadoop.io.WritableComparator;
+import com.google.common.base.Suppliers;
+
/**
* Validate the column visibility is a valid expression and set the visibility
for a Mutation. See
* {@link ColumnVisibility#ColumnVisibility(byte[])} for the definition of an
expression.
@@ -76,8 +81,10 @@ import org.apache.hadoop.io.WritableComparator;
*/
public class ColumnVisibility {
- Node node = null;
- private byte[] expression;
+ // This functionality is deprecated so its setup as a supplier so it is only
computed if the
+ // deprecated functionality is called.
+ private final Supplier<Node> nodeSupplier;
+ private final byte[] expression;
/**
* Accessor for the underlying byte string.
@@ -91,6 +98,7 @@ public class ColumnVisibility {
/**
* The node types in a parse tree for a visibility expression.
*/
+ @Deprecated(since = "3.1.0")
public enum NodeType {
EMPTY, TERM, OR, AND,
}
@@ -103,6 +111,7 @@ public class ColumnVisibility {
/**
* A node in the parse tree for a visibility expression.
*/
+ @Deprecated(since = "3.1.0")
public static class Node {
/**
* An empty list of nodes.
@@ -169,6 +178,7 @@ public class ColumnVisibility {
* A node comparator. Nodes sort according to node type, terms sort
lexicographically. AND and OR
* nodes sort by number of children, or if the same by corresponding
children.
*/
+ @Deprecated(since = "3.1.0")
public static class NodeComparator implements Comparator<Node>, Serializable
{
private static final long serialVersionUID = 1L;
@@ -216,6 +226,7 @@ public class ColumnVisibility {
* Convenience method that delegates to normalize with a new NodeComparator
constructed using the
* supplied expression.
*/
+ @Deprecated(since = "3.1.0")
public static Node normalize(Node root, byte[] expression) {
return normalize(root, expression, new NodeComparator(expression));
}
@@ -228,6 +239,7 @@ public class ColumnVisibility {
* 3) dedupes labels (`a&b&a` becomes `a&b`)
*/
// @formatter:on
+ @Deprecated(since = "3.1.0")
public static Node normalize(Node root, byte[] expression, NodeComparator
comparator) {
if (root.type != NodeType.TERM) {
TreeSet<Node> rolledUp = new TreeSet<>(comparator);
@@ -256,6 +268,7 @@ public class ColumnVisibility {
* Walks an expression's AST and appends a string representation to a
supplied StringBuilder. This
* method adds parens where necessary.
*/
+ @Deprecated(since = "3.1.0")
public static void stringify(Node root, byte[] expression, StringBuilder
out) {
if (root.type == NodeType.TERM) {
out.append(new String(expression, root.start, root.end - root.start,
UTF_8));
@@ -282,13 +295,12 @@ public class ColumnVisibility {
*
* @return normalized expression in byte[] form
*/
+ @Deprecated(since = "3.1.0")
public byte[] flatten() {
- Node normRoot = normalize(node, expression);
- StringBuilder builder = new StringBuilder(expression.length);
- stringify(normRoot, expression, builder);
- return builder.toString().getBytes(UTF_8);
+ return AccessExpression.of(expression,
true).getExpression().getBytes(UTF_8);
}
+ @Deprecated
private static class ColumnVisibilityParser {
private int index = 0;
private int parens = 0;
@@ -455,16 +467,17 @@ public class ColumnVisibility {
}
}
- private void validate(byte[] expression) {
+ private Node createNodeTree(byte[] expression) {
if (expression != null && expression.length > 0) {
ColumnVisibilityParser p = new ColumnVisibilityParser();
- node = p.parse(expression);
+ return p.parse(expression);
} else {
- node = EMPTY_NODE;
+ return EMPTY_NODE;
}
- this.expression = expression;
}
+ private static final byte[] EMPTY_BYTES = new byte[0];
+
/**
* Creates an empty visibility. Normally, elements with empty visibility can
be seen by everyone.
* Though, one could change this behavior with filters.
@@ -472,7 +485,8 @@ public class ColumnVisibility {
* @see #ColumnVisibility(String)
*/
public ColumnVisibility() {
- this(new byte[] {});
+ expression = EMPTY_BYTES;
+ nodeSupplier = Suppliers.memoize(() -> createNodeTree(expression));
}
/**
@@ -496,13 +510,34 @@ public class ColumnVisibility {
}
/**
- * Creates a column visibility for a Mutation from a string already encoded
in UTF-8 bytes.
+ * Creates a column visibility for a Mutation from bytes already encoded in
UTF-8.
*
* @param expression visibility expression, encoded as UTF-8 bytes
* @see #ColumnVisibility(String)
*/
public ColumnVisibility(byte[] expression) {
- validate(expression);
+ this.expression = expression;
+ try {
+ AccessExpression.validate(this.expression);
+ } catch (IllegalAccessExpressionException e) {
+ // This is thrown for compatability with the exception this class used
to throw when it parsed
+ // exceptions itself.
+ throw new BadArgumentException(e);
+ }
+ nodeSupplier = Suppliers.memoize(() -> createNodeTree(this.expression));
+ }
+
+ /**
+ * Creates a column visibility for a Mutation from an AccessExpression.
+ *
+ * @param expression visibility expression, encoded as UTF-8 bytes
+ * @see #ColumnVisibility(String)
+ * @since 3.1.0
+ */
+ public ColumnVisibility(AccessExpression expression) {
+ // AccessExpression is a validated immutable object, so no need to re
validate
+ this.expression = expression.getExpression().getBytes(UTF_8);
+ nodeSupplier = Suppliers.memoize(() -> createNodeTree(this.expression));
}
@Override
@@ -542,8 +577,9 @@ public class ColumnVisibility {
*
* @return parse tree node
*/
+ @Deprecated(since = "3.1.0")
public Node getParseTree() {
- return node;
+ return nodeSupplier.get();
}
/**
@@ -564,9 +600,11 @@ public class ColumnVisibility {
*
* @param term term to quote
* @return quoted term (unquoted if unnecessary)
+ * @deprecated use {@link AccessExpression#quote(String)}
*/
+ @Deprecated(since = "3.1.0")
public static String quote(String term) {
- return new String(quote(term.getBytes(UTF_8)), UTF_8);
+ return AccessExpression.quote(term);
}
/**
@@ -576,21 +614,10 @@ public class ColumnVisibility {
* @param term term to quote, encoded as UTF-8 bytes
* @return quoted term (unquoted if unnecessary), encoded as UTF-8 bytes
* @see #quote(String)
+ * @deprecated use {@link AccessExpression#quote(byte[])}
*/
+ @Deprecated(since = "3.1.0")
public static byte[] quote(byte[] term) {
- boolean needsQuote = false;
-
- for (byte b : term) {
- if (!Authorizations.isValidAuthChar(b)) {
- needsQuote = true;
- break;
- }
- }
-
- if (!needsQuote) {
- return term;
- }
-
- return VisibilityEvaluator.escape(term, true);
+ return AccessExpression.quote(term);
}
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/security/VisibilityEvaluator.java
b/core/src/main/java/org/apache/accumulo/core/security/VisibilityEvaluator.java
index dd07d64086..aa5d85f530 100644
---
a/core/src/main/java/org/apache/accumulo/core/security/VisibilityEvaluator.java
+++
b/core/src/main/java/org/apache/accumulo/core/security/VisibilityEvaluator.java
@@ -18,92 +18,18 @@
*/
package org.apache.accumulo.core.security;
-import java.util.ArrayList;
-
+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.ByteSequence;
-import org.apache.accumulo.core.security.ColumnVisibility.Node;
/**
* A class which evaluates visibility expressions against a set of
authorizations.
+ *
+ * @deprecated since 3.1.0 Use Accumulo Access library instead
*/
+@Deprecated(since = "3.1.0")
public class VisibilityEvaluator {
- private AuthorizationContainer auths;
-
- /**
- * Authorizations in column visibility expression are in escaped form.
Column visibility parsing
- * does not unescape. This class wraps an AuthorizationContainer and
unescapes auths before
- * checking the wrapped container.
- */
- private static class UnescapingAuthorizationContainer implements
AuthorizationContainer {
-
- private AuthorizationContainer wrapped;
-
- UnescapingAuthorizationContainer(AuthorizationContainer wrapee) {
- this.wrapped = wrapee;
- }
-
- @Override
- public boolean contains(ByteSequence auth) {
- return wrapped.contains(unescape(auth));
- }
- }
-
- static ByteSequence unescape(ByteSequence auth) {
- int escapeCharCount = 0;
- for (int i = 0; i < auth.length(); i++) {
- byte b = auth.byteAt(i);
- if (b == '"' || b == '\\') {
- escapeCharCount++;
- }
- }
-
- if (escapeCharCount > 0) {
- if (escapeCharCount % 2 == 1) {
- throw new IllegalArgumentException("Illegal escape sequence in auth :
" + auth);
- }
-
- byte[] unescapedCopy = new byte[auth.length() - escapeCharCount / 2];
- int pos = 0;
- for (int i = 0; i < auth.length(); i++) {
- byte b = auth.byteAt(i);
- if (b == '\\') {
- i++;
- b = auth.byteAt(i);
- if (b != '"' && b != '\\') {
- throw new IllegalArgumentException("Illegal escape sequence in
auth : " + auth);
- }
- } else if (b == '"') {
- // should only see quote after a slash
- throw new IllegalArgumentException("Illegal escape sequence in auth
: " + auth);
- }
-
- unescapedCopy[pos++] = b;
- }
-
- return new ArrayByteSequence(unescapedCopy);
- } else {
- return auth;
- }
- }
-
- /**
- * Creates a new {@link Authorizations} object with escaped forms of the
authorizations in the
- * given object.
- *
- * @param auths original authorizations
- * @return authorizations object with escaped authorization strings
- * @see #escape(byte[], boolean)
- */
- static Authorizations escape(Authorizations auths) {
- ArrayList<byte[]> retAuths = new
ArrayList<>(auths.getAuthorizations().size());
-
- for (byte[] auth : auths.getAuthorizations()) {
- retAuths.add(escape(auth, false));
- }
-
- return new Authorizations(retAuths);
- }
+ private final AccessEvaluator accessEvaluator;
/**
* Properly escapes an authorization string. The string can be quoted if
desired.
@@ -147,7 +73,9 @@ public class VisibilityEvaluator {
* @since 1.7.0
*/
public VisibilityEvaluator(AuthorizationContainer authsContainer) {
- this.auths = new UnescapingAuthorizationContainer(authsContainer);
+ // TODO need to look into efficiency and correctness of this
+ this.accessEvaluator =
+ AccessEvaluator.of(auth -> authsContainer.contains(new
ArrayByteSequence(auth)));
}
/**
@@ -157,7 +85,7 @@ public class VisibilityEvaluator {
* @param authorizations authorizations object
*/
public VisibilityEvaluator(Authorizations authorizations) {
- this.auths = escape(authorizations);
+ this.accessEvaluator =
AccessEvaluator.of(authorizations.toAccessAuthorizations());
}
/**
@@ -171,42 +99,12 @@ public class VisibilityEvaluator {
* subexpression is of an unknown type
*/
public boolean evaluate(ColumnVisibility visibility) throws
VisibilityParseException {
- // The VisibilityEvaluator computes a trie from the given Authorizations,
that ColumnVisibility
- // expressions can be evaluated against.
- return evaluate(visibility.getExpression(), visibility.getParseTree());
- }
-
- private final boolean evaluate(final byte[] expression, final Node root)
- throws VisibilityParseException {
- if (expression.length == 0) {
- return true;
- }
- switch (root.type) {
- case TERM:
- return auths.contains(root.getTerm(expression));
- case AND:
- if (root.children == null || root.children.size() < 2) {
- throw new VisibilityParseException("AND has less than 2 children",
expression,
- root.start);
- }
- for (Node child : root.children) {
- if (!evaluate(expression, child)) {
- return false;
- }
- }
- return true;
- case OR:
- if (root.children == null || root.children.size() < 2) {
- throw new VisibilityParseException("OR has less than 2 children",
expression, root.start);
- }
- for (Node child : root.children) {
- if (evaluate(expression, child)) {
- return true;
- }
- }
- return false;
- default:
- throw new VisibilityParseException("No such node type", expression,
root.start);
+ try {
+ return accessEvaluator.canAccess(visibility.getExpression());
+ } catch (IllegalAccessExpressionException e) {
+ // This is thrown for compatability with the exception this class used
to evaluate expressions
+ // itself.
+ throw new VisibilityParseException(e);
}
}
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/security/VisibilityParseException.java
b/core/src/main/java/org/apache/accumulo/core/security/VisibilityParseException.java
index 297e575fb1..46dc634db8 100644
---
a/core/src/main/java/org/apache/accumulo/core/security/VisibilityParseException.java
+++
b/core/src/main/java/org/apache/accumulo/core/security/VisibilityParseException.java
@@ -22,6 +22,8 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import java.text.ParseException;
+import org.apache.accumulo.access.IllegalAccessExpressionException;
+
/**
* An exception thrown when a visibility string cannot be parsed.
*/
@@ -41,6 +43,15 @@ public class VisibilityParseException extends ParseException
{
this.visibility = new String(visibility, UTF_8);
}
+ /**
+ * @since 3.1.0
+ */
+ public VisibilityParseException(IllegalAccessExpressionException e) {
+ // TODO need to look at output for this
+ super(e.getDescription(), e.getIndex());
+ this.visibility = e.getPattern();
+ }
+
@Override
public String getMessage() {
return super.getMessage() + " in string '" + visibility + "' at position "
diff --git
a/core/src/main/java/org/apache/accumulo/core/util/BadArgumentException.java
b/core/src/main/java/org/apache/accumulo/core/util/BadArgumentException.java
index ac9cafe1e4..88e1b6f3a6 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/BadArgumentException.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/BadArgumentException.java
@@ -20,10 +20,17 @@ package org.apache.accumulo.core.util;
import java.util.regex.PatternSyntaxException;
+import org.apache.accumulo.access.IllegalAccessExpressionException;
+
public final class BadArgumentException extends PatternSyntaxException {
private static final long serialVersionUID = 1L;
public BadArgumentException(String desc, String badarg, int index) {
super(desc, badarg, index);
}
+
+ public BadArgumentException(IllegalAccessExpressionException e) {
+ super(e.getDescription(), e.getPattern(), e.getIndex());
+ super.initCause(e);
+ }
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/data/constraints/VisibilityConstraintTest.java
b/core/src/test/java/org/apache/accumulo/core/data/constraints/VisibilityConstraintTest.java
index 70f10b9600..2f31877701 100644
---
a/core/src/test/java/org/apache/accumulo/core/data/constraints/VisibilityConstraintTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/data/constraints/VisibilityConstraintTest.java
@@ -43,11 +43,13 @@ public class VisibilityConstraintTest {
Environment env;
Mutation mutation;
- static final ColumnVisibility good = new ColumnVisibility("good");
- static final ColumnVisibility bad = new ColumnVisibility("bad");
+ static final ColumnVisibility good = new ColumnVisibility("good|bad");
+ static final ColumnVisibility bad = new ColumnVisibility("good&bad");
static final String D = "don't care";
+ static final List<Short> ILLEGAL = Arrays.asList((short) 1);
+
static final List<Short> ENOAUTH = Arrays.asList((short) 2);
@BeforeEach
@@ -98,4 +100,11 @@ public class VisibilityConstraintTest {
assertEquals(ENOAUTH, vc.check(env, mutation), "unauthorized");
}
+ @Test
+ public void testIllegalVisibility() {
+ mutation.put(D, D, good, D);
+ // set an illegal visibility string
+ mutation.at().family(D).qualifier(D).visibility("good&").put(D);
+ assertEquals(ILLEGAL, vc.check(env, mutation), "unauthorized");
+ }
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/security/ColumnVisibilityTest.java
b/core/src/test/java/org/apache/accumulo/core/security/ColumnVisibilityTest.java
index e37af5b265..2723f3abe7 100644
---
a/core/src/test/java/org/apache/accumulo/core/security/ColumnVisibilityTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/security/ColumnVisibilityTest.java
@@ -63,6 +63,7 @@ public class ColumnVisibilityTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testEmptyFlatten() {
// empty visibility is valid
new ColumnVisibility().flatten();
@@ -87,6 +88,7 @@ public class ColumnVisibilityTest {
shouldThrow("a*b");
}
+ @SuppressWarnings("deprecation")
public void normalized(String... values) {
for (int i = 0; i < values.length; i += 2) {
ColumnVisibility cv = new ColumnVisibility(values[i].getBytes());
@@ -161,6 +163,7 @@ public class ColumnVisibilityTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testToString() {
ColumnVisibility cv = new ColumnVisibility(quote("a"));
assertEquals("[a]", cv.toString());
@@ -171,6 +174,7 @@ public class ColumnVisibilityTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testParseTree() {
Node node = parse("(W)|(U&V)");
assertNode(node, NodeType.OR, 0, 9);
@@ -179,12 +183,14 @@ public class ColumnVisibilityTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testParseTreeWithNoChildren() {
Node node = parse("ABC");
assertNode(node, NodeType.TERM, 0, 3);
}
@Test
+ @SuppressWarnings("deprecation")
public void testParseTreeWithTwoChildren() {
Node node = parse("ABC|DEF");
assertNode(node, NodeType.OR, 0, 7);
@@ -193,6 +199,7 @@ public class ColumnVisibilityTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testParseTreeWithParenthesesAndTwoChildren() {
Node node = parse("(ABC|DEF)");
assertNode(node, NodeType.OR, 1, 8);
@@ -201,6 +208,7 @@ public class ColumnVisibilityTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testParseTreeWithParenthesizedChildren() {
Node node = parse("ABC|(DEF&GHI)");
assertNode(node, NodeType.OR, 0, 13);
@@ -211,6 +219,7 @@ public class ColumnVisibilityTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testParseTreeWithMoreParentheses() {
Node node = parse("(W)|(U&V)");
assertNode(node, NodeType.OR, 0, 9);
@@ -221,6 +230,7 @@ public class ColumnVisibilityTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testEmptyParseTreesAreEqual() {
Comparator<Node> comparator = new NodeComparator(new byte[] {});
Node empty = new ColumnVisibility().getParseTree();
@@ -228,6 +238,7 @@ public class ColumnVisibilityTest {
}
@Test
+ @SuppressWarnings("deprecation")
public void testParseTreesOrdering() {
byte[] expression = "(b&c&d)|((a|m)&y&z)|(e&f)".getBytes(UTF_8);
byte[] flattened = new ColumnVisibility(expression).flatten();
@@ -238,11 +249,13 @@ public class ColumnVisibilityTest {
assertTrue(flat.indexOf('b') < flat.indexOf('a'), "shortest children sort
first");
}
+ @SuppressWarnings("deprecation")
private Node parse(String s) {
ColumnVisibility v = new ColumnVisibility(s);
return v.getParseTree();
}
+ @SuppressWarnings("deprecation")
private void assertNode(Node node, NodeType nodeType, int start, int end) {
assertEquals(node.type, nodeType);
assertEquals(start, node.start);
diff --git
a/core/src/test/java/org/apache/accumulo/core/security/VisibilityEvaluatorTest.java
b/core/src/test/java/org/apache/accumulo/core/security/VisibilityEvaluatorTest.java
index 637fb6877d..31ae8ea3ed 100644
---
a/core/src/test/java/org/apache/accumulo/core/security/VisibilityEvaluatorTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/security/VisibilityEvaluatorTest.java
@@ -21,15 +21,12 @@ package org.apache.accumulo.core.security;
import static org.apache.accumulo.core.security.ColumnVisibility.quote;
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.accumulo.core.data.ArrayByteSequence;
import org.apache.accumulo.core.util.ByteArraySet;
import org.junit.jupiter.api.Test;
+@SuppressWarnings("deprecation")
public class VisibilityEvaluatorTest {
@Test
@@ -100,25 +97,6 @@ public class VisibilityEvaluatorTest {
assertEquals("\"五十\"", quote("五十"));
}
- @Test
- public void testUnescape() {
- assertEquals("a\"b", VisibilityEvaluator.unescape(new
ArrayByteSequence("a\\\"b")).toString());
- assertEquals("a\\b", VisibilityEvaluator.unescape(new
ArrayByteSequence("a\\\\b")).toString());
- assertEquals("a\\\"b",
- VisibilityEvaluator.unescape(new
ArrayByteSequence("a\\\\\\\"b")).toString());
- assertEquals("\\\"",
- VisibilityEvaluator.unescape(new
ArrayByteSequence("\\\\\\\"")).toString());
- assertEquals("a\\b\\c\\d",
- VisibilityEvaluator.unescape(new
ArrayByteSequence("a\\\\b\\\\c\\\\d")).toString());
-
- final String message = "Expected failure to unescape invalid escape
sequence";
- final var invalidEscapeSeqList = List.of(new ArrayByteSequence("a\\b"),
- new ArrayByteSequence("a\\b\\c"), new ArrayByteSequence("a\"b\\"));
-
- invalidEscapeSeqList.forEach(seq ->
assertThrows(IllegalArgumentException.class,
- () -> VisibilityEvaluator.unescape(seq), message));
- }
-
@Test
public void testNonAscii() throws VisibilityParseException {
VisibilityEvaluator ct = new VisibilityEvaluator(new Authorizations("五",
"六", "八", "九", "五十"));
diff --git a/pom.xml b/pom.xml
index 901e9db280..014424cdc9 100644
--- a/pom.xml
+++ b/pom.xml
@@ -145,6 +145,7 @@
<surefire.reuseForks>true</surefire.reuseForks>
<unitTestMemSize>-Xmx1G</unitTestMemSize>
<!-- dependency and plugin versions managed with properties -->
+ <version.accumulo-access>1.0.0-beta</version.accumulo-access>
<version.auto-service>1.1.1</version.auto-service>
<version.bouncycastle>1.78.1</version.bouncycastle>
<version.curator>5.5.0</version.curator>
@@ -315,6 +316,11 @@
<artifactId>commons-logging</artifactId>
<version>1.2</version>
</dependency>
+ <dependency>
+ <groupId>org.apache.accumulo</groupId>
+ <artifactId>accumulo-access</artifactId>
+ <version>${version.accumulo-access}</version>
+ </dependency>
<dependency>
<groupId>org.apache.accumulo</groupId>
<artifactId>accumulo-compaction-coordinator</artifactId>
