RFR: 8349550: Improve SASL random usage [v2]

Sean Mullan mullan at openjdk.org
Wed May 28 20:44:53 UTC 2025


On Wed, 28 May 2025 04:50:39 GMT, Koushik Muthukrishnan Thirupattur <duke at openjdk.org> wrote:

>> Check Digest-MD5 utilities SecureRandom Usage and update random usage with secure random
>
> Koushik Muthukrishnan Thirupattur has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8349550: Check Digest-MD5 utilities SecureRandom Usage and update random usage with secure random

For the bug, the `noreg-self` label is used for test only fixes. I would use something like `noreg-cleanup` probably.

src/java.security.sasl/share/classes/com/sun/security/sasl/CramMD5Server.java line 35:

> 33: import java.util.logging.Level;
> 34: import java.util.Map;
> 35: import java.util.Random;

You can remove this import.

src/java.security.sasl/share/classes/com/sun/security/sasl/CramMD5Server.java line 59:

> 57: final class CramMD5Server extends CramMD5Base implements SaslServer {
> 58: 
> 59:     /* Secure Random instance to generate nonce */

s/nonce/random digits used in challenge/

src/java.security.sasl/share/classes/com/sun/security/sasl/CramMD5Server.java line 59:

> 57: final class CramMD5Server extends CramMD5Base implements SaslServer {
> 58: 
> 59:     /* Secure Random instance to generate nonce */

s/Secure Random/SecureRandom/

src/java.security.sasl/share/classes/com/sun/security/sasl/digest/DigestMD5Base.java line 32:

> 30: import java.math.BigInteger;
> 31: import java.nio.charset.Charset;
> 32: import java.security.*;

This change seems unnecessary - I would restore the individual imports to be consistent with rest of file.

src/java.security.sasl/share/classes/com/sun/security/sasl/digest/DigestMD5Base.java line 132:

> 130:     protected static final byte[] EMPTY_BYTE_ARRAY = new byte[0];
> 131: 
> 132:     /* Secure Random instance to generate nonce */

s/Secure Random/SecureRandom/

src/java.security.sasl/share/classes/com/sun/security/sasl/digest/DigestMD5Base.java line 273:

> 271:      * digest challenge or response.
> 272:      * Using JCAUtil SecureRandom to be more secure and
> 273:      * achieve comparable performance with Random.

I would just remove this sentence as these kind of details can be included in the JBS issue. The previous sentence was more of a note.

-------------

PR Review: https://git.openjdk.org/jdk/pull/25241#pullrequestreview-2876347251
PR Review Comment: https://git.openjdk.org/jdk/pull/25241#discussion_r2112697954
PR Review Comment: https://git.openjdk.org/jdk/pull/25241#discussion_r2112703487
PR Review Comment: https://git.openjdk.org/jdk/pull/25241#discussion_r2112712345
PR Review Comment: https://git.openjdk.org/jdk/pull/25241#discussion_r2112705103
PR Review Comment: https://git.openjdk.org/jdk/pull/25241#discussion_r2112712509
PR Review Comment: https://git.openjdk.org/jdk/pull/25241#discussion_r2112711177


More information about the security-dev mailing list