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
