I agree with your comments below and the current CSR looks good so I've
added my name as Reviewer. One main comment is that I don't think you
should check the "Source" compatibility box. That would mean this would
be a source-incompatible change [1], which I don't think is right. I
don't think you should check any of the Compatibility boxes as according
to the "Compatibility Risk Description" box, you said there is no
compatibility risk.
--Sean
[1] https://wiki.openjdk.java.net/display/csr/Kinds+of+Compatibility
On 5/29/19 3:00 PM, Valerie Peng wrote:
Hi Sean,
Much thanks for the feedbacks. They are very helpful... I have updated
the CSR per your feedbacks.
https://bugs.openjdk.java.net/browse/JDK-8221442
Please find more replies inline below.
On 5/28/2019 1:51 PM, Sean Mullan wrote:
Hi Valerie,
On 5/24/19 6:37 PM, Valerie Peng wrote:
Hi Sean,
Thanks much for the suggestion. I have added the info on newly
supported algorithms to both the CSR and the bug record. Please let
me know if you have more comments.
- In the Summary section, add a hyperlink to the PKCS#11 v2.40
standard and the errata
Done.
- In general, I would put more information in the Specification
section. I think attaching a patch of all the implementation changes
is a bit too raw and not that useful as it is hard to discern what is
specification and what is not (also the patch is not currently
attached and pointing to a webrev is not acceptable per CSR rules
since it may go away). Instead, I would avoid attaching a patch and
instead include descriptions of the new attributes and algorithms in
the Specification section in a format similar to that what is
documented in
https://docs.oracle.com/en/java/javase/12/security/pkcs11-reference-guide1.html.
Basically, I think this CSR should include the information that is
exposed or configurable to users outside of the implementation, which
I think can be described in 2 types of use cases:
Yes, I think this is better. I was focusing on header file updates in
original CSR. Agree that it'd be useful to list changes exposed by this
RFE.
1. configuring a PKCS#11 provider (need to know what attributes are
supported and their defaults)
Well, PKCS11 reference guide uses the term "attribute" to refer to
provider configuration options. This RFE did not add new provider
configuration options. However, there is also attributes defined in
PKCS#11 whose name starts with CKA_xxx. This RFE enhances SunPKCS11
provider to recognize new PKCS#11 attributes and won't error out with
exceptions when specified inside the configuration file entries as
attribute values, e.g.
|attributes(*,CKO_PRIVATE_KEY,CKK_RSA) = { *CKA_DECRYPT* = true }|
These newly recognized PKCS#11 attributes (CKA_xxx), mechanisms
(CKM_xxx), key types (CKK_xxx), etc., are defined in
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11Constants.java.
2. using it as a provider in an application (need to know what
algorithms are supported and what is disabled/enabled by default)
Yes, these are the list of newly supported algorithms which I have added
to the CSR to address your earlier comment. None of them are disabled by
default. However, only when the underlying PKCS11 library also support
these, they will show up as available.
- Are there new attributes that are now supported than what are
currently listed in Table 5.1 of the PKCS#11 Reference Guide?:
https://docs.oracle.com/en/java/javase/12/security/pkcs11-reference-guide1.html#GUID-C4ABFACB-B2C9-4E71-A313-79F881488BB9
If you are referring to attributes per the convention of PKCS#11
reference guide, no new provider configuration attributes added.
However, new PKCS#11 attributes are recognized. We don't list recognized
PKCS#11 attributes in the reference guide as they are quite extensive.
PKCS11 header files define a long list of constants and depending on
what they are for, it may or may not lead to an error. For example,
unrecognized mechanisms and error codes will be accepted and their long
values are used instead. However, unrecognized PKCS#11 attributes
specified in the PKCS#11 provider configuration file will lead to exception.
If so, I think we should list them in the Specification section with
the same details as in the Reference Guide.
Given the above, I suppose that we don't need to list out all newly
supported PKCS#11 attributes? They do impact the user in some way, but
not sure if it's too much details. Perhaps we can just state that
whatever supported in the PKCS#11 v2.40 headers are now recognized by
SunPKCS11 provider.
- For the new algorithms, I would include those in the Specification
section, in a format like table 5.3 in the PKCS#11 Reference Guide:
https://docs.oracle.com/en/java/javase/12/security/pkcs11-reference-guide1.html#GUID-D3EF9023-7DDC-435D-9186-D2FD05674777
Sure, will do.
- I would include any new or changed defaults for attributes, etc.
No new defaults or new attributes for PKCS11 provider configurations.
Regards,
Valerie
--Sean
All,
RFEs need to be integrated by 6/13. Can someone help reviewing this
soon? Mach5 run is clean. I up'ed the version of webrev to webrev.01
due to the additional support for RSASSA-PSS signatures.
RFE: https://bugs.openjdk.java.net/browse/JDK-8080462
CSR: https://bugs.openjdk.java.net/browse/JDK-8221442
Webrev: http://cr.openjdk.java.net/~valeriep/8080462/webrev.01/
Thanks,
Valerie
On 5/22/2019 7:55 AM, Sean Mullan wrote:
On 5/21/19 7:19 PM, Valerie Peng wrote:
I thought we always file CSR when updating the version of external
standard, e.g. documenting the import aspect of JDK.
Good point though I think that was primarily based on whether the
external standard was referenced in the javadocs of the standard
APIs or influenced the behavior of existing APIs in some way. I
don't think PKCS#11 is referenced from any of our standard APIs, but
since this new version does add support for additional crypto
algorithms via the standard APIs that weren't previously available,
that sounds like a good enough reason for filing the CSR.
I would recommend adding some additional details to the CSR to list
what new features/algorithms PKCS#11 v2.40 provides and which
standard APIs those features are applicable to. It would also be
helpful to add similar details to the main issue and the release
note as there aren't many details about what features are in the new
version.
Thanks,
Sean
I'd love to close/withdraw the CSR if it's not needed.
Thanks,
Valerie
On 5/20/2019 12:11 PM, Sean Mullan wrote:
On 5/17/19 3:56 PM, Valerie Peng wrote:
Thanks Martin for helping me troubleshoot NSS side, I added PSS
support into PKCS11 provider and added PSS-specific regression
tests. Please find webrev updated as below:
http://cr.openjdk.java.net/~valeriep/8080462/webrev.01/
Can someone help review the CSR first as the approval may take a
week or so.
I am curious why a CSR is needed? This seems to be strictly an
implementation change with no compatibility effects.
--Sean
Thanks,
Valerie
On 4/12/2019 5:05 PM, Valerie Peng wrote:
Anyone has time to review this? Besides the header files update,
I added support for AES/GCM/NoPadding support. Ran into some
strange NSS error with RSASSA-PSS signature mechanism, so I have
not included the PSS signature impl here.
RFE: https://bugs.openjdk.java.net/browse/JDK-8080462
Webrev: http://cr.openjdk.java.net/~valeriep/8080462/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8221442
Thanks,
Valerie
