[
https://issues.apache.org/jira/browse/HADOOP-10720?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14055098#comment-14055098
]
Alejandro Abdelnur commented on HADOOP-10720:
---------------------------------------------
*CommonConfigurationKeysPublic.java:*
* there is a KMS constant in KMSClientProvider, with the prefix constant
defined there. we should bring that here (may be a simple JIRA to be done
before this one).
* Javadocs typo 'Defalt' should be 'Default'.
* Default cache size should be a bit higher, I’d say 500.
* Indicate the expected time unit of the keys in the javadocs and the state the
default value with the closest unit, i.e 43200000 is 12hrs.
* Have a preceding comment <!--- KMSClientProvider configurations —>
*We should have 2 different ACLs, one for generate and one for decrypt.*
*KeyProviderCryptoExtension.java:*
* why are we moving {{EncryptedKeyVersion}} inside of {{CryptoExtension}}?
*KMS.java:*
* I’d make {{ServerSideEncKeyVersionQueue}} and outer class.
* In the {{fillQueueForKey()}} method, the parameters to the
{{generateEncryptedKey()}} in the loop, can be done outside of the loop once.
* There are a few whitepace changes.
* Indentation of added code is funny and beyond 80 chars.
* DECRYPT should be a GET. both ENCRYPT and DECRYPT REST calls have unused
params. also, i could all the REST methods {{decryptEncryptionKey}} &
{{generateEncryptionKeys()}}. The {{doEncryptionOp()}} could be fold into the
{{generateEncryptionKeys()}}.
*KMSClientProvider.java:*
* don’t use wildcard imports (and preferably no static imports)
* white space changes
* {{parseJSONEncKeyVersion()}} should throw an {{IOException}} if any value is
null, all are required values.
*KMSRESTConstants.java:*
* {{EDEK_NUM_KEYS}} should be {{EEK_NUM_KEYS}}
* {{KEY_NAME_FIELD}} constant seems unused
*KMSServerJSONUtils:*
* white space changes
*KMSWebApp.java:*
* the change in this class is not correct
*TestKMS.java:*
* white space changes
*ValueQueue.java:*
* {{getAtleast()}} should be {{getAtLeast()}}
* {{getAtLeast()}}, if queue is empty should trigger async queue filling and
fill 1 value synchronous to avoid stealing from other request.
* Also, it seems we could make this class concrete and have an inner
{{Fetcher}} interface that is provided in the constructor.
> KMS: Implement generateEncryptedKey and decryptEncryptedKey in the REST API
> ---------------------------------------------------------------------------
>
> Key: HADOOP-10720
> URL: https://issues.apache.org/jira/browse/HADOOP-10720
> Project: Hadoop Common
> Issue Type: Improvement
> Components: security
> Affects Versions: 3.0.0
> Reporter: Alejandro Abdelnur
> Assignee: Arun Suresh
> Attachments: COMBO.patch, COMBO.patch, COMBO.patch, COMBO.patch,
> COMBO.patch, HADOOP-10720.1.patch, HADOOP-10720.2.patch,
> HADOOP-10720.3.patch, HADOOP-10720.4.patch, HADOOP-10720.patch,
> HADOOP-10720.patch, HADOOP-10720.patch, HADOOP-10720.patch, HADOOP-10720.patch
>
>
> KMS client/server should implement support for generating encrypted keys and
> decrypting them via the REST API being introduced by HADOOP-10719.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
