RFR: 8281561: Disable http DIGEST mechanism with MD5 by default

[Weijun Wang]

On Fri, 4 Mar 2022 09:37:21 GMT, Michael McMahon <michaelm at 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

Several comments added. Also, I am reading rfc7616 and it will be really nice if we can include the 2 examples at https://datatracker.ietf.org/doc/html/rfc7616#section-3.9 as a known answer test. Of course this means you need a way to provide a hardcoded `cnonce` and it's probably not easy.

src/java.base/share/classes/java/net/doc-files/net-properties.html line 232:

> 230:         includes {@code MD5} but other algorithms may be added in future. If it is still
> 231:         required to use one of these algorithms, then they can be re-enabled by setting
> 232:         this property to a comma separated list of the algorithm names.</P>

Is it necessary to emphasize that no whitespace is allowed around the comma in the property value? Or is it better to modify the implementation below to allow whitespaces? I notice that whitespace is allowed in some of the other properties. For example: https://github.com/openjdk/jdk/blob/de3113b998550021bb502cd6f766036fb8351e7d/src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java#L228

src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java line 75:

> 73:     // A net property which overrides the disabled set above.
> 74:     private static final String enabledAlgPropName = "http.auth.digest." +
> 75:         "reEnabledAlgs";

Why not put the string on one line?

src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java line 658:

> 656:     // truncate256 means only use the last 256 bits of the digest (32 bytes)
> 657:     private String encode(String src, char[] passwd, MessageDigest md, boolean truncate256) {
> 658:         md.update(src.getBytes(ISO_8859_1.INSTANCE));

Maybe we can support the "charset" parameter as well. The only allowed value is "UTF-8".

src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java line 670:

> 668:         if (truncate256) {
> 669:             assert digest.length >= 32;
> 670:             start = digest.length - 32;

Does this mean the left half is truncated? My understanding is that the right half should be.

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

PR: https://git.openjdk.java.net/jdk/pull/7688