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

[mandy chung]

> 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? 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.

It would be useful to record the timestamp of when the password
file is updated with the hashed passwords.

Mandy



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20171030/e48f6735/attachment-0001.html>