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


Reply via email to