[
https://issues.apache.org/jira/browse/HIVE-26799?focusedWorklogId=830439&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-830439
]
ASF GitHub Bot logged work on HIVE-26799:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 01/Dec/22 19:29
Start Date: 01/Dec/22 19:29
Worklog Time Spent: 10m
Work Description: jfsii commented on code in PR #3821:
URL: https://github.com/apache/hive/pull/3821#discussion_r1037478327
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -12550,6 +12550,21 @@ private ParseResult
rewriteASTWithMaskAndFilter(TableMask tableMask, ASTNode ast
}
}
+ void gatherUserSuppliedFunctions(ASTNode ast) {
+ int tokenType = ast.getToken().getType();
+ if (tokenType == HiveParser.TOK_FUNCTION ||
+ tokenType == HiveParser.TOK_FUNCTIONDI ||
+ tokenType == HiveParser.TOK_FUNCTIONSTAR) {
+ if (ast.getChild(0).getType() == HiveParser.Identifier) {
+ // maybe user supplied
+ this.userSuppliedFunctions.add(ast.getChild(0).getText());
Review Comment:
I'm not sure of Hive's stance on this - but I usually only use this when
forced to. Like in a setter in which the in parameter is named the same as the
object variable. Feel free to ignore if you disagree.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -12550,6 +12550,21 @@ private ParseResult
rewriteASTWithMaskAndFilter(TableMask tableMask, ASTNode ast
}
}
+ void gatherUserSuppliedFunctions(ASTNode ast) {
+ int tokenType = ast.getToken().getType();
+ if (tokenType == HiveParser.TOK_FUNCTION ||
+ tokenType == HiveParser.TOK_FUNCTIONDI ||
+ tokenType == HiveParser.TOK_FUNCTIONSTAR) {
+ if (ast.getChild(0).getType() == HiveParser.Identifier) {
+ // maybe user supplied
Review Comment:
Go ahead and remove this comment, I'm not sure why I added it.
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/command/CommandAuthorizerV2.java:
##########
@@ -139,6 +139,13 @@ private static List<HivePrivilegeObject>
getHivePrivObjects(List<? extends Entit
continue;
}
+ if (privObject.getTyp() == Type.FUNCTION &&
!HiveConf.getBoolVar(SessionState.get().getConf(),
+ HiveConf.ConfVars.HIVE_AUTHORIZATION_FUNCTIONS_IN_VIEW) &&
hiveOpType == HiveOperationType.QUERY) {
+ if
(!sem.getUserSuppliedFunctions().contains(privObject.getFunctionName())) {
+ continue;
Review Comment:
Is it possible to add test cases that show the behavior in a couple of cases
(maybe similar to the authorization_view_*.q tests?). Ideally - I would like to
see a query with a user supplied udf with a table, a view, and see how it also
interacts with MVs. Ideally the q.out file would show which objects are being
sent for authorization so it can catch the regression on when it sends an extra
UDFs for auth (or doesn't send the user supplied one for auth).
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java:
##########
@@ -1438,6 +1442,10 @@ public void setUpdateColumnAccessInfo(ColumnAccessInfo
updateColumnAccessInfo) {
this.updateColumnAccessInfo = updateColumnAccessInfo;
}
+ public Set<String> getUserSuppliedFunctions() {
Review Comment:
Maybe add java doc with a note that it contains user supplied functions -
and note that it doesn't contain UDFs defined in views, etc.
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/command/CommandAuthorizerV2.java:
##########
@@ -98,7 +98,7 @@ private static void addPermanentFunctionEntities(SessionState
ss, List<ReadEntit
}
private static List<HivePrivilegeObject> getHivePrivObjects(List<? extends
Entity> privObjects,
- Map<String, List<String>> tableName2Cols, HiveOperationType hiveOpType)
throws HiveException {
+ Map<String, List<String>> tableName2Cols, HiveOperationType hiveOpType,
BaseSemanticAnalyzer sem) throws HiveException {
List<HivePrivilegeObject> hivePrivobjs = new
ArrayList<HivePrivilegeObject>();
if (privObjects == null){
Review Comment:
nit: If I was editing this method - I would maybe consider making the format
uniform within in it.
I.E. it seems to have at least 3 different coding styles.
if (conditional){
if(conditional){
if (conditional) {
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -3599,6 +3599,9 @@ public static enum ConfVars {
HIVE_AUTHORIZATION_TABLES_ON_STORAGEHANDLERS("hive.security.authorization.tables.on.storagehandlers",
true,
"Enables authorization on tables with custom storage handlers as
implemented by HIVE-24705. " +
"Default setting is true. Useful for turning the feature off if the
corresponding ranger patch is missing."),
+
HIVE_AUTHORIZATION_FUNCTIONS_IN_VIEW("hive.security.authorization.functions.in.view",
true, "" +
Review Comment:
Is the "" + needed?
Issue Time Tracking
-------------------
Worklog Id: (was: 830439)
Time Spent: 20m (was: 10m)
> Make authorizations on custom UDFs involved in tables/view configurable.
> ------------------------------------------------------------------------
>
> Key: HIVE-26799
> URL: https://issues.apache.org/jira/browse/HIVE-26799
> Project: Hive
> Issue Type: New Feature
> Components: HiveServer2, Security
> Affects Versions: 4.0.0-alpha-2
> Reporter: Sai Hemanth Gantasala
> Assignee: Sai Hemanth Gantasala
> Priority: Major
> Labels: pull-request-available
> Time Spent: 20m
> Remaining Estimate: 0h
>
> When Hive is using Ranger/Sentry as an authorization service, consider the
> following scenario.
> {code:java}
> > create table test_udf(st string); // privileged user operation
> > create function Udf_UPPER as 'openkb.hive.udf.MyUpper' using jar
> > 'hdfs:///tmp/MyUpperUDF-1.0.0.jar'; // privileged user operation
> > create view v1_udf as select udf_upper(st) from test_udf; // privileged
> > user operation
> //unprivileged user test_user is given select permissions on view v1_udf
> > select * from v1_udf; {code}
> It is expected that test_user needs to have select privilege on v1_udf and
> select permissions on udf_upper custom UDF in order to do a select query on
> view.
> This patch introduces a configuration
> "hive.security.authorization.functions.in.view"=false which disables
> authorization on views associated with views/tables during the select query.
> In this mode, only UDFs explicitly stated in the query would still be
> authorized as it is currently.
> The reason for making these custom UDFs associated with view/tables
> authorizable is that currently, test_user will need to be granted select
> permissions on the custom udf. and the test_user can use this UDF and query
> against any other table, which is a security concern.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
