> On Oct 25, 2016, at 4:53 PM, Xuelei Fan <xuelei....@oracle.com> 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.
> 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.
> }
I have to do translation between arg in MessageDigest.getInstance(arg) and the
label inside the rcache file, and store a field name inside AuthTimeWithHash.
Do you mean a boolean is enough? Although the value is not fully customizable
now I still want it to a string instead of boolean so we can be free to change
later.
>
> Personally, I'd like to use "useMD5" as the ending part of the system
> property.
OK.
>
> 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?
OK, I remember hash was introduced in jdk7, will double check.
>
> BTW, can the rcache file can be refreshed to use SHA-256 if MD5 was used
> previously?
You mean inside the same file? Newly added entries will be SHA-256 and the old
ones remains. Still interoperable but just ignored.
Thanks
Max
>
> 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.w...@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
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>
