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/05/14 22:43:03 UTC
[impala] 03/06: IMPALA-6086: Use of permanent function should
require SELECT privilege on DB
This is an automated email from the ASF dual-hosted git repository.
tarmstrong pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 2e720ace8b285ae6a3b6b5ebc63dcfd04a763ca1
Author: Zoram Thanga <zo...@cloudera.com>
AuthorDate: Mon Jul 2 13:43:50 2018 -0700
IMPALA-6086: Use of permanent function should require SELECT privilege
on DB
To use a permanent UDF should require at least SELECT privilege on the
database. Functions that have constant arguments get constant-folded
into string literals, losing their privilege requests in the process.
This patch saves the privilege requests found during the first phase
of query analysis, where all the objects and the privileges required
to access them are identified. The requests are added back to the
new analyzer created for re-analysis post expression rewrite.
Testing:
New FE test cases have been added to AuthorizationStmtTest.
Manual tests were also done to identify the bug, as well as to test
the fix.
Ran exhaustive and covering tests.
Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57
Reviewed-on: http://gerrit.cloudera.org:8080/10850
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../apache/impala/analysis/AnalysisContext.java | 13 +++++++
.../impala/analysis/AuthorizationStmtTest.java | 41 +++++++++++++++++++---
2 files changed, 50 insertions(+), 4 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 5ed791e..9ad0807 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -51,6 +51,7 @@ import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import com.google.common.collect.ImmutableList;
/**
* Wrapper class for parsing, analyzing and rewriting a SQL stmt.
@@ -461,6 +462,13 @@ public class AnalysisContext {
List<String> origColLabels =
Lists.newArrayList(analysisResult_.stmt_.getColLabels());
+ // Some expressions, such as function calls with constant arguments, can get
+ // folded into literals. Since literals do not require privilege requests, we
+ // must save the original privileges in order to not lose them during
+ // re-analysis.
+ ImmutableList<PrivilegeRequest> origPrivReqs =
+ analysisResult_.analyzer_.getPrivilegeReqs();
+
// Re-analyze the stmt with a new analyzer.
analysisResult_.analyzer_ = createAnalyzer(stmtTableCache);
analysisResult_.stmt_.reset();
@@ -472,6 +480,11 @@ public class AnalysisContext {
if (LOG.isTraceEnabled()) {
LOG.trace("rewrittenStmt: " + analysisResult_.stmt_.toSql());
}
+
+ // Restore privilege requests found during the first pass
+ for (PrivilegeRequest req : origPrivReqs) {
+ analysisResult_.analyzer_.registerPrivReq(req);
+ }
if (isExplain) analysisResult_.stmt_.setIsExplain();
Preconditions.checkState(!analysisResult_.requiresSubqueryRewrite());
}
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
index 5406f53..52e0952 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
@@ -41,6 +41,7 @@ import org.apache.impala.thrift.TFunctionBinaryType;
import org.apache.impala.thrift.TPrivilege;
import org.apache.impala.thrift.TPrivilegeLevel;
import org.apache.impala.thrift.TPrivilegeScope;
+import org.apache.impala.thrift.TQueryOptions;
import org.apache.impala.thrift.TResultRow;
import org.apache.impala.thrift.TTableName;
import org.apache.impala.util.SentryPolicyService;
@@ -2118,6 +2119,33 @@ public class AuthorizationStmtTest extends FrontendTestBase {
} finally {
removeFunction(fn);
}
+
+ // IMPALA-6086: Make sure use of a permanent function requires SELECT (or higher)
+ // privilege on the database, and expr rewrite/constant-folding preserves
+ // privilege requests for functions.
+ ArrayList<Type> argTypes = new ArrayList<Type>();
+ argTypes.add(Type.STRING);
+ fn = addFunction("functional", "to_lower", argTypes, Type.STRING,
+ "/test-warehouse/libTestUdf.so",
+ "_Z7ToLowerPN10impala_udf15FunctionContextERKNS_9StringValE");
+ try {
+ TQueryOptions options = new TQueryOptions();
+ options.setEnable_expr_rewrites(true);
+ for (AuthzTest test: new AuthzTest[] {
+ authorize("select functional.to_lower('ABCDEF')"),
+ // Also test with expression rewrite enabled.
+ authorize(createAnalysisCtx(options),
+ "select functional.to_lower('ABCDEF')")}) {
+ test.ok(onServer(TPrivilegeLevel.SELECT))
+ .ok(onDatabase("functional", TPrivilegeLevel.ALL))
+ .ok(onDatabase("functional", viewMetadataPrivileges()))
+ .error(accessError("functional"))
+ .error(accessError("functional"), onDatabase("functional",
+ allExcept(viewMetadataPrivileges())));
+ }
+ } finally {
+ removeFunction(fn);
+ }
}
// Convert TDescribeResult to list of strings.
@@ -2172,14 +2200,19 @@ public class AuthorizationStmtTest extends FrontendTestBase {
return "User '%s' does not have privileges to DROP functions in: " + object;
}
- private ScalarFunction addFunction(String db, String fnName) {
- ScalarFunction fn = ScalarFunction.createForTesting(db, fnName,
- new ArrayList<Type>(), Type.INT, "/dummy", "dummy.class", null,
- null, TFunctionBinaryType.NATIVE);
+ private ScalarFunction addFunction(String db, String fnName, ArrayList<Type> argTypes,
+ Type retType, String uriPath, String symbolName) {
+ ScalarFunction fn = ScalarFunction.createForTesting(db, fnName, argTypes, retType,
+ uriPath, symbolName, null, null, TFunctionBinaryType.NATIVE);
authzCatalog_.addFunction(fn);
return fn;
}
+ private ScalarFunction addFunction(String db, String fnName) {
+ return addFunction(db, fnName, new ArrayList<Type>(), Type.INT, "/dummy",
+ "dummy.class");
+ }
+
private void removeFunction(ScalarFunction fn) {
authzCatalog_.removeFunction(fn);
}