[
https://issues.apache.org/jira/browse/HADOOP-10758?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Arun Suresh updated HADOOP-10758:
---------------------------------
Attachment: HADOOP-10758.7.patch
Uploading new patch with feedback suggestions.
Thanks for the review [~tucu00], couple of things :
* bq. 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.
{{getExtension()}} cannot return this, since KeyAuthKeyProv sets its extension
to null (in the constructor) and delegates all KPCE methods to the underlying
KPCE after authorization check.. so I felt {{provider.getExtension()}} was the
only logical return value. I have fixed {{getKeyProvider()}} to return this.
* bq. {{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).
This is not required. in the {{authorizeCreateKey()}} method, at key creation
time, if no {{KEY_ACL_NAME}} is found in the attribute, then the key name is
SET as the {{KEY_ACL_NAME}} in the metadata (if default key access has been
configured for MANAGEMENT ops).. subsequent accesses would fall back to the
default key access configured for that Op. in which case, we can still enable
key-acl for the key post create.
* bq. {{authorizeCkreateKey()}}, the {{success =...}} predicate assignment
could be done once by doing a refactoring on how the name/attribute is assigned.
I Agree.. but I was hoping to keep it this way.. since the code paths are a bit
easier to read.. also, like I mentioned in the previous comment, if the
{{KEY_ACL_NAME}} is not found, we set it in the metadata.. which is done in one
of the if blocks.. so clubbing everything into a single {{success = ..}} might
look a bit hairy.
* bq. t 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.
We require explicit ACLs OR or a configured default key acl for the specified
Operation. I have updated the docs... I have also added another testcase in
{{TestKMS}} that hopefully would better illustrate it.
> 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, HADOOP-10758.7.patch
>
>
> The KMS server should enforce ACLs on per key basis.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
