[
https://issues.apache.org/jira/browse/HDDS-1012?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16755534#comment-16755534
]
Anu Engineer commented on HDDS-1012:
------------------------------------
maybe this Jira should be fixed, it is very difficult to code review a security
client code in isolation, maybe it is better to take a complete scenerio and
write code with that and add code as needed into Certificate client
Some minor comments inline.
1. BlockToken.java:92: This code should not throw if this needs to work. Not
part of this patch.
2. BlockToken.java: Line 99, if we allow this function to throw IOExecption,
then we can remove all kinds of try catch completely. In fact, this function
will have no code modification AFAIK.
3. DefaultCertificateClient.javaE#init -- Think that bit based coding is very
fragile and hard to understand. If you want to create a mapping like this,
please add an enum and then handle that in the switch case. It is hard to read,
case 0... especially when the good and bad cases are mixed all over in the bit
map. Please consider rewriting, so suggestions, break them into functions
4. init: I don't think code path is possible at all.
{code}
publicKey = getCertificate().getPublicKey();
try {
if(publicKey == null) {
throw new CertificateException("Found private key and certificate " +
"but public key missing." +
"Error while trying to recover public key.", BOOTSTRAP_ERROR);
}
{code}
5. It is very hard to understand how this code will be used if there is no use
of this code in the patch. Can we have a patch, which focuses on a use case, so
that we can understand the security issues in the context? I am not able to
see/understand how the init function will be used?
6. KeyCodec.java: My Editor complains that it is duplicate code, Can you please
take a look?
7. OzoneConfigKeys.java: I think it is a security hole to create default
passwords in code. Most users will not know about this, and all you need one
bad guy to misuse this. We should just store normal .pem files in directories
protected by the file system. This hardcoded default password adds no extra
level security over the file system permissions. Once we move to that model,
the changes in SecurityConfig are superfluous too.
> Add Default CertificateClient implementation
> --------------------------------------------
>
> Key: HDDS-1012
> URL: https://issues.apache.org/jira/browse/HDDS-1012
> Project: Hadoop Distributed Data Store
> Issue Type: Sub-task
> Reporter: Ajay Kumar
> Assignee: Ajay Kumar
> Priority: Major
> Attachments: HDDS-1012.01.patch, HDDS-1012.02.patch
>
>
> Add Default CertificateClient implementation
--
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
