[14] RFR JDK-8229243 "SunPKCS11-Solaris provider tests failing on Solaris 11.4"

Valerie Peng valerie.peng at oracle.com
Wed Sep 25 04:45:42 UTC 2019


Great, thanks for the review and feedback~

Valerie

On 9/24/2019 9:20 PM, Xuelei Fan wrote:
> I looked at the latest webrev:
>   http://cr.openjdk.java.net/~valeriep/8229243/webrev.01/
>
> Which is good to me.
>
> Thanks,
> Xuelei
>
> On 9/19/2019 5:46 PM, Valerie Peng wrote:
>>
>> I am not on the PKCS#11 committee and not sure about the plan.
>> As for which one is right, I am more inclined to the "spec is right" 
>> side which is also what NSS picked.
>> Comparing between spec and header, shouldn't the former get more 
>> eyeballs in terms of review?
>> The header file also has a deprecated structure CK_AES_GCM_PARAMS 
>> which contains both ulIvLen and ulIvBits fields.
>> As ulIvBits and ulIvLen are essentially same in terms of meaning 
>> except bytes vs bits. Having both just creates confusion and 
>> potential inconsistency and makes not much sense to me.
>>
>> Valerie
>>
>> ----- Original Message -----
>> From: xuelei.fan at oracle.com
>> To: valerie.peng at oracle.com, security-dev at openjdk.java.net
>> Sent: Thursday, September 19, 2019 7:47:43 AM GMT -08:00 US/Canada 
>> Pacific
>> Subject: Re: [14] RFR JDK-8229243 "SunPKCS11-Solaris provider tests 
>> failing on Solaris 11.4"
>>
>> Will the inconsistency structure be continue?  I was just wondering if
>> OpenHSM2/Solaris/NSS will fix the bug and use one structure in the
>> future, then we may not need to workaround the issue in the calling 
>> side.
>>
>> I had a quick look the PKCS#11 3.0 draft, there is no update of the
>> structure yet.
>>
>> Xuelei
>>
>> On 9/18/2019 6:01 PM, Valerie Peng wrote:
>>> Ping?
>>>
>>> Can someone help take a look?
>>>
>>> Thanks,
>>>
>>> Valerie
>>>
>>> On 8/15/2019 4:49 PM, Valerie Peng wrote:
>>>>
>>>> Anyone has time to help review this fix? PKCS#11 v2.40 has
>>>> inconsistent definition for CK_GCM_PARAMS struct. The mechanism spec
>>>> (sec 2..12.3) has:
>>>>
>>>>      typedef struct CK_GCM_PARAMS {
>>>>          CK_BYTE_PTR       pIv;
>>>>          CK_ULONG          ulIvLen;
>>>>          CK_BYTE_PTR       pAAD;
>>>>          CK_ULONG          ulAADLen;
>>>>          CK_ULONG          ulTagBits;
>>>>      } CK_GCM_PARAMS;
>>>>
>>>> However, the header file pkcs11t.h has an extra "ulIvBits" field:
>>>>
>>>>      typedef struct CK_GCM_PARAMS {
>>>>          CK_BYTE_PTR       pIv;
>>>>          CK_ULONG          ulIvLen;
>>>>      *    CK_ULONG          ulIvBits;*
>>>>          CK_BYTE_PTR       pAAD;
>>>>          CK_ULONG          ulAADLen;
>>>>          CK_ULONG          ulTagBits;
>>>>      } CK_GCM_PARAMS;
>>>>
>>>> Some vendors such OpenHSM2 and Solaris go with the header file while
>>>> some vendor such as NSS go with the mech spec. Current SunPKCS11
>>>> provider impl works with NSS but not with other vendors whose
>>>> CK_GCM_PARAMS struct contains ulIvBits field. Based on testing
>>>> results, OpenHSM2 and Solaris error out at init time when the
>>>> parameter length does not have their expected value. Thus, one way to
>>>> workaround this inconsistency is to re-try with a different structure
>>>> for AES GCM when the init failed. In addition, given the parameters
>>>> contains Iv and AAD which some vendor may use during subsequent
>>>> enc/dec operations, I changed to use the same model as RSASSA-PSS to
>>>> defer the freeing after the enc/dec operation is finished. Verified
>>>> the changes on Solaris 11.4 against existing GCM regression tests.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8229243
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~valeriep/8229243/webrev.00/
>>>>
>>>> Thanks,
>>>> Valerie



More information about the security-dev mailing list