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

[Xuelei Fan]

Thanks for the update!  No more comments from me.

Xuelei

On 11/6/2018 11:38 AM, Jamil Nimeh wrote:
> 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
>>>>>>
>>>>
>