RFR: 8330842: Support AES CBC with Ciphertext Stealing (CTS) in SunPKCS11 [v2]

Wed May 29 22:30:35 UTC 2024

On Mon, 27 May 2024 14:22:51 GMT, Francisco Ferrari Bihurriet <fferrari at openjdk.org> wrote:

>> Francisco Ferrari Bihurriet 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 seven additional commits since the last revision:
>> 
>>  - Revert re-arrangement of native methods parameters
>>    
>>    This reverts commit 0a777e94229723376e1264e87cbf0ba805dc736f, except for
>>    the copyright which is retained as 2024.
>>    
>>    NOTE: new calls of the same methods are retained in the re-arrangement
>>    style, as we didn't introduce this re-arrangement, it was already
>>    present in most of the calls inside ::implUpdate() and ::implDoFinal().
>>    
>>    Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>>  - Merge 'openjdk/master' into JDK-8330843
>>  - 8330842: Add AES CBC with Ciphertext Stealing (CTS) SunPKCS11 tests
>>    
>>    Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>>  - 8330842: Support AES CBC with Ciphertext Stealing (CTS) in SunPKCS11
>>    
>>    Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>>  - Fix cipher tests logging
>>    
>>    Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>>  - Implement integer constants as enum
>>    
>>    Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>>  - Arrange parameters of native methods evenly
>>    
>>    C_EncryptUpdate / C_DecryptUpdate / C_EncryptFinal / C_DecryptFinal
>>    
>>    If the call doesn't fit on a single line, use the following order:
>>         long hSession,
>>      [  long directIn,  byte[] in,  int inOfs,  int inLen,  ]
>>         long directOut, byte[] out, int outOfs, int outLen
>>    
>>    [ ]: optional, not present in C_EncryptFinal / C_DecryptFinal
>>    
>>    Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
>>    Co-authored-by: Martin Balao <mbalao at redhat.com>
>
> The [PKCS #<wbr/>11 definition of `CKM_AES_CTS`](https://docs.oasis-open.org/pkcs11/pkcs11-curr/v3.0/pkcs11-curr-v3.0.html#_Toc30061253) refers to the [Addendum to NIST Special Publication 800-38A, "Recommendation for Block Cipher Modes of Operation: Three Variants of Ciphertext Stealing for CBC Mode"](https://doi.org/10.6028/NIST.SP.800-38A-Add) document. However, the standard does not prescribe which of the three CTS variants described by the document to use. This left the door open for PKCS #<wbr/>11 implementers to make different assumptions and create interoperability issues.
> 
> We are aware that the _NSS Software Token_ implemented the CS1 variant (see [lib/freebl/cts.c](https://github.com/nss-dev/nss/blob/NSS_3_90_RTM/lib/freebl/cts.c#L64-L65)), but [public clarification had to be given in NSS Bug 373108](https://bugzilla.mozilla.org/show_bug.cgi?id=373108#c7). That's why we chose `CS1` as the `cipherTextStealingVariant` default.
> 
>> As for the different variations, are you aware of different vendors supporting different variations?
> 
> At the moment of developing this enhancement, we were not aware of any other variant in use, but we had only evaluated NSS. The standard has a clear ambiguity, so there was a latent problem. Now, I have found another open source project using CS3 instead (see below).
> 
>> This assumes prior knowledge on the variation used by underlying PKCS11 library, right?
> 
> Unfortunately yes, but it gives the user a chance to adjust for their PKCS11 library without the need of a _SunPKCS11_ update from our side.
> 
>> Are all these variations under "CTS" name?
> 
> Yes, they are known as "variants" —according to the mentioned NIST addendum— or "formats" —according to [Wikipedia](https://en.wikipedia.org/wiki/Ciphertext_stealing#Ciphertext_format "Wikipedia: Ciphertext stealing - Ciphertext format")—, always in the context of "Cipher Text Stealing".
> 
>> Are they generally supported by all?
> 
> According to _Wikipedia_, the most popular alternative is CS3. It also happens to be the Kerberos choice, and so the one implemented by _SunJCE_'s `AES/CTS/NoPadding`. However, NSS chose CS1 as it was the first one described in the NIST addendum, referred by the PKCS #<wbr/>11 standard.
> 
> In the sake of interoperability, the PKCS #<wbr/>11 standard should choose one of the three variants. But since it is too late, PKCS #<wbr/>11 implementers supporting `CKM_AES_CTS` have already made their choice.
> 
> ### OP-TEE provides a PKCS #<wbr/>11 interface, and has chosen CS...

> I'm personally okay with not having a default value and forcing users to define the configuration property to enable CTS. Perhaps we can document in the CSR/guide that the NSS Software Token requires CS1 as an example. @franferrax what do you think?

@martinuy: I agree, it makes sense to require the `cipherTextStealingVariant` configuration to be explicitly set in order for _SunPKCS11_ to register the _Cipher_ services that depend on `CKM_AES_CTS` (`AES*/CTS/NoPadding` algorithm names).

Done in 997777e86c6fa03f070dcf0f219813c11cb480ce, I will adjust the CSR tomorrow.

### New `cipherTextStealingVariant` behaviour

We slightly changed the error message format when an invalid value is passed. Here it is an example for `cipherTextStealingVariant=CS4`:

sun.security.pkcs11.ConfigurationException: cipherTextStealingVariant must be one of [CS1, CS2, CS3], read: Token[CS4], line 33

If no value is passed, the `CKM_AES_CTS` mechanism is [handled as if it were disabled by the configuration](https://github.com/openjdk/jdk/blob/997777e86c6fa03f070dcf0f219813c11cb480ce/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java#L1302-L1308). This is the message logged when running with `-Djava.security.debug=sunpkcs11`:

Mechanism CKM_AES_CTS:
  ulMinKeySize: 16
  ulMaxKeySize: 32
  flags: 768 = CKF_ENCRYPT | CKF_DECRYPT
DISABLED due to an unspecified cipherTextStealingVariant in configuration

As an exception, to improve the user experience, when no value is passed but the token is NSS ([by means of the token label](https://github.com/openjdk/jdk/blob/997777e86c6fa03f070dcf0f219813c11cb480ce/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java#L57-L64)), `cipherTextStealingVariant` [is assumed to be `CS1`](https://github.com/openjdk/jdk/blob/997777e86c6fa03f070dcf0f219813c11cb480ce/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Token.java#L425-L430).

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

PR Comment: https://git.openjdk.org/jdk/pull/18898#issuecomment-2138360201