[13] RFR JDK-8080462: Update SunPKCS11 provider with PKCS11 v2.40 support

Wed May 29 19:00:29 UTC 2019

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
>>>>>>>
>>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20190529/7977236f/attachment.htm>