From 4dbe5a8eb3fd9c446b5d5a763d916437d567a775 Mon Sep 17 00:00:00 2001 From: Ashish Singhi Date: Tue, 13 Jan 2015 15:48:34 +0530 Subject: [PATCH] HBASE-12831 Changing the set of vis labels a user has access to doesn't generate an audit log event --- conf/log4j.properties | 1 + .../security/visibility/VisibilityController.java | 88 +++++++++++++++++++--- 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/conf/log4j.properties b/conf/log4j.properties index 4e8f145..472fc03 100644 --- a/conf/log4j.properties +++ b/conf/log4j.properties @@ -55,6 +55,7 @@ log4j.appender.RFAS.layout.ConversionPattern=%d{ISO8601} %p %c: %m%n log4j.category.SecurityLogger=${hbase.security.logger} log4j.additivity.SecurityLogger=false #log4j.logger.SecurityLogger.org.apache.hadoop.hbase.security.access.AccessController=TRACE +#log4j.logger.SecurityLogger.org.apache.hadoop.hbase.security.visibility.VisibilityController=TRACE # # Null Appender diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java index 9deeca3..550e71c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java @@ -24,6 +24,7 @@ import static org.apache.hadoop.hbase.security.visibility.VisibilityConstants.LA import static org.apache.hadoop.hbase.security.visibility.VisibilityConstants.LABELS_TABLE_NAME; import java.io.IOException; +import java.net.InetAddress; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -38,15 +39,16 @@ import org.apache.hadoop.hbase.CellScanner; import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.DoNotRetryIOException; -import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HTableDescriptor; -import org.apache.hadoop.hbase.TagRewriteCell; +import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.Tag; -import org.apache.hadoop.hbase.MetaTableAccessor; +import org.apache.hadoop.hbase.TagRewriteCell; import org.apache.hadoop.hbase.TagType; +import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.client.Append; import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Get; @@ -97,7 +99,6 @@ import org.apache.hadoop.hbase.replication.ReplicationEndpoint; import org.apache.hadoop.hbase.security.AccessDeniedException; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.AccessControlLists; -import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.security.access.AccessController; import org.apache.hadoop.hbase.util.ByteStringer; import org.apache.hadoop.hbase.util.Bytes; @@ -119,6 +120,8 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements VisibilityLabelsService.Interface, CoprocessorService { private static final Log LOG = LogFactory.getLog(VisibilityController.class); + private static final Log AUDITLOG = LogFactory.getLog("SecurityLogger." + + VisibilityController.class.getName()); // flags if we are running on a region of the 'labels' table private boolean labelsRegion = false; // Flag denoting whether AcessController is available or not. @@ -726,9 +729,9 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements new VisibilityControllerNotReadyException("VisibilityController not yet initialized!"), response); } else { + List labels = new ArrayList(visLabels.size()); try { checkCallingUserAuth(); - List labels = new ArrayList(visLabels.size()); RegionActionResult successResult = RegionActionResult.newBuilder().build(); for (VisibilityLabel visLabel : visLabels) { byte[] label = visLabel.getLabel().toByteArray(); @@ -739,6 +742,7 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements } if (!labels.isEmpty()) { OperationStatus[] opStatus = this.visibilityLabelService.addLabels(labels); + logResult(true, "addLabels", "Adding labels allowed", null, labels, null); int i = 0; for (OperationStatus status : opStatus) { while (response.getResult(i) != successResult) @@ -752,6 +756,10 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements i++; } } + } catch (AccessDeniedException e) { + logResult(false, "addLabels", e.getMessage(), null, labels, null); + LOG.error(e); + setExceptionResults(visLabels.size(), e, response); } catch (IOException e) { LOG.error(e); setExceptionResults(visLabels.size(), e, response); @@ -780,14 +788,17 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements new VisibilityControllerNotReadyException("VisibilityController not yet initialized!"), response); } else { + byte[] user = request.getUser().toByteArray(); + List labelAuths = new ArrayList(auths.size()); try { checkCallingUserAuth(); - List labelAuths = new ArrayList(auths.size()); + for (ByteString authBS : auths) { labelAuths.add(authBS.toByteArray()); } - OperationStatus[] opStatus = this.visibilityLabelService.setAuths(request.getUser() - .toByteArray(), labelAuths); + OperationStatus[] opStatus = this.visibilityLabelService.setAuths(user, labelAuths); + logResult(true, "setAuths", "Setting authorization for labels allowed", user, labelAuths, + null); RegionActionResult successResult = RegionActionResult.newBuilder().build(); for (OperationStatus status : opStatus) { if (status.getOperationStatusCode() == SUCCESS) { @@ -799,6 +810,10 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements response.addResult(failureResultBuilder.build()); } } + } catch (AccessDeniedException e) { + logResult(false, "setAuths", e.getMessage(), user, labelAuths, null); + LOG.error(e); + setExceptionResults(auths.size(), e, response); } catch (IOException e) { LOG.error(e); setExceptionResults(auths.size(), e, response); @@ -807,6 +822,39 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements done.run(response.build()); } + private void logResult(boolean isAllowed, String request, String reason, byte[] user, + List labelAuths, String regex) { + if (AUDITLOG.isTraceEnabled()) { + RequestContext ctx = RequestContext.get(); + InetAddress remoteAddr = null; + if (ctx != null) { + remoteAddr = ctx.getRemoteAddress(); + } + + List labelAuthsStr = new ArrayList<>(); + if (labelAuths != null) { + int labelAuthsSize = labelAuths.size(); + labelAuthsStr = new ArrayList<>(labelAuthsSize); + for (int i = 0; i < labelAuthsSize; i++) { + labelAuthsStr.add(Bytes.toString(labelAuths.get(i))); + } + } + + User requestingUser = null; + try { + requestingUser = VisibilityUtils.getActiveUser(); + } catch (IOException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } + AUDITLOG.trace("Access " + (isAllowed ? "allowed" : "denied") + " for user " + + (requestingUser != null ? requestingUser.getShortName() : "UNKNOWN") + "; reason: " + + reason + "; remote address: " + (remoteAddr != null ? remoteAddr : "") + "; request: " + + request + "; user: " + (user != null ? Bytes.toShort(user) : "null") + "; labels: " + + labelAuthsStr + "; regex: " + regex); + } + } + @Override public synchronized void getAuths(RpcController controller, GetAuthsRequest request, RpcCallback done) { @@ -826,6 +874,10 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements + "' is not authorized to perform this action."); } labels = this.visibilityLabelService.getAuths(user, false); + logResult(true, "getAuths", "Get authorizations for user allowed", user, null, null); + } catch (AccessDeniedException e) { + logResult(false, "getAuths", e.getMessage(), user, null, null); + ResponseConverter.setControllerException(controller, e); } catch (IOException e) { ResponseConverter.setControllerException(controller, e); } @@ -848,6 +900,8 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements setExceptionResults(auths.size(), new CoprocessorException( "VisibilityController not yet initialized"), response); } else { + byte[] requestUser = request.getUser().toByteArray(); + List labelAuths = new ArrayList(auths.size()); try { // When AC is ON, do AC based user auth check if (this.acOn && !isSystemOrSuperUser()) { @@ -857,12 +911,14 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements } checkCallingUserAuth(); // When AC is not in place the calling user should have SYSTEM_LABEL // auth to do this action. - List labelAuths = new ArrayList(auths.size()); for (ByteString authBS : auths) { labelAuths.add(authBS.toByteArray()); } - OperationStatus[] opStatus = this.visibilityLabelService.clearAuths(request.getUser() - .toByteArray(), labelAuths); + + OperationStatus[] opStatus = + this.visibilityLabelService.clearAuths(requestUser, labelAuths); + logResult(true, "clearAuths", "Removing authorization for labels allowed", requestUser, + labelAuths, null); RegionActionResult successResult = RegionActionResult.newBuilder().build(); for (OperationStatus status : opStatus) { if (status.getOperationStatusCode() == SUCCESS) { @@ -874,6 +930,10 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements response.addResult(failureResultBuilder.build()); } } + } catch (AccessDeniedException e) { + logResult(false, "clearAuths", e.getMessage(), requestUser, labelAuths, null); + LOG.error(e); + setExceptionResults(auths.size(), e, response); } catch (IOException e) { LOG.error(e); setExceptionResults(auths.size(), e, response); @@ -890,6 +950,7 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements controller.setFailed("VisibilityController not yet initialized"); } else { List labels = null; + String regex = request.hasRegex() ? request.getRegex() : null; try { // We do ACL check here as we create scanner directly on region. It will not make calls to // AccessController CP methods. @@ -899,8 +960,11 @@ public class VisibilityController extends BaseMasterAndRegionObserver implements + (requestingUser != null ? requestingUser.getShortName() : "null") + "' is not authorized to perform this action."); } - String regex = request.hasRegex() ? request.getRegex() : null; labels = this.visibilityLabelService.listLabels(regex); + logResult(false, "listLabels", "Listing labels allowed", null, null, regex); + } catch (AccessDeniedException e) { + logResult(false, "listLabels", e.getMessage(), null, null, regex); + ResponseConverter.setControllerException(controller, e); } catch (IOException e) { ResponseConverter.setControllerException(controller, e); } -- 1.9.2.msysgit.0