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

Wed May 29 21:01:57 UTC 2019

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
>>>>>>>>>
>>>>>>>>>