Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updates column visibility to use accumulo access library #3746

Merged
merged 20 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions assemble/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
6 changes: 6 additions & 0 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand All @@ -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());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {

Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
Loading