You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/06/13 23:46:59 UTC
[impala] 02/02: IMPALA-8654: Log the SQL statement in the Ranger
audit log
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 11b18d7228a18e8609aa9ed8ff04f6784b30cb74
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Tue Jun 11 15:58:33 2019 -0700
IMPALA-8654: Log the SQL statement in the Ranger audit log
This patch logs the SQL statement to be authorized in the Ranger audit
log since it was a required field prior to the fix in RANGER-2463 to
avoid a NullPointerException in the Ranger admin that could cause the
Ranger audit logs to not show up in the Ranger web UI.
Testing:
- Updated the RangerAuditLogTest
- Tested the changes against Solr cluster
- Ran all FE tests
Change-Id: Id9f584ac4209604675eb13b6d6f349c6cbb1a387
Reviewed-on: http://gerrit.cloudera.org:8080/13590
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../apache/impala/analysis/AnalysisContext.java | 3 ++-
.../org/apache/impala/analysis/CreateDbStmt.java | 4 +--
.../impala/authorization/AuthorizationChecker.java | 3 ++-
.../authorization/BaseAuthorizationChecker.java | 3 ++-
.../authorization/NoopAuthorizationFactory.java | 3 ++-
.../ranger/RangerAuthorizationChecker.java | 12 ++++++---
.../ranger/RangerBufferAuditHandler.java | 19 +++++++++++++-
.../sentry/SentryAuthorizationChecker.java | 3 ++-
.../authorization/ranger/RangerAuditLogTest.java | 29 ++++++++++++++++++++++
.../org/apache/impala/common/FrontendTestBase.java | 4 ++-
10 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
index cdd456c..47ee010 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -420,7 +420,8 @@ public class AnalysisContext {
AuthorizationException authException = null;
AuthorizationContext authzCtx = null;
try {
- authzCtx = authzChecker.createAuthorizationContext(true);
+ authzCtx = authzChecker.createAuthorizationContext(true,
+ queryCtx_.client_request.stmt);
authzChecker.authorize(authzCtx, analysisResult_, catalog_);
} catch (AuthorizationException e) {
authException = e;
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
index 2b52278..c0e5cdf 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
@@ -66,8 +66,8 @@ public class CreateDbStmt extends StatementBase {
@Override
public String toSql(ToSqlOptions options) {
- StringBuilder sb = new StringBuilder("CREATE DATABASE");
- if (ifNotExists_) sb.append(" IF NOT EXISTS");
+ StringBuilder sb = new StringBuilder("CREATE DATABASE ");
+ if (ifNotExists_) sb.append("IF NOT EXISTS");
sb.append(dbName_);
if (comment_ != null) sb.append(" COMMENT '" + comment_ + "'");
if (location_ != null) sb.append(" LOCATION '" + location_ + "'");
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
index 454cad3..2b8525c 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
+++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
@@ -39,8 +39,9 @@ public interface AuthorizationChecker {
* created per authorization execution.
*
* @param doAudits a flag whether or not to do the audits
+ * @param sqlStmt the SQL statement to be logged for auditing
*/
- AuthorizationContext createAuthorizationContext(boolean doAudits);
+ AuthorizationContext createAuthorizationContext(boolean doAudits, String sqlStmt);
/**
* Authorize an analyzed statement.
diff --git a/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java b/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
index 02cb9cb..83975c6 100644
--- a/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
+++ b/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
@@ -63,7 +63,8 @@ public abstract class BaseAuthorizationChecker implements AuthorizationChecker {
// We don't want to do an audit log here. This method is used by "show databases",
// "show tables", "describe" to filter out unauthorized database, table, or column
// names.
- return hasAccess(createAuthorizationContext(false), user, request);
+ return hasAccess(createAuthorizationContext(false /*no audit log*/,
+ null /*no SQL statement*/), user, request);
}
private boolean hasAccess(AuthorizationContext authzCtx, User user,
diff --git a/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java b/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
index 6f6a571..1be26e7 100644
--- a/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
+++ b/fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java
@@ -204,7 +204,8 @@ public class NoopAuthorizationFactory implements AuthorizationFactory {
public void invalidateAuthorizationCache() {}
@Override
- public AuthorizationContext createAuthorizationContext(boolean doAudits) {
+ public AuthorizationContext createAuthorizationContext(boolean doAudits,
+ String sqlStmt) {
return new AuthorizationContext();
}
};
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 98ba01a..3a553cc 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
@@ -206,11 +206,16 @@ public class RangerAuthorizationChecker extends BaseAuthorizationChecker {
}
@Override
- public AuthorizationContext createAuthorizationContext(boolean doAudits) {
+ public AuthorizationContext createAuthorizationContext(boolean doAudits,
+ String sqlStmt) {
RangerAuthorizationContext authzCtx = new RangerAuthorizationContext();
if (doAudits) {
// Any statement that goes through {@link authorize} will need to have audit logs.
- authzCtx.setAuditHandler(new RangerBufferAuditHandler());
+ if (sqlStmt != null) {
+ authzCtx.setAuditHandler(new RangerBufferAuditHandler(sqlStmt));
+ } else {
+ authzCtx.setAuditHandler(new RangerBufferAuditHandler());
+ }
}
return authzCtx;
}
@@ -226,7 +231,8 @@ public class RangerAuthorizationChecker extends BaseAuthorizationChecker {
// case 4: table (select) ERROR, columns (select) ERROR --> only add the first column
// event
RangerAuthorizationContext tmpCtx = new RangerAuthorizationContext();
- tmpCtx.setAuditHandler(new RangerBufferAuditHandler());
+ tmpCtx.setAuditHandler(new RangerBufferAuditHandler(
+ originalCtx.getAuditHandler().getSqlStmt()));
try {
super.authorizeTableAccess(tmpCtx, analysisResult, catalog, requests);
} catch (AuthorizationException e) {
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 8beefa3..8fe0a96 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
@@ -41,21 +41,37 @@ import java.util.Optional;
public class RangerBufferAuditHandler implements RangerAccessResultProcessor {
private final RangerDefaultAuditHandler auditHandler_ = new RangerDefaultAuditHandler();
private final List<AuthzAuditEvent> auditEvents_ = new ArrayList<>();
+ private final String sqlStmt_; // The SQL statement to be logged
+
+ public RangerBufferAuditHandler() {
+ // This can be empty but should not be null to avoid NPE. See RANGER-2463.
+ this("");
+ }
+
+ public RangerBufferAuditHandler(String sqlStmt) {
+ sqlStmt_= sqlStmt;
+ }
public static class AutoFlush extends RangerBufferAuditHandler
implements AutoCloseable {
+ public AutoFlush(String sqlStmt) {
+ super(sqlStmt);
+ }
+
@Override
public void close() {
super.flush();
}
}
+ public String getSqlStmt() { return sqlStmt_; }
+
/**
* Creates an instance of {@link RangerBufferAuditHandler} that will do an auto-flush.
* Use it with try-resource.
*/
public static AutoFlush autoFlush() {
- return new AutoFlush();
+ return new AutoFlush("");
}
@Override
@@ -90,6 +106,7 @@ public class RangerBufferAuditHandler implements RangerAccessResultProcessor {
AuthzAuditEvent auditEvent = auditHandler_.getAuthzEvents(result);
auditEvent.setAccessType(request.getAccessType());
+ auditEvent.setRequestData(sqlStmt_);
auditEvent.setResourcePath(resource != null ? resource.getAsString() : null);
if (resourceType != null) {
auditEvent.setResourceType("@" + resourceType);
diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
index 288a3e5..82cd9ab 100644
--- a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
@@ -87,7 +87,8 @@ public class SentryAuthorizationChecker extends BaseAuthorizationChecker {
}
@Override
- public AuthorizationContext createAuthorizationContext(boolean doAudits) {
+ public AuthorizationContext createAuthorizationContext(boolean doAudits,
+ String sqlStmt) {
return new AuthorizationContext();
}
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 9521995..c0837aa 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
@@ -62,11 +62,14 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
authzOk(events -> {
assertEquals(1, events.size());
assertEventEquals("@database", "create", "test_db", 1, events.get(0));
+ assertEquals("create database test_db", events.get(0).getRequestData());
}, "create database test_db", onServer(TPrivilegeLevel.CREATE));
authzOk(events -> {
assertEquals(1, events.size());
assertEventEquals("@table", "create", "functional/test_tbl", 1, events.get(0));
+ assertEquals("create table functional.test_tbl(i int)",
+ events.get(0).getRequestData());
}, "create table functional.test_tbl(i int)", onDatabase("functional",
TPrivilegeLevel.CREATE));
@@ -75,6 +78,9 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
assertEventEquals("@udf", "create", "functional/f()", 1, events.get(0));
assertEventEquals("@url", "all",
"hdfs://localhost:20500/test-warehouse/libTestUdfs.so", 1, events.get(1));
+ assertEquals("create function functional.f() returns int location " +
+ "'hdfs://localhost:20500/test-warehouse/libTestUdfs.so' symbol='NoArgs'",
+ events.get(0).getRequestData());
}, "create function functional.f() returns int location " +
"'hdfs://localhost:20500/test-warehouse/libTestUdfs.so' symbol='NoArgs'",
onDatabase("functional", TPrivilegeLevel.CREATE),
@@ -86,6 +92,9 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
assertEventEquals("@table", "create", "functional/new_table", 1, events.get(0));
assertEventEquals("@url", "all", "hdfs://localhost:20500/test-warehouse/new_table",
1, events.get(1));
+ assertEquals("create table functional.new_table(i int) location " +
+ "'hdfs://localhost:20500/test-warehouse/new_table'",
+ events.get(0).getRequestData());
}, "create table functional.new_table(i int) location " +
"'hdfs://localhost:20500/test-warehouse/new_table'",
onDatabase("functional", TPrivilegeLevel.CREATE),
@@ -95,6 +104,8 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
// Only the table event.
assertEquals(1, events.size());
assertEventEquals("@table", "select", "functional/alltypes", 1, events.get(0));
+ assertEquals("select id, string_col from functional.alltypes",
+ events.get(0).getRequestData());
}, "select id, string_col from functional.alltypes",
onTable("functional", "alltypes", TPrivilegeLevel.SELECT));
@@ -103,8 +114,12 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
// short circuiting.
assertEquals(2, events.size());
assertEventEquals("@column", "select", "functional/alltypes/id", 1, events.get(0));
+ assertEquals("select id, string_col from functional.alltypes",
+ events.get(0).getRequestData());
assertEventEquals("@column", "select", "functional/alltypes/string_col", 1,
events.get(1));
+ assertEquals("select id, string_col from functional.alltypes",
+ events.get(1).getRequestData());
}, "select id, string_col from functional.alltypes",
onColumn("functional", "alltypes", "id", TPrivilegeLevel.SELECT),
onColumn("functional", "alltypes", "string_col", TPrivilegeLevel.SELECT));
@@ -112,8 +127,11 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
authzOk(events -> {
assertEquals(3, events.size());
assertEventEquals("@column", "refresh", "*/*/*", 1, events.get(0));
+ assertEquals("invalidate metadata", events.get(0).getRequestData());
assertEventEquals("@udf", "refresh", "*/*", 1, events.get(1));
+ assertEquals("invalidate metadata", events.get(1).getRequestData());
assertEventEquals("@url", "refresh", "*", 1, events.get(2));
+ assertEquals("invalidate metadata", events.get(2).getRequestData());
}, "invalidate metadata", onServer(TPrivilegeLevel.REFRESH));
}
@@ -122,17 +140,23 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
authzError(events -> {
assertEquals(1, events.size());
assertEventEquals("@database", "create", "test_db", 0, events.get(0));
+ assertEquals("create database test_db", events.get(0).getRequestData());
}, "create database test_db");
authzError(events -> {
assertEquals(1, events.size());
assertEventEquals("@table", "create", "functional/test_tbl", 0, events.get(0));
+ assertEquals("create table functional.test_tbl(i int)",
+ events.get(0).getRequestData());
}, "create table functional.test_tbl(i int)");
authzError(events -> {
// Only log first the first failure.
assertEquals(1, events.size());
assertEventEquals("@udf", "create", "functional/f()", 0, events.get(0));
+ assertEquals("create function functional.f() returns int location " +
+ "'hdfs://localhost:20500/test-warehouse/libTestUdfs.so' symbol='NoArgs'",
+ events.get(0).getRequestData());
}, "create function functional.f() returns int location " +
"'hdfs://localhost:20500/test-warehouse/libTestUdfs.so' symbol='NoArgs'");
@@ -140,6 +164,9 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
assertEquals(1, events.size());
assertEventEquals("@url", "all", "hdfs://localhost:20500/test-warehouse/new_table",
0, events.get(0));
+ assertEquals("create table functional.new_table(i int) location " +
+ "'hdfs://localhost:20500/test-warehouse/new_table'",
+ events.get(0).getRequestData());
}, "create table functional.new_table(i int) location " +
"'hdfs://localhost:20500/test-warehouse/new_table'",
onDatabase("functional", TPrivilegeLevel.CREATE));
@@ -149,6 +176,8 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
// short-circuiting.
assertEquals(1, events.size());
assertEventEquals("@column", "select", "functional/alltypes/id", 0, events.get(0));
+ assertEquals("select id, string_col from functional.alltypes",
+ events.get(0).getRequestData());
}, "select id, string_col from functional.alltypes");
}
diff --git a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
index b08ecf1..b3dad6a 100644
--- a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
+++ b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
@@ -297,6 +297,7 @@ public class FrontendTestBase extends AbstractFrontendTest {
protected AnalysisResult parseAndAnalyze(String stmt, AnalysisContext ctx, Frontend fe)
throws ImpalaException {
+ ctx.getQueryCtx().getClient_request().setStmt(stmt);
StatementBase parsedStmt = Parser.parse(stmt, ctx.getQueryOptions());
StmtMetadataLoader mdLoader =
new StmtMetadataLoader(fe, ctx.getQueryCtx().session.database, null);
@@ -374,7 +375,8 @@ public class FrontendTestBase extends AbstractFrontendTest {
public void invalidateAuthorizationCache() {}
@Override
- public AuthorizationContext createAuthorizationContext(boolean doAudits) {
+ public AuthorizationContext createAuthorizationContext(boolean doAudits,
+ String sqlStmt) {
return new AuthorizationContext();
}
};