You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org> on 2021/07/01 19:34:53 UTC

[Impala-ASF-CR] IMPALA-10436: Require lower privilege for external Kudu table creation

Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/17640 )

Change subject: IMPALA-10436: Require lower privilege for external Kudu table creation
......................................................................


Patch Set 3:

(1 comment)

I left some additional thoughts on the patch. Let me know what you think about them. Thanks!

http://gerrit.cloudera.org:8080/#/c/17640/3/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/17640/3/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@341
PS3, Line 341: if (getTblProperties().containsKey(KuduTable.KEY_MASTER_HOSTS) ||
             :           (isExternal && !BackendConfig.INSTANCE.isKuduAuthorizationEnabled())) {
             :         String authzServer = authzConfig.getServerName();
             :         Preconditions.checkNotNull(authzServer);
             :         analyzer.registerPrivReq(builder -> builder.onServer(authzServer).all().build());
             :       }
Taking a look at this block and reviewing my previous patch for https://issues.apache.org/jira/browse/IMPALA-9990, I realized that the owner of an external Kudu table could be different from Kudu's and Impala's perspectives. I thus have some additional thoughts in the following.

Specifically, I think for the creation of an external Kudu table by Impala, we may still need to register the ALL privilege on the specified Kudu table since the user that created this already existing Kudu table, e.g., usr_1, may not be the same as the requesting user that asked Impala to create an external Kudu table, e.g., usr_2.

If usr_1 is different from usr_2 above and we do not perform the check of the ALL privilege on the specified Kudu table against usr_2, then it would look like a security breach since usr_2 would be treated like the owner of the external Kudu table after the external table is successfully created in Impala, and therefore usr_2 would be able to perform any operation on the Kudu table without being explicitly granted the necessary privilege on the external Kudu table.

Let me know if you also have similar concerns. Thank you very much!



-- 
To view, visit http://gerrit.cloudera.org:8080/17640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7936e1d8c48696169f7ad7ad92abe44a26eea3c4
Gerrit-Change-Number: 17640
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 01 Jul 2021 19:34:53 +0000
Gerrit-HasComments: Yes