Thanks for taking this on Valerie. It's a shame to see such confusion
arise from the new PKCS11 spec.
The changes look fine to me. It'll certainly benefit from widespread
interoperability testing.
Regards,
Sean.
On 16/08/19 00:49, 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