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.

Sign up for GitHub

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

Jump to bottom

HBASE-25160 Refactor AccessController and VisibilityController #2506

Merged
merged 1 commit into from Oct 8, 2020
Merged

HBASE-25160 Refactor AccessController and VisibilityController #2506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
66 changes: 11 additions & 55 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
View file
Open in desktop
Expand Up @@ -429,7 +429,6 @@ private enum OpType {
DELETE("delete"),
CHECK_AND_PUT("checkAndPut"),
CHECK_AND_DELETE("checkAndDelete"),
INCREMENT_COLUMN_VALUE("incrementColumnValue"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used anymore? Is this feature no longer in place?

APPEND("append"),
INCREMENT("increment");

Expand Down Expand Up @@ -1503,18 +1502,27 @@ public void preBatchMutate(ObserverContext<RegionCoprocessorEnvironment> c,
// We have a failure with table, cf and q perm checks and now giving a chance for cell
// perm check
OpType opType;
long timestamp;
if (m instanceof Put) {
checkForReservedTagPresence(user, m);
opType = OpType.PUT;
timestamp = m.getTimestamp();
} else if (m instanceof Delete) {
opType = OpType.DELETE;
timestamp = m.getTimestamp();
} else if (m instanceof Increment) {
opType = OpType.INCREMENT;
timestamp = ((Increment) m).getTimeRange().getMax();
} else if (m instanceof Append) {
opType = OpType.APPEND;
timestamp = ((Append) m).getTimeRange().getMax();
} else {
// If the operation type is not Put or Delete, do nothing
// If the operation type is not Put/Delete/Increment/Append, do nothing
continue;
}
AuthResult authResult = null;
if (checkCoveringPermission(user, opType, c.getEnvironment(), m.getRow(),
m.getFamilyCellMap(), m.getTimestamp(), Action.WRITE)) {
m.getFamilyCellMap(), timestamp, Action.WRITE)) {
authResult = AuthResult.allow(opType.toString(), "Covering cell set",
user, Action.WRITE, table, m.getFamilyCellMap());
} else {
Expand Down Expand Up @@ -1695,32 +1703,6 @@ public Result preAppend(ObserverContext<RegionCoprocessorEnvironment> c, Append
return null;
}

@Override
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not used any more?

public Result preAppendAfterRowLock(final ObserverContext<RegionCoprocessorEnvironment> c,
final Append append) throws IOException {
if (append.getAttribute(CHECK_COVERING_PERM) != null) {
// We had failure with table, cf and q perm checks and now giving a chance for cell
// perm check
TableName table = c.getEnvironment().getRegion().getRegionInfo().getTable();
AuthResult authResult = null;
User user = getActiveUser(c);
if (checkCoveringPermission(user, OpType.APPEND, c.getEnvironment(), append.getRow(),
append.getFamilyCellMap(), append.getTimeRange().getMax(), Action.WRITE)) {
authResult = AuthResult.allow(OpType.APPEND.toString(),
"Covering cell set", user, Action.WRITE, table, append.getFamilyCellMap());
} else {
authResult = AuthResult.deny(OpType.APPEND.toString(),
"Covering cell set", user, Action.WRITE, table, append.getFamilyCellMap());
}
AccessChecker.logResult(authResult);
if (authorizationEnabled && !authResult.isAllowed()) {
throw new AccessDeniedException("Insufficient permissions " +
authResult.toContextString());
}
}
return null;
}

@Override
public Result preIncrement(final ObserverContext<RegionCoprocessorEnvironment> c,
final Increment increment)
Expand Down Expand Up @@ -1756,32 +1738,6 @@ public Result preIncrement(final ObserverContext<RegionCoprocessorEnvironment> c
return null;
}

@Override
public Result preIncrementAfterRowLock(final ObserverContext<RegionCoprocessorEnvironment> c,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto?

final Increment increment) throws IOException {
if (increment.getAttribute(CHECK_COVERING_PERM) != null) {
// We had failure with table, cf and q perm checks and now giving a chance for cell
// perm check
TableName table = c.getEnvironment().getRegion().getRegionInfo().getTable();
AuthResult authResult = null;
User user = getActiveUser(c);
if (checkCoveringPermission(user, OpType.INCREMENT, c.getEnvironment(), increment.getRow(),
increment.getFamilyCellMap(), increment.getTimeRange().getMax(), Action.WRITE)) {
authResult = AuthResult.allow(OpType.INCREMENT.toString(), "Covering cell set",
user, Action.WRITE, table, increment.getFamilyCellMap());
} else {
authResult = AuthResult.deny(OpType.INCREMENT.toString(), "Covering cell set",
user, Action.WRITE, table, increment.getFamilyCellMap());
}
AccessChecker.logResult(authResult);
if (authorizationEnabled && !authResult.isAllowed()) {
throw new AccessDeniedException("Insufficient permissions " +
authResult.toContextString());
}
}
return null;
}

@Override
public List<Pair<Cell, Cell>> postIncrementBeforeWAL(
ObserverContext<RegionCoprocessorEnvironment> ctx, Mutation mutation,
Expand Down
66 changes: 2 additions & 64 deletions ...erver/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java
View file
Open in desktop
Expand Up @@ -45,11 +45,9 @@
import org.apache.hadoop.hbase.Tag;
import org.apache.hadoop.hbase.TagType;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.Append;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.client.Delete;
import org.apache.hadoop.hbase.client.Get;
import org.apache.hadoop.hbase.client.Increment;
import org.apache.hadoop.hbase.client.MasterSwitchType;
import org.apache.hadoop.hbase.client.Mutation;
import org.apache.hadoop.hbase.client.Put;
Expand All @@ -69,7 +67,6 @@
import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
import org.apache.hadoop.hbase.coprocessor.RegionObserver;
import org.apache.hadoop.hbase.exceptions.DeserializationException;
import org.apache.hadoop.hbase.exceptions.FailedSanityCheckException;
import org.apache.hadoop.hbase.filter.Filter;
import org.apache.hadoop.hbase.filter.FilterBase;
import org.apache.hadoop.hbase.filter.FilterList;
Expand Down Expand Up @@ -323,7 +320,7 @@ public void preBatchMutate(ObserverContext<RegionCoprocessorEnvironment> c,
}
}
}
if (!sanityFailure) {
if (!sanityFailure && (m instanceof Put || m instanceof Delete)) {
if (cellVisibility != null) {
String labelsExp = cellVisibility.getExpression();
List<Tag> visibilityTags = labelCache.get(labelsExp);
Expand Down Expand Up @@ -360,7 +357,7 @@ public void preBatchMutate(ObserverContext<RegionCoprocessorEnvironment> c,
if (m instanceof Put) {
Put p = (Put) m;
p.add(cell);
} else if (m instanceof Delete) {
} else {
Delete d = (Delete) m;
d.add(cell);
}
Expand Down Expand Up @@ -470,35 +467,6 @@ private Pair<Boolean, Tag> checkForReservedVisibilityTagPresence(Cell cell,
return pair;
}

/**
* Checks whether cell contains any tag with type as VISIBILITY_TAG_TYPE. This
* tag type is reserved and should not be explicitly set by user. There are
* two versions of this method one that accepts pair and other without pair.
* In case of preAppend and preIncrement the additional operations are not
* needed like checking for STRING_VIS_TAG_TYPE and hence the API without pair
* could be used.
*
* @param cell
* @throws IOException
*/
private boolean checkForReservedVisibilityTagPresence(Cell cell) throws IOException {
// Bypass this check when the operation is done by a system/super user.
// This is done because, while Replication, the Cells coming to the peer
// cluster with reserved
// typed tags and this is fine and should get added to the peer cluster
// table
if (isSystemOrSuperUser()) {
return true;
}
Iterator<Tag> tagsItr = PrivateCellUtil.tagsIterator(cell);
while (tagsItr.hasNext()) {
if (RESERVED_VIS_TAG_TYPES.contains(tagsItr.next().getType())) {
return false;
}
}
return true;
}

private void removeReplicationVisibilityTag(List<Tag> tags) throws IOException {
Iterator<Tag> iterator = tags.iterator();
while (iterator.hasNext()) {
Expand Down Expand Up @@ -657,36 +625,6 @@ private boolean isSystemOrSuperUser() throws IOException {
return Superusers.isSuperUser(VisibilityUtils.getActiveUser());
}

@Override
public Result preAppend(ObserverContext<RegionCoprocessorEnvironment> e, Append append)
throws IOException {
// If authorization is not enabled, we don't care about reserved tags
if (!authorizationEnabled) {
return null;
}
for (CellScanner cellScanner = append.cellScanner(); cellScanner.advance();) {
if (!checkForReservedVisibilityTagPresence(cellScanner.current())) {
throw new FailedSanityCheckException("Append contains cell with reserved type tag");
}
}
return null;
}

@Override
public Result preIncrement(ObserverContext<RegionCoprocessorEnvironment> e, Increment increment)
throws IOException {
// If authorization is not enabled, we don't care about reserved tags
if (!authorizationEnabled) {
return null;
}
for (CellScanner cellScanner = increment.cellScanner(); cellScanner.advance();) {
if (!checkForReservedVisibilityTagPresence(cellScanner.current())) {
throw new FailedSanityCheckException("Increment contains cell with reserved type tag");
}
}
return null;
}

@Override
public List<Pair<Cell, Cell>> postIncrementBeforeWAL(
ObserverContext<RegionCoprocessorEnvironment> ctx, Mutation mutation,
Expand Down