Hi Xuelei I rethink about it. Here is the updated webrev:
http://cr.openjdk.java.net/~weijun/8168518/webrev.01/ Changes from webrev.00: 1. Default value is now SHA256. This is also the same default for latest MIT krb5. 2. User can only revert it to HASH with a boolean system property. No more free setting. 3. Test fixed. I forgot to pass the system property to child process. Please also review the release note at https://bugs.openjdk.java.net/browse/JDK-8168635. Thanks Max > On Oct 25, 2016, at 1:46 PM, Wang Weijun <[email protected]> wrote: > > > > On 10/25/2016 1:40 PM, Xuelei Fan wrote: >> Is the "HASH" algorithm a new name? I'm not sure why introduce this >> none-standard name. > > HASH is actually MD5, it is the label used by MIT krb5 in its previous > releases. In 1.15, it's SHA256. I just want it to the same with MIT krb5. > >> 39 // The hash algorithm can be "HASH" or "SHA256". >> What if the algorithm is not one of the two? Why not use the standard >> names? Do you want to support SHA3? > > No. > > The system property is not meant to be fully customizable. The only usage is > that you can set it to SHA256 if you want full krb5-1.15 interoperability. I > mainly created it so I can test on it. > >> >> As the DEFAULT_HASH_ALG system property is customizable, the value may >> be not valid. Therefore, the AssertionError may be too strong to use in >> line 310 of KrbApReq.java, and the message can have more information. > > OK, I'll change the message. > > >> >> Otherwise, looks fine to me. >> >> Xuelei >> >> On 10/25/2016 12:18 PM, Wang Weijun wrote: >>> http://cr.openjdk.java.net/~weijun/8168518/webrev.00/ >>> >>> Please read https://bugs.openjdk.java.net/browse/JDK-8168518 for the >>> reason. This code change includes: >>> >>> 1. Add a hashAlg field in AuthTimeWithHash.java. >>> >>> 2. Add AuthTimeWithHash.DEFAULT_HASH_ALG so we can change it later. >>> >>> 3. The fix of the bug is inside DflCache.java: >>> >>> @@ -300,7 +302,7 @@ >>> >>> if (time.equals(a)) { >>> // Exact match, must be a replay >>> throw new KrbApErrException(Krb5.KRB_AP_ERR_REPEAT); >>> >>> - } else if (time.isSameIgnoresHash(a)) { >>> + } else if (time.sameTimeDiffHash((AuthTimeWithHash)a)) { >>> >>> // Two different authenticators in the same second. >>> // Remember it >>> seeNewButNotSame = true; >>> >>> When a AuthTimeWithHash is seen with a different hash, we believe it's >>> a new one. Before this fix, we simply compare the HASH string. Now >>> that the algorithm can be different, we only treat it a new one if the >>> algorithm is the same and the hash value is different. >>> >>> This code change has not tried to understand what a different hashAlg >>> means and try to re-calculate with it. It is just treated as unknown. >>> >>> Tests updated. ReplayCacheTestProc.java enhanced so it can be called >>> with some special system properties to test interop between a >>> non-system-default native library or even between 2 different native >>> libraries. >>> >>> Thanks >>> Max >>> >>> >>> >>> >>>
