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 src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java line 423: > 421: String algorithm = params.getAlgorithm (); > 422: boolean userhash = params.getUserhash (); > 423: params.incrementNC (); I am not familiar with the code and the request/response flow involved in this code, so what I mention may not be relevant. However, do you think the algorithm validation that we added in this PR should happen before this `incrementNC()` is called and the `ncString` construction is done? I see that this incremented `ncCount` then gets used in the `checkResponse` part. In the case where the algorithm validation fails and we return `null` from this method (which effectively means not setting the authorization header), do you think the subsequent `checkResponse` would run into issues due to the `incrementNC()` being done here? ------------- PR: https://git.openjdk.java.net/jdk/pull/7688
