Code Review Request: TLS 1.3 full handshake (JDK-8196584)
Xuelei Fan
xuelei.fan at oracle.com
Fri Jun 8 19:54:51 UTC 2018
> http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.02
Update: http://hg.openjdk.java.net/jdk/sandbox/rev/f4c7a97a1275
On 6/8/2018 12:19 PM, Valerie Peng wrote:
> 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
>
Good catches! Fixed in the above changeset.
> 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?
>
So far, they cannot co-exist in the same scheme. I would like to make
an update together with the following coding style.
> 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...
>
It makes senses to me.
I plan to do it later, maybe after we merge to the mainline. Here is
the JBS bug I used to track the issues:
https://bugs.openjdk.java.net/browse/JDK-8204636
Thanks,
Xuelei
> 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