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

Mon May 27 14:25:05 UTC 2024

On Fri, 24 May 2024 11:42:37 GMT, Francisco Ferrari Bihurriet <fferrari at openjdk.org> wrote:

>> Hi,
>> 
>> I would like to propose an implementation to support AES CBC with Ciphertext Stealing (CTS) in SunPKCS11, according to what has been specified in [JDK-8330843 CSR](https://bugs.openjdk.org/browse/JDK-8330843).
>> 
>> What follows are implementation notes that describe the most relevant aspects of the proposed change.
>> 
>> ### Buffering
>> 
>> A buffering mechanism was implemented so PKCS #11 tokens only receive multipart input updates (`C_EncryptUpdate` or `C_DecryptUpdate`) in multiples of the block size. This mechanism protects against tokens —such as NSS— that assume an update with data not multiple of the block size is final and do variant ordering between the last 2 blocks, returning an incorrect partial output and resetting state. For example, when encrypting, a token may receive an update with an input not multiple of the block size and prematurely finalize the operation returning the last two blocks of ciphertext according to its ordering. By buffering on the Java side, the token is not aware of the end of the stream during updates and SunPKCS11 retains full control to do the last two blocks at `C_EncryptFinal` or `C_DecryptFinal`. A bug in NSS (see [1823875](https://bugzilla.mozilla.org/show_bug.cgi?id=1823875#c2)) requires buffering three blocks instead of two. If this bug gets fixed, the three block b
 uffering will still work and allow it to support old versions of the NSS library.
>> 
>> ### `implUpdate` implementation
>> 
>> The code added to `P11Cipher::implUpdate` follows the existing three-stage strategy: 1) Process data in the `padBuffer`, 2) Process data in the input and 3) Buffer to `padBuffer`. Stage #1 for CTS has some additional complexity that is worth describing in this section.
>> 
>> The stage begins by calculating the total data available (what is already buffered in `padBuffer` + the new input) and assigning this value to the variable `totalInLen`. `newPadBufferLen` is the number of bytes that must be buffered at the end of the update operation, irrespective of where they come from (`padBuffer` or input). This number of bytes is determined out of `totalInLen` and based on the fact that each operation must retain bytes from the last two —or three, for NSS— blocks in `padBuffer`, or retain whatever is available if less than that. `dataForP11Update` is the number of bytes to be passed to the token during the operation (`C_EncryptUpdate` or `C_DecryptUpdate` calls), irrespective of the stage in which they are passed and of the dat...
>
> 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 #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 #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 #11 standard.

In the sake of interoperability, the PKCS #11 standard should choose one of the three variants. But since it is too late, PKCS #11 implementers supporting `CKM_AES_CTS` have already made their choice.

### OP-TEE provides a PKCS #<wbr/>11 interface, and has chosen CS3

Searching for other open source implementers, I have found that the [OP-TEE Trusted Execution Environment (TEE) exposes a PKCS #11 module](https://optee.readthedocs.io/en/latest/building/userland_integration.html). I don't know how widespread this project is, but it works as an example of how other implementors might behave.

This PKCS #11 implementation uses CS3 for `CKM_AES_CTS`, contrary to NSS' choice of CS1:

1. The `CKM_AES_CTS` constant [has the `0x1089` value](https://github.com/openjdk/jdk/blob/jdk-23+24/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11Constants.java#L889)
2. `CKM_AES_CTS` is [represented by the `PKCS11_CKM_AES_CTS` constant](https://github.com/OP-TEE/optee_os/blob/4.2.0/ta/pkcs11/include/pkcs11_ta.h#L1326) in OP-TEE
3. `PKCS11_CKM_AES_CTS` [is mapped to `TEE_ALG_AES_CTS`](https://github.com/OP-TEE/optee_os/blob/4.2.0/ta/pkcs11/src/processing_symm.c#L73)
4. When creating a `TEE_ALG_AES_CTS` context, [the `crypto_aes_cts_alloc_ctx()` function is used](https://github.com/OP-TEE/optee_os/blob/4.2.0/core/crypto/crypto.c#L136-L138)
5. This function [puts in the context a structure with operation callbacks that implement CTS](https://github.com/OP-TEE/optee_os/blob/4.2.0/core/crypto/aes-cts.c#L249)
6. The `cts_update.update` callback leads to `cts_update()`, which [then calls `cbc_cts_update()`](https://github.com/OP-TEE/optee_os/blob/4.2.0/core/crypto/aes-cts.c#L194-L195)
7. In `cbc_cts_update()`, [`len_last_block` is equal to `block_size` when the message would not require padding](https://github.com/OP-TEE/optee_os/blob/4.2.0/core/crypto/aes-cts.c#L113-L114)
8. Given the previous item fact, [the encryption code unconditionally swaps the last two blocks](https://github.com/OP-TEE/optee_os/blob/4.2.0/core/crypto/aes-cts.c#L130-L136), which corresponds to CS3
9. In addition, [CS3 is mentioned in a previous code comment](https://github.com/OP-TEE/optee_os/blob/4.2.0/core/crypto/aes-cts.c#L30)

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

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