Code Review Request: TLS 1.3 full handshake (JDK-8196584)

Valerie Peng valerie.peng at oracle.com
Fri Jun 8 19:19:36 UTC 2018


Hi Xuelei,

<sun/security/ssl/SignatureScheme.java>

Some typos, line 139: minial -> minimal, line 144: caculate -> 
calculate, minial -> minimal
When constructing PSSParameterSpec object on line 174, I think it's 
better to specify new MGF1ParameterSpec(hash) instead of null. Although 
current RSASSA-PSS signature algorithm impl in SunRsaSign provider will 
use the same message digest algorithm if no MGF1ParameterSpec is 
specified. However, this is somewhat a gray area and maybe better to be 
explicit

Do we need a constructor with both NamedGroup and SigAlgParamSpec? 
NamedGroup is for EC and SigAlgParamSpec is for RSASSA-PSS, they 
shouldn't co-exist in the same scheme, right?

There are currently 5 constructors in SignatureScheme class, I find the 
ordering of arguments somewhat confusing. The required arguments should 
appear first and we can add optional ones after them. Easier to read 
this way, at least for me...

That's it for this class. More to review.
Thanks,
Valerie

On 2/20/2018 11:57 AM, Xuelei Fan wrote:
> Hi,
>
> I'd like to invite you to review the TLS 1.3 full handshake 
> implementation.  I appreciate it if I could have feedback before March 
> 9, 2018.
>
> In the "JDK-8185576: New handshake implementation" [1] code review 
> around, I was trying to re-org the TLS handshaking implementation in the
> SunJSSE provider.  If you had reviewed that part, you can start from 
> the following webrev that based on the update of JDK-8185576:
> http://cr.openjdk.java.net/~xuelei/8196584/webrev-step.00
>
> If you would like start from earlier, here is the webrev that contains 
> the handshaking implementation re-org in JDK-8185576:
> http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.00
>
>
> This changeset only implements the full handshake of TLS 1.3, rather 
> then a fully implementation of the latest TLS 1.3 draft [2].
>
> In this implementation, I removed:
> 1. the KRB5 cipher suite implementation.
> Please let me know if you are still using KRB5 cipher suite.  I may 
> not add them back if no objections.
>
> 2. OCSP stapling.
> This feature will be added back later.
>
> Resumption and key update, and more features may be added later.
>
> Thanks & Regards,
> Xuelei
>
> [1]: 
> http://mail.openjdk.java.net/pipermail/security-dev/2017-December/016642.html
> [2]: https://tools.ietf.org/html/draft-ietf-tls-tls13-24




More information about the security-dev mailing list