Thanks, I have unchecked the "Source" compatibility box. Will finalize
the CSR and wait for Joe's comments if any.
Valerie
On 5/29/2019 1:12 PM, Sean Mullan wrote:
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
