KRB5 (was Re: Code Review Request: TLS 1.3 Implementation)

[Xuelei Fan]

Looks good to me.

Xuelei

On 6/7/2018 8:56 AM, Weijun Wang wrote:
> Oops, another place needs change
> 
>    http://hg.openjdk.java.net/jdk/sandbox/rev/64aa781522be
> 
> --Max
> 
>> On Jun 7, 2018, at 11:03 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>>
>> Looks fine to me.  Thanks!
>>
>> Xuelei
>>
>> On 6/7/2018 8:01 AM, Weijun Wang wrote:
>>> Still at
>>>     http://cr.openjdk.java.net/~weijun/9999999/webrev.more-krb5-cleanup/
>>> I keep the existing unsupported KRB5 ciphersuites and move the 10 previously supported ones there.
>>> --Max
>>>> On Jun 7, 2018, at 10:49 PM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Jun 7, 2018, at 10:47 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>>>>>
>>>>> CipherSuite.java
>>>>> ----------------
>>>>> I think we may still want to have KRB5 cipher suite in the unsupported list, as we did for from line 455.
>>>>>
>>>>> -    TLS_KRB5_WITH_3DES_EDE_CBC_SHA(
>>>>> -          0x001F, false, "TLS_KRB5_WITH_3DES_EDE_CBC_SHA", "",
>>>>> -          ProtocolVersion.PROTOCOLS_TO_T12,
>>>>> -          K_KRB5, B_3DES, M_SHA, H_SHA256),
>>>>> +    TLS_KRB5_WITH_3DES_EDE_CBC_SHA("TLS_KRB5_WITH_3DES_EDE_CBC_SHA", 0x001F),
>>>>
>>>> Or
>>>>
>>>>    CS_001F("TLS_KRB5_WITH_3DES_EDE_CBC_SHA", 0x001F),
>>>>
>>>> which matches the naming style of other unsupported CS.
>>>>
>>>> --Max
>>>>
>>>>>
>>>>> I may prefer to have lines 545-549 (old lines) there as unsupported cipher suites.
>>>>>
>>>>> Otherwise, looks fine to me.
>>>>>
>>>>> Thanks,
>>>>> Xuelei
>>>>>
>>>>>
>>>>> On 6/7/2018 7:41 AM, Weijun Wang wrote:
>>>>>> Please take a review
>>>>>>   http://cr.openjdk.java.net/~weijun/9999999/webrev.more-krb5-cleanup/
>>>>>> --Max
>>>>>>> On Jun 7, 2018, at 10:24 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>>>>>>>
>>>>>>>> Yes, please KRB5 cipher suite from the supported list.
>>>>>>> Typo: Yes, please remove KRB5 cipher suite from the supported list.
>>>>>>>
>>>>>>> On 6/7/2018 7:23 AM, Xuelei Fan wrote:
>>>>>>>> Yes, please KRB5 cipher suite from the supported list.
>>>>>>>> For the public APIs part, please leave it as it is before we deprecate the specification.  Some other JSSE provider might still support KRB5 cipher suites.
>>>>>>>> Xuelei
>>>>>>>> On 6/7/2018 1:45 AM, Weijun Wang wrote:
>>>>>>>>> And there are the Kerberos word in public APIs:
>>>>>>>>>
>>>>>>>>> share/classes/javax/net/ssl/SSLContext.java
>>>>>>>>> 336:     * Some cipher suites (such as Kerberos) require remote hostname
>>>>>>>>> 366:     * Some cipher suites (such as Kerberos) require remote hostname
>>>>>>>>>
>>>>>>>>> share/classes/javax/net/ssl/HttpsURLConnection.java
>>>>>>>>> 106:     * such as Kerberos, will throw an SSLPeerUnverifiedException.
>>>>>>>>> 130:     * such as Kerberos.
>>>>>>>>> 134:     * KerberosPrincipal for Kerberos cipher suites.
>>>>>>>>> 158:     * return null for non-certificate based ciphersuites, such as Kerberos.
>>>>>>>>> 162:     * KerberosPrincipal for Kerberos cipher suites. If no principal was
>>>>>>>>>
>>>>>>>>> share/classes/javax/net/ssl/SSLContextSpi.java
>>>>>>>>> 90:     * Some cipher suites (such as Kerberos) require remote hostname
>>>>>>>>> 110:     * Some cipher suites (such as Kerberos) require remote hostname
>>>>>>>>>
>>>>>>>>> share/classes/javax/net/ssl/SSLEngine.java
>>>>>>>>> 395:     * Some cipher suites (such as Kerberos) require remote hostname
>>>>>>>>> 397:     * constructor to use Kerberos.
>>>>>>>>>
>>>>>>>>> share/classes/javax/net/ssl/SSLSession.java
>>>>>>>>> 221:     * such as Kerberos, will throw an SSLPeerUnverifiedException.
>>>>>>>>> 264:     * such as Kerberos, will throw an SSLPeerUnverifiedException.
>>>>>>>>> 295:     * KerberosPrincipal for Kerberos cipher suites.
>>>>>>>>> 313:     * KerberosPrincipal for Kerberos cipher suites. If no principal was
>>>>>>>>>
>>>>>>>>> share/classes/javax/net/ssl/HandshakeCompletedEvent.java
>>>>>>>>> 122:     * such as Kerberos, will throw an SSLPeerUnverifiedException.
>>>>>>>>> 145:     * such as Kerberos, will throw an SSLPeerUnverifiedException.
>>>>>>>>> 178:     * KerberosPrincipal for Kerberos cipher suites.
>>>>>>>>> 208:     * KerberosPrincipal for Kerberos cipher suites. If no principal was
>>>>>>>>>
>>>>>>>>> --Max
>>>>>>>>>
>>>>>>>>>> On Jun 7, 2018, at 4:31 PM, Weijun Wang <weijun.wang at oracle.com> wrote:
>>>>>>>>>>
>>>>>>>>>> I still see K_KRB5 KeyExchange and TLS_KRB5_WITH_3DES_EDE_CBC_SHA etc in CipherSuite.java. Shall I also remove them.
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Max
>>>>>>>>>>
>>>>>>>>>
>>>>
>