You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by fr...@apache.org on 2019/07/02 14:02:20 UTC
[impala] 03/03: IMPALA-8716: Log a group of privileges into a
single audit event.
This is an automated email from the ASF dual-hosted git repository.
fredyw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit aee0abd76b762e57ce9f0a2e40a9a8b4f97dc986
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Wed Jun 26 16:30:50 2019 -0700
IMPALA-8716: Log a group of privileges into a single audit event.
This patch updates the audit log handler to group a privilege that
consists of multiple privileges into a single audit event.
For example if we run "show partitions foo.bar" and we have
SELECT privilege on table "foo.bar", before this patch, we would be
creating 2 audit events:
- Attempt to check if there's INSERT privilege on table "foo.bar"
Result: denied, access type: insert, resource: foo.bar
- Attempt to check if there's SELECT privilege on table "foo.bar"
Result: allowed, access type: select, resource: foo.bar
After this patch, we will only create a single audit event, e.g.
Result: allowed, access type: view_metadata, resource: foo.bar
Testing:
- Updated tests in RangerAuditLogTest
- Ran FE tests
Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Reviewed-on: http://gerrit.cloudera.org:8080/13744
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../ranger/RangerAuthorizationChecker.java | 96 ++++++++++++++++------
.../ranger/RangerBufferAuditHandler.java | 10 +++
.../authorization/ranger/RangerAuditLogTest.java | 19 ++++-
3 files changed, 101 insertions(+), 24 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
index 7f76cad..9a4e67d 100644
--- a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
+++ b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
@@ -151,15 +151,14 @@ public class RangerAuthorizationChecker extends BaseAuthorizationChecker {
for (RangerAccessResourceImpl resource: resources) {
if (request.getPrivilege() == Privilege.ANY) {
- if (!authorizeResource(rangerAuthzCtx, resource, user, request.getPrivilege())) {
+ if (!authorizeResource(rangerAuthzCtx, resource, user, request.getPrivilege(),
+ ((RangerAuthorizationContext) authzCtx).getAuditHandler())) {
return false;
}
} else {
boolean authorized = request.getPrivilege().hasAnyOf() ?
- authorizeAny(rangerAuthzCtx, resource, user,
- request.getPrivilege().getImpliedPrivileges()) :
- authorizeAll(rangerAuthzCtx, resource, user,
- request.getPrivilege().getImpliedPrivileges());
+ authorizeAny(rangerAuthzCtx, resource, user, request.getPrivilege()) :
+ authorizeAll(rangerAuthzCtx, resource, user, request.getPrivilege());
if (!authorized) {
return false;
}
@@ -240,9 +239,7 @@ public class RangerAuthorizationChecker extends BaseAuthorizationChecker {
// event
RangerAuthorizationContext tmpCtx = new RangerAuthorizationContext(
originalCtx.getSessionState(), originalCtx.getTimeline());
- tmpCtx.setAuditHandler(new RangerBufferAuditHandler(
- originalAuditHandler.getSqlStmt(), originalAuditHandler.getClusterName(),
- originalAuditHandler.getClientIp()));
+ tmpCtx.setAuditHandler(new RangerBufferAuditHandler(originalAuditHandler));
try {
super.authorizeTableAccess(tmpCtx, analysisResult, catalog, requests);
} catch (AuthorizationException e) {
@@ -314,30 +311,84 @@ public class RangerAuthorizationChecker extends BaseAuthorizationChecker {
}
private boolean authorizeAny(RangerAuthorizationContext authzCtx,
- RangerAccessResourceImpl resource, User user, EnumSet<Privilege> privileges)
+ RangerAccessResourceImpl resource, User user, Privilege privilege)
throws InternalException {
- for (Privilege privilege: privileges) {
- if (authorizeResource(authzCtx, resource, user, privilege)) {
- return true;
+ boolean authorized = false;
+ RangerBufferAuditHandler originalAuditHandler = authzCtx.getAuditHandler();
+ // Use a temporary audit handler instead of the original audit handler
+ // so that we know all the audit events generated by the temporary audit
+ // handler are contained to the given resource.
+ RangerBufferAuditHandler tmpAuditHandler = originalAuditHandler == null ?
+ null : new RangerBufferAuditHandler(originalAuditHandler);
+ for (Privilege impliedPrivilege: privilege.getImpliedPrivileges()) {
+ if (authorizeResource(authzCtx, resource, user, impliedPrivilege,
+ tmpAuditHandler)) {
+ authorized = true;
+ break;
}
}
- return false;
+ if (originalAuditHandler != null) {
+ updateAuditEvents(tmpAuditHandler, originalAuditHandler, true /*any*/,
+ privilege);
+ }
+ return authorized;
}
private boolean authorizeAll(RangerAuthorizationContext authzCtx,
- RangerAccessResourceImpl resource, User user, EnumSet<Privilege> privileges)
+ RangerAccessResourceImpl resource, User user, Privilege privilege)
throws InternalException {
- for (Privilege privilege: privileges) {
- if (!authorizeResource(authzCtx, resource, user, privilege)) {
- return false;
+ boolean authorized = true;
+ RangerBufferAuditHandler originalAuditHandler = authzCtx.getAuditHandler();
+ // Use a temporary audit handler instead of the original audit handler
+ // so that we know all the audit events generated by the temporary audit
+ // handler are contained to the given resource.
+ RangerBufferAuditHandler tmpAuditHandler = originalAuditHandler == null ?
+ null : new RangerBufferAuditHandler(originalAuditHandler);
+ for (Privilege impliedPrivilege: privilege.getImpliedPrivileges()) {
+ if (!authorizeResource(authzCtx, resource, user, impliedPrivilege,
+ tmpAuditHandler)) {
+ authorized = false;
+ break;
}
}
- return true;
+ if (originalAuditHandler != null) {
+ updateAuditEvents(tmpAuditHandler, originalAuditHandler, false /*not any*/,
+ privilege);
+ }
+ return authorized;
+ }
+
+ /**
+ * Updates the {@link AuthzAuditEvent} from the source to the destination
+ * {@link RangerBufferAuditHandler} with the given privilege.
+ *
+ * For example we want to log:
+ * VIEW_METADATA - Allowed
+ * vs
+ * INSERT - Denied, INSERT - Allowed.
+ */
+ private static void updateAuditEvents(RangerBufferAuditHandler source,
+ RangerBufferAuditHandler dest, boolean isAny, Privilege privilege) {
+ // A custom audit event to log the actual privilege instead of the implied
+ // privileges.
+ AuthzAuditEvent newAuditEvent = null;
+ for (AuthzAuditEvent auditEvent : source.getAuthzEvents()) {
+ if (auditEvent.getAccessResult() == (isAny ? 1 : 0)) {
+ newAuditEvent = auditEvent;
+ break; // break if at least one is a success.
+ } else {
+ newAuditEvent = auditEvent;
+ }
+ }
+ if (newAuditEvent != null) {
+ newAuditEvent.setAccessType(privilege.name().toLowerCase());
+ dest.getAuthzEvents().add(newAuditEvent);
+ }
}
private boolean authorizeResource(RangerAuthorizationContext authzCtx,
- RangerAccessResourceImpl resource, User user, Privilege privilege)
- throws InternalException {
+ RangerAccessResourceImpl resource, User user, Privilege privilege,
+ RangerBufferAuditHandler auditHandler) throws InternalException {
String accessType;
if (privilege == Privilege.ANY) {
accessType = RangerPolicyEngine.ANY_ACCESS;
@@ -354,9 +405,8 @@ public class RangerAuthorizationChecker extends BaseAuthorizationChecker {
request.setClientIPAddress(
authzCtx.getSessionState().getNetwork_address().getHostname());
}
- RangerAccessResult result = plugin_.isAccessAllowed(request,
- authzCtx.getAuditHandler());
- return result != null && result.getIsAllowed();
+ RangerAccessResult authorized = plugin_.isAccessAllowed(request, auditHandler);
+ return authorized != null && authorized.getIsAllowed();
}
@VisibleForTesting
diff --git a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java
index 4b192c5..b922080 100644
--- a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java
+++ b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java
@@ -61,6 +61,16 @@ public class RangerBufferAuditHandler implements RangerAccessResultProcessor {
clientIp_ = Preconditions.checkNotNull(clientIp);
}
+ /**
+ * A copy constructor for {@link RangerBufferAuditHandler}.
+ */
+ public RangerBufferAuditHandler(RangerBufferAuditHandler auditHandler) {
+ Preconditions.checkNotNull(auditHandler);
+ sqlStmt_ = Preconditions.checkNotNull(auditHandler.getSqlStmt());
+ clusterName_ = Preconditions.checkNotNull(auditHandler.getClusterName());
+ clientIp_ = Preconditions.checkNotNull(auditHandler.getClientIp());
+ }
+
public static class AutoFlush extends RangerBufferAuditHandler
implements AutoCloseable {
public AutoFlush(String sqlStmt, String clusterName, String clientIp) {
diff --git a/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java b/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
index fb85d7b..1f4a6a4 100644
--- a/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
+++ b/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
@@ -134,6 +134,15 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
assertEventEquals("@url", "refresh", "*", 1, events.get(2));
assertEquals("invalidate metadata", events.get(2).getRequestData());
}, "invalidate metadata", onServer(TPrivilegeLevel.REFRESH));
+
+ authzOk(events -> {
+ // Show the actual view_metadata audit log.
+ assertEquals(1, events.size());
+ assertEventEquals("@table", "view_metadata", "functional/alltypes", 1,
+ events.get(0));
+ assertEquals("show partitions functional.alltypes", events.get(0).getRequestData());
+ }, "show partitions functional.alltypes",
+ onTable("functional", "alltypes", TPrivilegeLevel.SELECT));
}
@Test
@@ -180,6 +189,14 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
assertEquals("select id, string_col from functional.alltypes",
events.get(0).getRequestData());
}, "select id, string_col from functional.alltypes");
+
+ authzError(events -> {
+ // Show the actual view_metadata audit log.
+ assertEquals(1, events.size());
+ assertEventEquals("@table", "view_metadata", "functional/alltypes", 0,
+ events.get(0));
+ assertEquals("show partitions functional.alltypes", events.get(0).getRequestData());
+ }, "show partitions functional.alltypes");
}
private void authzOk(Consumer<List<AuthzAuditEvent>> resultChecker, String stmt,
@@ -201,7 +218,7 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
private static void assertEventEquals(String resourceType, String accessType,
String resourcePath, int accessResult, AuthzAuditEvent event) {
assertEquals(resourceType, event.getResourceType());
- assertEquals(accessType.toUpperCase(), event.getAccessType());
+ assertEquals(accessType, event.getAccessType());
assertEquals(resourcePath, event.getResourcePath());
assertEquals(accessResult, event.getAccessResult());
assertEquals("test-cluster", event.getClusterName());