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

Wed Jun 5 19:06:14 UTC 2024

> 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 bu
 ffering 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 data source (`padBuffer` or input...

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 13 additional commits since the last revision:

 - Fix penultimate block length calculation

   It is not correct to calculate the penultimate block length based on
   the output array offset, since the output array can include arbitrary
   user-supplied data.

   Add a test case to check this fix.

   Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
   Co-authored-by: Martin Balao <mbalao at redhat.com>
 - Extract swapLastTwoBlocks() unified logic

   Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
   Co-authored-by: Martin Balao <mbalao at redhat.com>
 - Merge 'openjdk/master' into JDK-8330843
 - Apply code-review suggestion

   Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
   Co-authored-by: Martin Balao <mbalao at redhat.com>
 - Improve handling when the token variant is unknown

   Avoid registering CTS algorithms (those depending on CKM_AES_CTS) when
   the token CTS variant has not been specified in the configuration. Make
   NSS an exception, as we know that it uses the CS1 variant.

   Take advantage to extract a pkcs11.Config::parseEnumEntry() method for
   a cleaner entry in the main switch statement of pkcs11.Config::parse(),
   also slightly improving the error message.

   Co-authored-by: Francisco Ferrari <fferrari at redhat.com>
   Co-authored-by: Martin Balao <mbalao at redhat.com>
 - Merge 'openjdk/master' into JDK-8330843
 - 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>
 - ... and 3 more: https://git.openjdk.org/jdk/compare/f2bfb671...37d6eb80

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/18898/files
  - new: https://git.openjdk.org/jdk/pull/18898/files/0d82e7e9..37d6eb80

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18898&range=04
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18898&range=03-04

  Stats: 18948 lines in 507 files changed: 11911 ins; 4963 del; 2074 mod
  Patch: https://git.openjdk.org/jdk/pull/18898.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18898/head:pull/18898

PR: https://git.openjdk.org/jdk/pull/18898