> 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 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 <fferr...@redhat.com>
   Co-authored-by: Martin Balao <mba...@redhat.com>
 - Merge 'openjdk/master' into JDK-8330843
 - 8330842: Add AES CBC with Ciphertext Stealing (CTS) SunPKCS11 tests
   
   Co-authored-by: Francisco Ferrari <fferr...@redhat.com>
   Co-authored-by: Martin Balao <mba...@redhat.com>
 - 8330842: Support AES CBC with Ciphertext Stealing (CTS) in SunPKCS11
   
   Co-authored-by: Francisco Ferrari <fferr...@redhat.com>
   Co-authored-by: Martin Balao <mba...@redhat.com>
 - Fix cipher tests logging
   
   Co-authored-by: Francisco Ferrari <fferr...@redhat.com>
   Co-authored-by: Martin Balao <mba...@redhat.com>
 - Implement integer constants as enum
   
   Co-authored-by: Francisco Ferrari <fferr...@redhat.com>
   Co-authored-by: Martin Balao <mba...@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 <fferr...@redhat.com>
   Co-authored-by: Martin Balao <mba...@redhat.com>

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/18898/files
  - new: https://git.openjdk.org/jdk/pull/18898/files/938aaaec..68197517

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

  Stats: 104288 lines in 2907 files changed: 65968 ins; 25486 del; 12834 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

Reply via email to