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

Xuelei Fan xuelei.fan at oracle.com
Tue Nov 6 19:29:42 UTC 2018


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
>>>>
>>



More information about the security-dev mailing list