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

Xiaoyu Yao commented on HDDS-580:
---------------------------------

Thanks [~ajayydv] for working on this. Patch looks good to me overall. 
Here are a few comments:

SecurityUtil.java
Line 68: when bootstrap SCM, should we special handling if there are key exist? 
In the normal non-bootstrap case, it is fine to read keys if they exist. I 
think we need to define what to expect wrt. bootstrap. If there are key exist, 
I think we can throw exception for now and assume no existing key. We should 
move existing key to another dir to support rotate key, which will be needed 
later. 

SecurityConfig.java
Line 150: should we check and enforce component!=null to avoid key overwritten 
and simplify the logic?

Line 72/108/189/190/196/211: NIT: incorrect global replace from x509->x, please 
check other places. Let's just skip the unnecessary variable name change 
x509SignatureAlgo. 

StorageContainerManager.java
Line 152/210: keyPair should be assigned in the non-bootstrap case when 
ozone_security is enabled. In other words, we should detect if scm security 
boot-strap has not been done and return proper message. 

Line 493: we should init_security when --init scm when security is invoked.

Line 509: should we terminate after the init_security like init.

OMConfigKeys.java
Line 87: unnecessary change that will break the secure docker-compose. To be 
consistent, let's just keep the .file for now in the key name.

HDDSKeyPEMWriter.java
Line 66: Filename should be HDDSKeyPEMHandler.java after changing the class 
name.

Line 89: let's remove the cstor without component so that each component is 
guaranteed to have their separate key location.

Line 309: suggest using DFSUtil.bytes2String for byte->String conversion.

Line 320: we need to call KeyFactor.getInstance with "BC" as provider. 
Otherwise, the default one from JDK will be used. (Similarly apply to 
readPublicKeyFromFile)

Also, we will need a ticket to support encrypt private key from 
HDDSKeyPEMHandler.

TestHDDSKeyGenerator.java
Line 46: please use "test" as the component for test keys.

TestHDDSKeyPEMWriter.java
File needs to be renamed.

TestRootCertificate.java
Line 63: please use "test" as the component for test keys.
 




 

> Bootstrap OM/SCM with private/public key pair
> ---------------------------------------------
>
>                 Key: HDDS-580
>                 URL: https://issues.apache.org/jira/browse/HDDS-580
>             Project: Hadoop Distributed Data Store
>          Issue Type: Sub-task
>            Reporter: Xiaoyu Yao
>            Assignee: Ajay Kumar
>            Priority: Major
>         Attachments: HDDS-4-HDDS-580.00.patch, HDDS-580-HDDS-4.00.patch, 
> HDDS-580-HDDS-4.01.patch, HDDS-580-HDDS-4.02.patch
>
>
> We will need to add API that leverage the key generator from HDDS-100 to 
> generate public/private key pair for OM/SCM, this will be called by the 
> scm/om admin cli with "-init" cmd.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to