RFR, JDK-8212885: TLS 1.3 resumed session does not retain peer certificate chain

[Jamil Nimeh]

Hi Xuelei,

I've made the change.  I think in this specific case 
CipherSuite.hashAlg.toString is just a simple return of the name field 
so it should be no less reliable than hitting the name field directly.  
Changing it does make it more consistent with other places in the 
method, so that's good.

http://cr.openjdk.java.net/~jnimeh/reviews/8212885/webrev.03

--Jamil

On 11/6/2018 11:29 AM, Xuelei Fan wrote:
> Looks fine to me.
>
> As you are already there, would you mind have an additional 
> improvement in PreSharedKeyExtension.java?
>
> -   MessageDigest md = MessageDigest.getInstance(hashAlg.toString());;
> +   MessageDigest md = MessageDigest.getInstance(hashAlg.toString());
>
> Normally, the toString() is not a reliable method to get the precise 
> algorithm name.  Would you mind update to use hashAlg.name instead?
>
> -   MessageDigest md = MessageDigest.getInstance(hashAlg.toString());;
> +   MessageDigest md = MessageDigest.getInstance(hashAlg.name);
>
>
> Thanks,
> Xuelei
>
>
> On 11/6/2018 11:05 AM, Jamil Nimeh wrote:
>> Hi Xuelei, updated review here:
>>
>> http://cr.openjdk.java.net/~jnimeh/reviews/8212885/webrev.02
>>
>> I followed your suggestions and also cleaned up some remnant comments 
>> and removed a double-semicolon...just cosmetic stuff.
>>
>> --Jamil
>>
>> On 11/6/18 10:11 AM, Jamil Nimeh wrote:
>>> Okay, I can move this into PreSharedKeyExtension.java and re-run the 
>>> local tests that were having issues with it.  Should work pretty well.
>>>
>>> I'll put out another code review shortly.
>>>
>>> Thanks,
>>> --Jamil
>>>
>>> On 11/6/2018 7:36 AM, Xuelei Fan wrote:
>>>> Nice update!
>>>>
>>>> For the update in ClientHello.java, I may suggest moving it to 
>>>> pre_shared_key extension class.  It may be a little bit safer if 
>>>> the extension can be loaded in other places.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>> On 11/5/2018 11:51 PM, Jamil Nimeh wrote:
>>>>> Hello all,
>>>>>
>>>>> This fixes an issue where TLS 1.3 resumed sessions were not 
>>>>> carrying forward many of the parameters from the parent session, 
>>>>> namely the peer certificates, but also the local certificates and 
>>>>> a few other SSLSessionImpl fields. This also moves the fix from an 
>>>>> earlier, related issue with SNI names (JDK-8211806) into this new 
>>>>> solution.
>>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8212885
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8212885/webrev.01
>>>>>
>>>>> Thanks,
>>>>>
>>>>> --Jamil
>>>>>
>>>