Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16976 )
Change subject: IMPALA-9234: Support Ranger row filtering policies ...................................................................... Patch Set 11: (2 comments) http://gerrit.cloudera.org:8080/#/c/16976/9/fe/src/main/java/org/apache/impala/authorization/TableMask.java File fe/src/main/java/org/apache/impala/authorization/TableMask.java: http://gerrit.cloudera.org:8080/#/c/16976/9/fe/src/main/java/org/apache/impala/authorization/TableMask.java@109 PS9, Line 109: // Parse the row filter string to AST by using it in a fake query. > > Note that only the WHERE clause is returned. So "SELECT *" is used for si Done. I think "SELECT 1" is the same complexity since we just do parsing (not analyzing) here. But maybe "SELECT 1" can avoid the above confusion. http://gerrit.cloudera.org:8080/#/c/16976/9/fe/src/main/java/org/apache/impala/authorization/TableMask.java@111 PS9, Line 111: SelectStmt selectStmt = (SelectStmt) Parser.parse(stmtSql); > I would think that the admin user who has access to the row filter definition > should see the parse exception details such that he can fix it but for the > normal user there should be some level of obscuring of the error message > ..otherwise it would reveal the filter details and in any case the normal > user won't be able to fix it by himself. No, the admin user can't see the error if the policy doesn't apply to him/her. Usually the policy is set for normal users. I think the process of adding a row filter should be: 1) Admin user adds a new table containing sensitive data. 2) Admin user adds a row-filtering policy for it in Ranger Admin WebUI. Note that Ranger treates the row filter as a string and won't validate it. The admin user should also choose what roles/groups/users the policy applies to. 3) Admin user switchs to target users and run some queries on the table. Verify that the row filter works as expected. At this point, a detailed error exposing the row filter helps to debug the malformed row filter string. 4) Admin user publishs the table (notify the target users about this new table). 5) If the normal users encounter any errors, they should reach out to the admin user for help. They usually don't have permission to edit the row filter policy. > BTW, in the above execution I assume you were the privileged user. But it > sounds like the behavior would have been the same if you were not. Good point! I think this is a problem that we resolve the column/row masking before privilege checks. I'll file a follow-up JIRA for this. > I think the Hive message is also problematic ..it just happened to show less > for this case but what if the row filter subquery had some syntax issue ? Here's another example for syntax issue. Adding a row filter "id = (select from functional.alltypessmall)" on table functional.alltypestiny for my username, then launch beeline 0: jdbc:hive2://localhost:11050> select * from functional.alltypestiny; Error: Error while compiling statement: FAILED: SemanticException org.apache.hadoop.hive.ql.parse.ParseException: line 1:487 cannot recognize input near '(' 'select' 'from' in expression specification (state=42000,code=40000) You can play around with Hive by launching it using "testdata/bin/run-hive-server.sh -with_ranger". > EDIT: I see later that you mention both Impala and Hive will show the full > rewritten query in the EXPLAIN plan and the query profile. This contains the > expanded row filter even for the normal user .. so I am not sure if the parse > exception is that relevant. I would be curious to know how other products > like Spark deal with such things. The row filter could have things like > Salary or Social Security number predicate etc which should ideally be hidden. Yeah, I will do some investigations. -- To view, visit http://gerrit.cloudera.org:8080/16976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I580517be241225ca15e45686381b78890178d7cc Gerrit-Change-Number: 16976 Gerrit-PatchSet: 11 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 18 Mar 2021 00:16:03 +0000 Gerrit-HasComments: Yes
