[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13547870#comment-13547870 ] Hudson commented on HIVE-3705: -- Integrated in Hive-trunk-hadoop2 #54 (See [https://builds.apache.org/job/Hive-trunk-hadoop2/54/]) HIVE-3705 : Adding authorization capability to the metastore (Sushanth Sowmyan via Ashutosh Chauhan) (Revision 1418802) Result = ABORTED hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1418802 Files : * /hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java * /hive/trunk/conf/hive-default.xml.template * /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java * /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java * /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultAuthenticator.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultMetastoreAuthenticator.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/HiveMetastoreAuthenticationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/BitSetCheckedAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveMetastoreAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveMetastoreAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java * /hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/security/DummyHiveMetastoreAuthorizationProvider.java * /hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/security/InjectableDummyAuthenticator.java * /hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java * /hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/security/TestDefaultHiveMetastoreAuthorizationProvider.java > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Fix For: 0.10.0 > > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, HIVE-3705.D6681.5.patch, > HIVE-3705.giant.svn-0.10.patch, HIVE-3705.giant.svn.patch, > hive-backend-auth.2.git.patch, hive-backend-auth.git.patch, > hive-backend-auth.post-review.git.patch, > hive-backend-auth.post-review-part2.git.patch, > hive-backend-auth.post-review-part3.git.patch, hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13529478#comment-13529478 ] Ashutosh Chauhan commented on HIVE-3705: Committed to 0.10 branch. Thanks, Sushanth! > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Fix For: 0.10.0 > > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, HIVE-3705.D6681.5.patch, > HIVE-3705.giant.svn-0.10.patch, HIVE-3705.giant.svn.patch, > hive-backend-auth.2.git.patch, hive-backend-auth.git.patch, > hive-backend-auth.post-review.git.patch, > hive-backend-auth.post-review-part2.git.patch, > hive-backend-auth.post-review-part3.git.patch, hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13527916#comment-13527916 ] Sushanth Sowmyan commented on HIVE-3705: I've regenerated this patch for svn branch-0.10, but with the changes in the testcases, we also require the patch for HIVE-3724 to be applied to 0.10 first. I'll attach a patch there too for the patch ported to branch-0.10. > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Fix For: 0.11 > > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, HIVE-3705.D6681.5.patch, > HIVE-3705.giant.svn-0.10.patch, HIVE-3705.giant.svn.patch, > hive-backend-auth.2.git.patch, hive-backend-auth.git.patch, > hive-backend-auth.post-review.git.patch, > hive-backend-auth.post-review-part2.git.patch, > hive-backend-auth.post-review-part3.git.patch, hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13527404#comment-13527404 ] Hudson commented on HIVE-3705: -- Integrated in Hive-trunk-h0.21 #1844 (See [https://builds.apache.org/job/Hive-trunk-h0.21/1844/]) HIVE-3705 : Adding authorization capability to the metastore (Sushanth Sowmyan via Ashutosh Chauhan) (Revision 1418802) Result = FAILURE hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1418802 Files : * /hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java * /hive/trunk/conf/hive-default.xml.template * /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java * /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java * /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultAuthenticator.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultMetastoreAuthenticator.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/HiveMetastoreAuthenticationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/BitSetCheckedAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveMetastoreAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveMetastoreAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java * /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java * /hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/security/DummyHiveMetastoreAuthorizationProvider.java * /hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/security/InjectableDummyAuthenticator.java * /hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java * /hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/security/TestDefaultHiveMetastoreAuthorizationProvider.java > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Fix For: 0.11 > > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, HIVE-3705.D6681.5.patch, > HIVE-3705.giant.svn.patch, hive-backend-auth.2.git.patch, > hive-backend-auth.git.patch, hive-backend-auth.post-review.git.patch, > hive-backend-auth.post-review-part2.git.patch, > hive-backend-auth.post-review-part3.git.patch, hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13527216#comment-13527216 ] Ashutosh Chauhan commented on HIVE-3705: +1 Running tests. > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, HIVE-3705.D6681.5.patch, > HIVE-3705.giant.svn.patch, hive-backend-auth.2.git.patch, > hive-backend-auth.git.patch, hive-backend-auth.post-review.git.patch, > hive-backend-auth.post-review-part2.git.patch, > hive-backend-auth.post-review-part3.git.patch, hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13526494#comment-13526494 ] Sushanth Sowmyan commented on HIVE-3705: In my setup, the phabricator-generated patch does not cleanly apply on trunk, so I have generated incremental patches for each of the review changes. So, to apply this patch in full, the following patches can be applied in order : hive-backend-auth.2.git.patch -> hive-backend-auth.post-review.git.patch -> hive-backend-auth.post-review-part2.git.patch -> hive-backend-auth.post-review-part3.git.patch > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, HIVE-3705.D6681.5.patch, > hive-backend-auth.2.git.patch, hive-backend-auth.git.patch, > hive-backend-auth.post-review.git.patch, > hive-backend-auth.post-review-part2.git.patch, > hive-backend-auth.post-review-part3.git.patch, hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13526490#comment-13526490 ] Phabricator commented on HIVE-3705: --- khorgath has commented on the revision "HIVE-3705 [jira] Adding authorization capability to the metastore". INLINE COMMENTS ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java:44 Ashutosh, I have tried rebasing to head, and I do not see the changes in MetaStoreUtils that provides that function. This may probably be because I modify MetaStoreUtils in my patch as well, so I have updated the latest patch as if that were available. REVISION DETAIL https://reviews.facebook.net/D6681 To: JIRA, ashutoshc, khorgath > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, HIVE-3705.D6681.5.patch, > hive-backend-auth.2.git.patch, hive-backend-auth.git.patch, > hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13510776#comment-13510776 ] Phabricator commented on HIVE-3705: --- khorgath has commented on the revision "HIVE-3705 [jira] Adding authorization capability to the metastore". INLINE COMMENTS ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultMetastoreAuthenticator.java:27 Ah, wait, I see what you mean - sorry, some of my response is because I thought I was responding to the setHandler() method in the Metastore Authorization Provider as opposed to the Metastore Authentication Provider. And yes, as of right now, from the authentication side, all we really need is the conf, but my point, as you note as well, is from a perspective of thinking about what interface makes sense. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:256 Makes sense, adding. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:122 Agreed. Adding docs. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:168 Makes sense, changing. ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java:44 That's useful. Changing. REVISION DETAIL https://reviews.facebook.net/D6681 BRANCH HIVE-3705 To: JIRA, ashutoshc, khorgath > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, > hive-backend-auth.2.git.patch, hive-backend-auth.git.patch, > hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13509042#comment-13509042 ] Phabricator commented on HIVE-3705: --- ashutoshc has requested changes to the revision "HIVE-3705 [jira] Adding authorization capability to the metastore". Patch is getting close. I added few more comments, mostly around documenting semantics used in the patch. INLINE COMMENTS ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:256 This warrants a java-doc comment that in case of partition for which location is not available yet, table's location is used. This will be the case when some one does alter table add partition. Actually, this will be true even in case of create table, where dir will get created only after the authorization has happened, so will it check permissions of parent ? I think it will be good to add java-doc of all the scenarios, which dir will be used to base authentication. e.g., in case if user does create table and provides location and dir already exists, then that dir is used, otherwise parent dir. Semantics of which dir is used in which use-case need to be called out for explicitly. I assume your semantics model is based on http://wiki.apache.org/pig/Howl/HowlAuthorizationProposal and http://wiki.apache.org/pig/Howl/AuthorizationImplNotes If so, please document various pieces of code of this patch with all this information. ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultMetastoreAuthenticator.java:27 You don't store the handler, just use its conf. So, I don't see how this interface and its impl is useful as it stands today. On the other hand, I agree that, this is an important interface and is good to have this interface here because there could be alternative impl possible here. So, lets have the interface. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:122 As I noted above, these kind of semantic decision need to be documented in the code. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:168 If they are not used currently than instead of defining semantics now, we should let it fail in default branch and have the exception thrown, such that when someone later does add support for these, we can formalize the semantics. ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java:44 In the trunk, this is done via MetaStoreUtils::startMetaStore() . See, https://issues.apache.org/jira/browse/HIVE-3724 for recommended usage pattern for it. REVISION DETAIL https://reviews.facebook.net/D6681 BRANCH HIVE-3705 To: JIRA, ashutoshc, khorgath > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, HIVE-3705.D6681.4.patch, > hive-backend-auth.2.git.patch, hive-backend-auth.git.patch, > hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13505754#comment-13505754 ] Phabricator commented on HIVE-3705: --- khorgath has commented on the revision "HIVE-3705 [jira] Adding authorization capability to the metastore". INLINE COMMENTS ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:49 Good point. Thought about it a bit, and decided that the best place for this constant was in MetaStoreUtils along with some other similar constants there. Have refactored to push it there. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:77 Agreed. Removing. I was mimicing existing code in HCatalog's HDFS Auth Provider, but you're right, we need to be stricter. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:89 Agreed, same response. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:111 Agreed, same response. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:95 getPath is equivalent to a path constructed on table.getTTable().getSd().getLocation() only if it is nonEmpty, which it is in the else segment. Have made it clearer by avoiding referencing .getPath altogether. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:117 Done. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:122 There isn't a case with partition being null, removing that bit as with 77,89 and 111 above. But it is possible for the location to be null as with the case of creating a partition - when the PreEventListener is triggered, it's possible for the part location to be null, in which case the correct behaviour is to check the table's permissions. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:165 Interesting question - I assumed it was for creating an Index. That said, this is currently unused in Hive - there's no reference to this that I find in the codebase. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:168 Ditto as with 165, but with the exception that HiveOperation does define them as read privileges for both LOCKTABLE and UNLOCKTABLE. That doesn't sound terribly right to me, as I don't think read privileges are enough to be able to perform either of these operations. I'm going to leave this as-is, and ditto with the INDEX case above unless you think we should change it. At any rate, these are not privileges currently in use, even if LOCK is partially defined. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:218 Agreed, changing across all files in this patch. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:301 I was trying to keep changes minimal and not change HiveMetaStore too much, but yes, okay, refactoring, moving over to Warehouse ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:308 This does not reimplement Warehouse::getDatabasePath, it extends it(it calls it) by providing a default path if the location was null. ql/src/test/org/apache/hadoop/hive/ql/security/TestAuthorizationPreEventListener.java:44 Agreed, done across tests. ql/src/test/org/apache/hadoop/hive/ql/security/TestDefaultHiveMetastoreAuthorizationProvider.java:137 Added as requested. REVISION DETAIL https://reviews.facebook.net/D6681 BRANCH HIVE-3705 To: JIRA, ashutoshc, khorgath > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, hive-backend-auth.2.git.patch, > hive-backend-auth.git.patch, hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JI
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13505064#comment-13505064 ] Phabricator commented on HIVE-3705: --- khorgath has commented on the revision "HIVE-3705 [jira] Adding authorization capability to the metastore". Replied to a few of the review comments. INLINE COMMENTS conf/hive-default.xml.template:1269 Agreed, changing. conf/hive-default.xml.template:1284 Agreed, changing ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultMetastoreAuthenticator.java:27 Not true. The handler that is set is used to do things like fetch dbs, fetch privilege bitmaps, etc for the Default authorizer, and in general, as an interface point, it is an important bit of functionality to extend to the authorization managers that run on the metastore-side to check metadata. Without this, a metastore-side authorization provider will wind up making much more costly calls (instantiating a hive object/etc) to do any metadata fetches. ql/src/java/org/apache/hadoop/hive/ql/security/HiveMetastoreAuthenticationProvider.java:25 Agreed, done. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:62 It was because we want some HiveConf parameters from a HiveConf object, but the PreEventListener interface is set up to be instantiated by a Configuration object as opposed to a HiveConf object. In implementation, it is instantiated with a HiveConf which is-a Configuration, and thus, we can get away with it, but this is a case of relying on implementation as opposed to interface. That said, this is easily fixed by HiveConf.getVar used by MetastoreUtils, so I was avoiding a nonexistant problem. Fixed. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:64 I did not want to leave metastore-auth to a side-effect of configuring the AuthorizationPreListener, and wanted to have the metastore-side auth config to mimic client-side auth config, but I see the benefit in what you're suggesting. I'll change it. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:112 The listener can only act on the types of events that a listener is triggered for. We'd have to follow it up by changing so that grant/revokes also cause events. But yes, will change javadoc to reflect. And yes, your understanding is correct, a user can still use a DefaultHiveMetastoreAuthProvider and grant privileges to themselves with this patch. We'd have to change that further as we improve it. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:125 Good point, changing. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:232 Agreed, was a result of an eclipse refactor default, changed. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:250 Agreed, was a result of an eclipse refactor default, changed. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:256 This is needed because leaving it as null(as the case will be with Partition objects yet to be created) will cause a NullPointerException when trying to construct a ql.metadata.Partition, which assumes a .getSd() will not return null. Given that we then need to pick a default, the table's .getSd() is a useful default for when the Partition object has not yet been populated. This way, authorizaton providers can check the table directory for permissions if they need to, rather than being faced with a null, which is not useful. Also, the only place this is used is for Authorization, so it does not affect the rest of the codeflow. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveAuthorizationProvider.java:30 This would be a bad idea because you're being passed a Configuration object. I understand that in practice you'll likely get a HiveConf object, but this type of runtime casting is asking for trouble later on when someone instantiates it with a Configuration object. We can do runtime checks to see if we got passed a HiveConf object, in which case we retain it, or if we got a Configuration object, in which case we create a new HiveConf, but that leads to kludgy code. If we want to prevent instantiating a HiveConf object, then either Hive.get needs to change to accept a Configuration, or HiveAuthorizationProvider.init interface needs to change to require a HiveConf. Also, this is not new code, this is taken verbatim from HiveAuthorizationProviderBase that already did this. REVISION DETAIL https://reviews.facebook.net/D6681 BRANCH HIVE-3705 To: JIRA, ashutoshc, khorgath > Adding authorization capability to the metastore > -
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13504069#comment-13504069 ] Phabricator commented on HIVE-3705: --- ashutoshc has requested changes to the revision "HIVE-3705 [jira] Adding authorization capability to the metastore". Please see inline comments. INLINE COMMENTS conf/hive-default.xml.template:1254 I think it can be better worded as ... The hive metastore authorization.. conf/hive-default.xml.template:1269 same as previous comment. conf/hive-default.xml.template:1269 This will read better as "The authorization manager class name..." conf/hive-default.xml.template:1284 Same as above. ql/src/java/org/apache/hadoop/hive/ql/security/HadoopDefaultMetastoreAuthenticator.java:27 It seems that you have created new interface HiveMetaStoreAuthenticationProvider and added method setHandler() just to setConf(). If this is the only reason, then there is no need of this new interface, since HiveAuthenticationProvider already extends Configurable and at instantiation time of this interface in HiveUtils, you have access to conf which you can set. So, this interface and this new implementation of it seems overkill. ql/src/java/org/apache/hadoop/hive/ql/security/HiveMetastoreAuthenticationProvider.java:25 Need to update this javadoc. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:62 Is there any reason for creating new instance of HiveConf? You can just save passed-in config. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:64 Ideally, this bool should not be checked in here. If pre-event listener is already set for this listener class, we can safely assume that he intends this listener to fire and checks to happen. Infact, I don't see point of this boolean in HiveConf at all. Since, these checks are meant to be invoked via listener interface, if user has to consciously put these class names in the config. Why then he has to also set that boolean to be true? More configs usually results in more confusion. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:125 This will break chaining of exception stack. I think it will be better done as: InvalidOperationException ex = new InvalidOperationException(e.getMessage()); ex.initCause(e.getCause()); throw ex; Same comment applies for all other try-catch blocks. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:112 For all other operations (including grant, revoke etc.) no checks are performed and are allowed straight through. I think you should add a note in javadoc of this listener class that it only performs checks for create/add/alter/drop of db/tbl/partitions. I am not sure if following deployment is supported: This listener configured with DefaultHiveAuthProvider which does auth based on privs stored in metastore? If it is, then it has same problem which that provider has. User can grant himself all privs, no checks are done for that and then drop tables/dbs. I understand you are not improving semantics in this patch, but merely shifting checks on metastore from client, but just wanted to make sure my understanding is correct. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:232 This method should be private. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:250 This method should be private. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java:256 Why is this needed ? In particular, this will set location of partition to table's location in null-case. Is that desirable? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveAuthorizationProvider.java:30 You can just cast (HiveConf)conf, instead of doing new HiveConf()? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:49 This is also defined in HiveMetaStore. This should be statically defined in HiveConf and should be referenced from there, instead of private copy in each class. ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:165 I am not sure about this, but is this about creating index, in which case Write makes sense or is this about reading indexing, in which case read should suffice ? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:168 Same as above. Locks could be a shared lock or exclusive lock, resulting in equivalent of read and write privs? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java:301 This method already exists in Hiv
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13501484#comment-13501484 ] Sushanth Sowmyan commented on HIVE-3705: @Shreepadma : I've now updated the diff with a reference implementation of a StorageBasedAuthorizationProvider, which is a port of HCatalog's HdfsAuthorizationProvider. > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > HIVE-3705.D6681.3.patch, hive-backend-auth.git.patch, > hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13500375#comment-13500375 ] Rob Weltman commented on HIVE-3705: --- A new JIRA has been opened for the larger issues around the desired semantics of Hive authorization and ensuring they are enforced: https://issues.apache.org/jira/browse/HIVE-3720 > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > hive-backend-auth.git.patch, hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13499331#comment-13499331 ] Sushanth Sowmyan commented on HIVE-3705: @Ashutosh : Funny - I've uploaded a lone patch as well now, just in case something was wrong with the autogeneration. It's called hive-backend-auth.git.patch and can be applied with a patch -p1 @Shreepadma : Thanks for your comments! a) The idea is that the box (or vm/etc) on which the metastore server runs can be secured and given limited login privileges, and kept away from most of the hive users. That way, the "secrets" such as the hive-site.xml that contains the mysql password, and the hive-site.xml config that sets which AuthorizationProvider to use from the metastore is under strict control of the admin. b) I agree that the current model's semantics can sometimes be confusing, especially when you consider separate grant/revoke based permissions and HDFS permissions, and indeed, the authorization provider I intend to use with hive that builds on top of this will be a HDFS-permissions based Authorization Provider, but the hooks for how to launch the Authorization(&Authentication) Providers and what objects they authorize on are good interfaces, I think, and I've merely extended them in where/how they can be invoked in this patch. I think there's a lot of work yet to be done, in cleaning up the privilege definitions in HiveOperation, and making sure that the various authorization calls are actually called. Take, for example the notion that "drop database" does not make any auth calls! So yes, there is more work to be done on this end. I'll be glad to put up a list of other items I felt needed to be changed/fixed. c) The userid for performing authorization is obtained from the provided authenticator. I have an implementation of a default MetastoreAuthenticator with this patch which tries to obtain the userid from the ugi that the HMSHandler has access to. I use this because it makes the most sense as it is as that user that HMSHandler uses to create wh objects, and to perform filesystem calls, for example. I'm afraid I don't know the thrift-side plumbing too well to know where it gets that setting from. As for how it interacts with HS2, again, I don't know how HS2 itself works, but if HS2 is performing a task on behalf of a user, then it would call create_table_core, for eg., as that user. Additional Authentication providers can also be written and plugged in to this scheme - I merely describe how the one I wrote and use works. > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > hive-backend-auth.git.patch, hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498520#comment-13498520 ] Shreepadma Venugopalan commented on HIVE-3705: -- @Sushanth: Thanks for posting the document and the patch. Securing the metastore is necessary to provide reliable authorization in Hive. I looked at the document and the code and have the following high level questions, a)The document contains an example of how the current pluggable authorization provider can be exploited to circumvent security. This patch seems to introduce a new config param - hive.security.metastore.authorization.manager - that allows a pluggable authorization provider. Perhaps I'm missing something here, but wondering how we would prevent a user from plugging in their own authorization provider. b)The current Hive authorization model exposes semantics that is confusing and at times inconsistent. While this patch has moved the auth checks to the metastore (IMO, this is the right thing to do) it seems to implement the existing semantics. Wondering if there is a plan to fix the semantics at some point. c)How do we obtain the userid for performing authorization? Are we using the authentication id from the Thrift context? If so, how do we handle the case where the authentication id is different from the authorization id, for e.g., HS2 authenticates to the metastore as HS2 but is executing a statement on behalf of user 'u1'? Thanks. > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[ https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13498423#comment-13498423 ] Ashutosh Chauhan commented on HIVE-3705: Sushanth, Patch doesn't apply cleanly on trunk. Can you refresh it for trunk. > Adding authorization capability to the metastore > > > Key: HIVE-3705 > URL: https://issues.apache.org/jira/browse/HIVE-3705 > Project: Hive > Issue Type: New Feature > Components: Authorization, Metastore >Reporter: Sushanth Sowmyan >Assignee: Sushanth Sowmyan > Attachments: HIVE-3705.D6681.1.patch, HIVE-3705.D6681.2.patch, > hivesec_investigation.pdf > > > In an environment where multiple clients access a single metastore, and we > want to evolve hive security to a point where it's no longer simply > preventing users from shooting their own foot, we need to be able to > authorize metastore calls as well, instead of simply performing every > metastore api call that's made. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HIVE-3705) Adding authorization capability to the metastore
[
https://issues.apache.org/jira/browse/HIVE-3705?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13496067#comment-13496067
]
Sushanth Sowmyan commented on HIVE-3705:
Phabricator review submitted : https://reviews.facebook.net/D6681
--
HIVE-3705 Enabling authorization from the metastore:
New HiveConf parameters:
hive.security.metastore.authorization.enabled : true/false determining whether
or not to do authorization in the metastore
hive.security.metastore.authorization.manager : The class to load to do
metastore-side authorization
hive.security.metastore.authenticator.manager : The class to load to do
metastore-side authentication
If the first parameter isn't set, default behaviour of hive in both client-mode
and metastore-mode is not affected, and this is disabled by default.
New Interface :
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveMetastoreAuthorizationProvider.java
:
an extension of HiveAuthorizationProvider, except with one more function that
allows the metastore to pass a HMSHandler to it
Modifications of existing classes :
Minor modifications :
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java :
added ability to instantiate HiveAuth{orization,entication}Providers given
HiveConf key to use
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java :
changed to account for above
Major modifications :
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java
:
refactored to introduce a new HiveProxy that can proxy for either a hive object
or a HMSHandler to perform necessary metadata operations
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveAuthorizationProvider.java
:
refactored most of the functionality into a new class :
BitSetCheckedAuthorizationProvider, which in turn is extended trivially by
DefaultHiveAuthorizationProvider and DefaultHiveMetastoreAuthorizationProvider
which implement small glue functionality to make them
work from the hive client side and from the hive metastore respectively.
New Classes :
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/BitSetCheckedAuthorizationProvider.java
:
As discussed above.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveMetastoreAuthorizationProvider.java
:
As discussed above.
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationPreEventListener.java
:
An implementation of the Hive Metastore PreEventListener interface that kicks
off the metastore-side authorization
> Adding authorization capability to the metastore
>
>
> Key: HIVE-3705
> URL: https://issues.apache.org/jira/browse/HIVE-3705
> Project: Hive
> Issue Type: New Feature
> Components: Authorization, Metastore
>Reporter: Sushanth Sowmyan
> Attachments: HIVE-3705.D6681.1.patch, hivesec_investigation.pdf
>
>
> In an environment where multiple clients access a single metastore, and we
> want to evolve hive security to a point where it's no longer simply
> preventing users from shooting their own foot, we need to be able to
> authorize metastore calls as well, instead of simply performing every
> metastore api call that's made.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
