RFR: 8261433: Better pkcs11 performance for libpkcs11:C_EncryptInit/libpkcs11:C_DecryptInit

[Daniel Jeliński]

On Thu, 21 Mar 2024 09:23:43 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

Does this PR actually improve the performance of AES/GCM? I'm asking because NSS documentation states that the newer NSS versions actually support both forms of the parameters, so I'd expect no performance penalty for using the old version. Is this on some other provider?

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11AEADCipher.java line 464:

> 462:              * additional field ulvIV bits and then invoke C_EncryptInit
> 463:              */
> 464:             CK_VERSION cryptokiVersion = token.p11.C_GetInfo().cryptokiVersion;

use p11.getVersion() instead

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

PR Review: https://git.openjdk.org/jdk/pull/18425#pullrequestreview-1957308754
PR Review Comment: https://git.openjdk.org/jdk/pull/18425#discussion_r1537336036