[jira] [Commented] (HADOOP-10758) KMS: add ACLs on per key basis.

["Alejandro Abdelnur (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-10758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14118382#comment-14118382 ] 

Alejandro Abdelnur commented on HADOOP-10758:
---------------------------------------------

Looks good, just a few minor things:


*KeyAuthorizationKeyProvider.java*:

* shouldn’t {{getExtension()}} and {{getKeyProvider()}} return {{this}}? or is the intention to return the unguarded entity? if the later, we should log a warning on the GET call.

* {{doAccessCheck()}}, if the {{KEY_ACL_NAME}} attribute is NULL, shouldn’t we pass the name of the key? by doing this you can key-acl existing keys via its name (in the case you enable key-acl after the keys were created).

* {{authorizeCkreateKey()}}, the {{success =...}} predicate assignment could be done once by doing a refactoring on how the name/attribute is assigned.

*KMSACLs.java*:

* {{setKeyACLs()}}, if name of the key has dots (can it?) then the logic here will fail as you are expecting 4 elements after split. I think you should look for postfix without assuming dots, you already filtered the prefix.

* it is not clear to me what is the behavior if no default ACLs are set. are we assuming '*' or we are requiring explicit ACLs for every key? it seems the later makes more sense, no? we should log a warning and put that in the docs.

*KMSConstants.java*:

* {{KEY_ACL_PREFIX}} does not seem used.




> KMS: add ACLs on per key basis.
> -------------------------------
>
>                 Key: HADOOP-10758
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10758
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: security
>    Affects Versions: 3.0.0
>            Reporter: Alejandro Abdelnur
>            Assignee: Arun Suresh
>         Attachments: HADOOP-10758.1.patch, HADOOP-10758.2.patch, HADOOP-10758.3.patch, HADOOP-10758.4.patch, HADOOP-10758.5.patch, HADOOP-10758.6.patch
>
>
> The KMS server should enforce ACLs on per key basis.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)