On Fri, 4 Mar 2022 09:37:21 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 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
