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

[Michael McMahon]

On Fri, 4 Mar 2022 14:59:48 GMT, Weijun Wang <weijun 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
>
> 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

Right, probably better to allow whitespace, which seems to be commonly used in the existing security properties

> 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?

I'll try and see if it fits the normal line width

> 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.

Okay, I'll double check that. I haven't found any server implementations of this feature to test with yet,

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

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