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