RFE Review : JDK-5016517 - Replace plaintext passwords by hashed passwords for out-of-the-box JMX Agent

[Harsha Wardhana B]

Hi Mandy,

Below is the new webrev incorporating below review comments.

http://cr.openjdk.java.net/~hb/5016517/webrev.06/


On Monday 30 October 2017 11:34 PM, mandy chung wrote:
>> http://cr.openjdk.java.net/~hb/5016517/webrev.05/
>
> Looks okay in general.   Daniel is closer to the FileLoginModule 
> implementation that I will count on his review.
>
> FileLoginModule.java
>
> 225 if(hashPwdMgr == null) {
> 226 hashPwdMgr = new HashedPasswordManager(passwordFile, hashPasswords);
> 227 } else { 228 hashPwdMgr.loadPasswords(); 229 } Will hashPwdMgr be 
> initialized multiple threads concurrently? Does this need to be 
> synchronized?
Without synchronization, it would leave an orphan instance of 
HashedPasswordManager. Fixed it.
> 243 ace.setStackTrace(e.getStackTrace()); I think ace.initCause(e) 
> instead of replacing the stack trace would help debugging. ACE should 
> have been rev'ed to take the cause parameter (a separate issue).  
> jmxremote.password.template 49 # 
> https://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#MessageDigest
>
> Please refer to JDK 9 docs.
> 255 "jmx.remote.x.password.tohashes";
> 305 # com.sun.management.jmxremote.password.tohashes = true|falseSince 
> "to hashes" are two words, capitalize "H" is a recommended convention.
> HashedPasswordManager.java
>
>   214         Stream<String> lines = Files.lines(Paths.get(passwordFile));
>
> This should be called with try-with-resource.
Done.
>
> It would be useful to record the timestamp of when the password
> file is updated with the hashed passwords.
Added it as a part of the header. The header now looks like below.

# The passwords in this file are hashed.
# In order to change password for a role, replace hashed password entry
# with clear text password or new hashed password. If new password is in 
clear,
# it will be replaced with its hash when new login attempt is made.

# file last updated on - 10/31/2017 21:23:52

-Harsha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20171031/3c279a0b/attachment.html>