Sure. Changes look fine.
Valerie
On 6/8/2018 12:54 PM, Xuelei Fan wrote:
> 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
