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