RFR 8168518: rcache interop with krb5-1.15

[Xuelei Fan]

 >>> 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.
I see.  As the HASH and SHA256 was converted to MD5 and SHA-256 if using 
(with realAlg()),  and the names are never expose to external, it may be 
not necessary to use the intermediate variables and realAlg() method.

    if (the system property is true) {
       // make a note, it is the same name as HASH in MIT world.
       set the alg to MD5
    } else {
       // make a note, it is the same name as SHA256 in MIT world.
       set the alg to SHA-56.
    }

Personally, I'd like to use "useMD5" as the ending part of the system 
property.

Otherwise, the source code looks fine to me.

For the release note, it compatibility impact on JDK 9/8 are mentioned. 
What about JDK 7/6?  I guess not.  Would you mind mention the impact on 
JDK 7/6 explicitly?

BTW, can the rcache file can be refreshed to use SHA-256 if MD5 was used 
previously?

Thanks,
Xuelei

On 10/25/2016 3:36 PM, Wang Weijun wrote:
> 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 <weijun.wang at oracle.com> 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
>>>>
>>>>
>>>>
>>>>
>>>>
>