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

Hi Daniel, Maintaining a warning or info log requires internationalization and since java.management module does not have the necessary resource bundles, it becomes complicated to take up that activity as a part of this enhancement.  Hence I have changed all the logs to debug. We can take up

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

Hi Harsha, On 10/11/2017 12:38, Harsha Wardhana B wrote: Hi, Please find the below webrev with the following changes. 1. All the reads/writes into the password file are synchronized w.r.t threads within the JVM and across multiple JVM processes. It is possible that some edits made to file

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

Hi, Please find the below webrev with the following changes. 1. All the reads/writes into the password file are synchronized w.r.t threads within the JVM and across multiple JVM processes. It is possible that some edits made to file while the agent is running might be lost and hence added a

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

Hi Daniel, On Tuesday 07 November 2017 10:43 PM, Daniel Fuchs wrote: Hi Harsha, HashedPasswordManager.java: I'd suggest to use java.nio.charset.StandardCharset.UTF_8 instead of Charset.forName("UTF-8"); ok. The reading of the password file should probably not be allowed if some process

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

On 11/7/17 9:04 AM, Harsha Wardhana B wrote: Hi Mandy, To summarize the changes, 1. The header will not contain the file modification timestamp. Instead when the password file is modified, a debug log will be printed. The log will contain the timestamp. 2. The password file is now

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

Hi Harsha, HashedPasswordManager.java: I'd suggest to use java.nio.charset.StandardCharset.UTF_8 instead of Charset.forName("UTF-8"); The reading of the password file should probably not be allowed if some process (this one or another one) is currently writing to it, because you could just

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

Hi Mandy, To summarize the changes, 1. The header will not contain the file modification timestamp. Instead when the password file is modified, a debug log will be printed. The log will contain the timestamp. 2. The password file is now protected from concurrent writes from within the JVM.

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

On 11/7/17 8:26 AM, Harsha Wardhana B wrote: Hi, Please find below the webrev addressing Daniel and Mandy's comments. http://cr.openjdk.java.net/~hb/5016517/webrev.07/ Can you summarize the change? I thought we agree to only replace the clear passwords with the hashes and not to alter

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

Hi, Please find below the webrev addressing Daniel and Mandy's comments. http://cr.openjdk.java.net/~hb/5016517/webrev.07/ -Harsha On Wednesday 01 November 2017 09:42 PM, Daniel Fuchs wrote: On 31/10/2017 17:07, mandy chung wrote: On 10/31/17 8:55 AM, Harsha Wardhana B wrote: Hi Mandy,

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

On 31/10/2017 17:07, mandy chung wrote: On 10/31/17 8:55 AM, Harsha Wardhana B wrote: Hi Mandy, Below is the new webrev incorporating below review comments. http://cr.openjdk.java.net/~hb/5016517/webrev.06/ Looks okay in general except this: 286 // Check if header needs to be

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

Hi Harsha, Sorry for the late editorial recommendations: In jmxremote.password.template: 41: "Clear text" -> "A clear text" 43: 'below format" -> "format below" 53: "in clear" -> "in the clear" 63: "in clear" -> "in the clear" 77: "by ONLY the owner" -> "ONLY by the owner" 80-81: Is not

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

On 10/31/17 8:55 AM, Harsha Wardhana B wrote: Hi Mandy, Below is the new webrev incorporating below review comments. http://cr.openjdk.java.net/~hb/5016517/webrev.06/ Looks okay in general except this: 286 // Check if header needs to be inserted 287 if (sbuf.indexOf("#

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

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

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

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 }

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

On 10/26/17 8:57 PM, Harsha Wardhana B wrote: Hi, Below is the updated webrev incorporating review comments from Daniel, Roger and Mandy. The password file will now be locked before writing. What is the link to the new webrev? Mandy, 49 #

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

Sorry. Missed the webrev - http://cr.openjdk.java.net/~hb/5016517/webrev.05/ On Friday 27 October 2017 09:27 AM, Harsha Wardhana B wrote: Hi, Below is the updated webrev incorporating review comments from Daniel, Roger and Mandy. The password file will now be locked before writing.

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

Hi, Below is the updated webrev incorporating review comments from Daniel, Roger and Mandy. The password file will now be locked before writing. Mandy, 49 # https://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#MessageDigest 50 # MD5, SHA-1 and SHA-256 are

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

Sure. I will send out a modified webrev soon. -Harsha On Thursday 12 October 2017 08:52 PM, mandy chung wrote: On 10/12/17 8:18 AM, Harsha Wardhana B wrote: On Thursday 12 October 2017 08:40 PM, mandy chung wrote: On 10/12/17 1:16 AM, Harsha Wardhana B wrote: I'm thinking any

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

On 10/12/17 8:18 AM, Harsha Wardhana B wrote: On Thursday 12 October 2017 08:40 PM, mandy chung wrote: On 10/12/17 1:16 AM, Harsha Wardhana B wrote: I'm thinking any better alternative to the new property name?? com.sun.management.jmxremote.password.hashes

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

On Thursday 12 October 2017 08:40 PM, mandy chung wrote: On 10/12/17 1:16 AM, Harsha Wardhana B wrote: I'm thinking any better alternative to the new property name?? com.sun.management.jmxremote.password.hashes com.sun.management.jmxremote.password.asHashes

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

On 10/12/17 1:16 AM, Harsha Wardhana B wrote: I'm thinking any better alternative to the new property name?? com.sun.management.jmxremote.password.hashes com.sun.management.jmxremote.password.asHashes com.sun.management.jmxremote.passowrd.toHashes I suggest to rename

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

Hi Roger, On Wednesday 11 October 2017 09:21 PM, Roger Riggs wrote: Hi Harsha, conf/management.properties: - typo line 307: pa*sss*words HashedPasswordManager.java:  - line 46: "classes" -> "class" - line 84-87 "private" and 'static" come before "final" in declarations.  - 158 and

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

Hi Daniel, The contents written into the password file are identical and written at the same offset. Hence the order of the writes should not matter. However there is a possibility that file could be read in the midst of password change and different file contents could be read by different

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

On 10/8/17 10:34 PM, Harsha Wardhana B wrote: Hi Daniel, Below is the webrev addressing the review comments. http://cr.openjdk.java.net/~hb/5016517/webrev.04/ This approach seems reasonable.   I only review management.properties and jmxremote.password.template file. 304 #

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

Hi Harsha, conf/management.properties: - typo line 307: pa*sss*words HashedPasswordManager.java:  - line 46: "classes" -> "class" - line 84-87 "private" and 'static" come before "final" in declarations.  - 158 and everywhere: add space after "if"  before "("  - line 202: add "the" before

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

Hi Harsha, Your changes look good. However I have still a nagging doubt: What happens if two Java process share the same password file, and it needs hashing? Are there any protection in place to prevent the two processes from writing to the same file concurrently? best regards, -- daniel On

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

Hi Daniel, Below is the webrev addressing the review comments. http://cr.openjdk.java.net/~hb/5016517/webrev.04/ On Friday 06 October 2017 03:38 PM, Daniel Fuchs wrote: Hi Harsha, Good work! > http://cr.openjdk.java.net/~hb/5016517/webrev.03/ long standing typo in management.properties at

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

Hi Harsha, Good work! > http://cr.openjdk.java.net/~hb/5016517/webrev.03/ long standing typo in management.properties at line 90: measureRole => monitorRole HashedPasswordManager.java: loadPasswords() It seems this function will add the header to the file even if it already contains the

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

Hi All, Previously, for default agent, hashing of the passwords was done during the agent boot-up (ConnectorBootstrap.java). That was an error since login configuration could be different and is determined only when a login attempt is made. It would be then pointless to hash the password

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

Hi Roger, Below is the webrev incorporating changes suggested by you. http://cr.openjdk.java.net/~hb/5016517/webrev.02/ -Harsha On Wednesday 04 October 2017 12:54 AM, Roger Riggs wrote: Hi Harsha, FileLoginModule.java:  104:  Add a period at the end of the the sentence.

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

Hi Harsha, FileLoginModule.java:  104:  Add a period at the end of the the sentence. JMXPluggableAuthenticator.java: line 306:  Is the difference between singular and plural significant?   It would be less confusing if both were plural (hashPasswords). ConnectorBootstrap: 134:

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

Hi Roger, Thanks for the detailed review. Below is the webrev addressing all the review comments. http://cr.openjdk.java.net/~hb/5016517/webrev.01/ -Harsha On Tuesday 25 April 2017 10:56 PM, Roger Riggs wrote: Hi Harsha, Thanks for this important improvement. Comments: *

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

Hello, Could I get one more reviewer for this enhancement? -Harsha On Thursday 27 April 2017 12:06 PM, Harsha Wardhana B wrote: Hi Roger, Thanks for the detailed review. I will wait for more review comments before incorporating the ones below. -Harsha On Tuesday 25 April 2017 10:56 PM,

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

Hi Roger, Thanks for the detailed review. I will wait for more review comments before incorporating the ones below. -Harsha On Tuesday 25 April 2017 10:56 PM, Roger Riggs wrote: Hi Harsha, Thanks for this important improvement. Comments: * jmxremote.password.template: "Passwords will

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

Hi Harsha, Thanks for this important improvement. Comments: * jmxremote.password.template: "Passwords will be hashed by server if they are in clear." Perhaps should be more explicit: "The jmxremote.passwords file will be re-written by the server to replace all plain text passwords

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

Hi Gruss, Crypt format has additional params (|param|name and its|value|: hash complexity parameters, like rounds/iterations count ) which are not applicable to current implementation. Also, hash algorithms shipped with JDK are applicable (MD5, SHA1, SHA256) and any other algorithms

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

Hm, why introduce a new password hash format. Just use modular crypt() format (and iterations). This allows to use common tools (like htpasswd) to generate the hashes. It would use $5$ prefix for SHA256 but actually I would use $6$ for iterated SHA512 as it is the default on most recent Linux

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

Hi All, Please review this enhancement to replace plain-text password for JMX agent with SHA-256 hash. Issue: https://bugs.openjdk.java.net/browse/JDK-5016517 webrev: http://cr.openjdk.java.net/~hb/5016517/webrev.00/ Overview of