Hi Xuelei,
Thanks for your review~ I will incorporate your feedback and re-test
before integrating it.
As for making the new init methods public, I can see both pros and cons.
On one hand, I think it'd be nice have the new init methods being public
as PKCS11 API passes both key and parameters in one init call. To
support PKCS11 PSS signature with existing Signature APIs would
implicitly require the parameter be set before init() is called. This
may not work well with apps as we don't have such restriction specified
in current Signature class javadoc. On the other hand, I also see the
potential confusion of having too much APIs doing essentially the same
thing. With current changes integrated, at least existing JDK code can
work with BC FIPS provider. Probably good enough for now.
Valerie
On 4/8/2019 10:35 AM, Xuelei Fan wrote:
The update is clear and straightforward to me.
Signature.java:542:
- 542 if (cert instanceof java.security.cert.X509Certificate) {
+ 542 if (cert instanceof X509Certificate) {
546 X509Certificate c = (X509Certificate)cert;
To keep the two lines consistent, I would prefer to remove the
package, and use X509Certificate directly. Minor update, no real need
for another code review to me.
I might have one or two concerns of the co-existing of two types of
init methods, if making the APIs public in the future. But so far so
good. let's talk about them later when filing the CSR.
Thanks,
Xuelei
On 3/25/2019 1:58 PM, Valerie Peng wrote:
Based on the earlier internal discussion, here is a "backportable"
fix for JDK-8216039 "TLS with BC and RSASSA-PSS breaks
ECDHServerKeyExchange" which does not bear any public API change.
Existing JDK codes which uses PSS signature with parameters will call
the new internal JDK APIs which select the provider based on both key
and parameters. There is no provider-specific checking and it
accommodate the usage of the BouncyCastle FIPS provider for TLS and
other applications.
Default implementations of the new methods are provided, so existing
JDK crypto providers should continue to work without change. The
default impl also set the parameters before calling init() to avoid
trigger the known PSS behavior/issue in BC FIPS provider which leads
to signature interoperabilities.
As for making the JDK internal APIs public, I plan to file a separate
bug (and CCC) later if this approach is acceptable.
Bug: https://bugs.openjdk.java.net/browse/JDK-8216039
Webrev: http://cr.openjdk.java.net/~valeriep/8216039/webrev.00/
Mach5 result is clean.
Thanks,
Valerie
