On Wed, 9 Mar 2022 14:23:38 GMT, Michael McMahon <micha...@openjdk.org> wrote:
>> Hi, >> >> Could I get the following change reviewed please, which is to disable the >> MD5 message digest algorithm by default in the HTTP Digest authentication >> mechanism? The algorithm can be opted into by setting a new system property >> "http.auth.digest.reEnabledAlgs" to include the value MD5. The change also >> updates the Digest authentication implementation to use some of the more >> secure features defined in RFC7616, such as username hashing and additional >> digest algorithms like SHA256 and SHA512-256. >> >> - Michael > > Michael McMahon has updated the pull request incrementally with two > additional commits since the last revision: > > - update > - update after first review round src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java line 99: > 97: // A net property which overrides the disabled set above. > 98: private static final String enabledAlgPropName = > 99: "http.auth.digest.enabledAlgorithms"; I'm not familiar with the practice of overriding a security property with a net property. Just FYI, in security libs, we often override a security property with a system property and we have a dedicated method for this at https://github.com/openjdk/jdk/blob/6765f902505fbdd02f25b599f942437cd805cad1/src/java.base/share/classes/sun/security/util/SecurityProperties.java#L46. src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java line 232: > 230: ? StandardCharsets.UTF_8 > 231: : StandardCharsets.ISO_8859_1; > 232: } Do you want to reject other values? According to the RFC, `utf-8` is the only valid one. src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java line 426: > 424: algorithm = "MD5"; // The default, accoriding to rfc2069 > 425: } > 426: algorithm = algorithm.toUpperCase(); Please use `toUpperCase(Locale.ROOT)` or `toUpperCase(Locale.ENGLISH)`. ------------- PR: https://git.openjdk.java.net/jdk/pull/7688
