RFR: 8261433: Better pkcs11 performance for libpkcs11:C_EncryptInit/libpkcs11:C_DecryptInit [v2]

[Valerie Peng]

On Wed, 17 Apr 2024 08:01:17 GMT, Prajwal Kumaraswamy <pkumaraswamy at openjdk.org> wrote:

>> This fix intends to eliminate additional library call to C_EncryptInit or C_DecryptInit for Ciphers running through the CKM_AES_GCM.
>> 
>> Background: 
>> 
>> There are two types of CK_GCM_PARAMS struct that are used, one with IV bits and the other without it.
>> 
>> Initially there was issue in NSS library, due to the struct being different in header and spec version.
>> NSS was using version from header but Solaris and SoftHsm was using normative version from spec.
>> To maintain compatibility Java used to try library call with non-normative (header) version first and then upon failure retrial was made with updated GCM struct with IV bits.
>> 
>> Note: Trying normative (spec) version first with NSS library results in JVM crash.
>> 
>> Refer below for more information:
>> https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/native/libj2pkcs11/pkcs11gcm2.h#L36  
>> 
>> However NSS has fixed this to use normative/spec version since 3.52 which has spec version 2.40
>> Solaris and SoftHSM was already complying to the version mentioned in spec 2.40
>> 
>> The fix now check if spec version is 2.40 and then makes library call with appropriate structure.
>> 
>> Internal testing is green, further I have done internal testing manually with NSS library 3.96, 3.76, 3.51 (non-normative spec), 3.52 and 3.53
>> Results are attached [nss_logs.zip](https://github.com/openjdk/jdk/files/14692787/nss_logs.zip)
>> 
>> Our existing tests like sun/security/pkcs11/Cipher/TestKATForGCM.java already tests the functionality and I have used the same for internal testing
>
> Prajwal Kumaraswamy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Refactored code
>  - Merge remote-tracking branch 'origin/master' into JDK-8261433
>  - 8261433: Better pkcs11 performance for libpkcs11:C_EncryptInit/libpkcs11:C_DecryptInit

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java line 797:

> 795: 
> 796:     /**
> 797:      * C_GCMEncryptInitWithRetry initializes an GCM encryption operation and retry

nit: an -> a?

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java line 807:

> 805:      * @param hKey the handle of the encryption key
> 806:      *         (PKCS#11 param: CK_OBJECT_HANDLE hKey)
> 807:      * @param useNormativeVerFirst which version of params to use first

nit: maybe "whether to use normative version of GCM parameters first"? Same goes for C_GCMDecryptInitWithRetry(...)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18425#discussion_r1595894989
PR Review Comment: https://git.openjdk.org/jdk/pull/18425#discussion_r1595893687