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

[Francisco Ferrari Bihurriet]

On Wed, 5 Jun 2024 03:49:31 GMT, Martin Balao <mbalao at openjdk.org> wrote:

>> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line 1183:
>> 
>>> 1181:                 // Temporary buffer to the penultimate block
>>> 1182:                 ciphertextBuf.put(start, tmp);
>>> 1183:             } else {
>> 
>> Personally, I find it easier to follow if this code block follows the decrypt case (line 1184-1190), the allocated `tmp` could be smaller, e.g.
>> Suggestion:
>> 
>>                 byte[] tmp = new byte[pad];
>>                 // .... pp[pp] ffff -> .... ffff pp[pp]
>>                 ciphertextBuf.get(start, tmp);
>>                 ciphertextBuf.put(start, ciphertextBuf, end - blockSize, blockSize);
>>                 ciphertextBuf.put(end - pad, tmp);
>> 
>> Have you considered this?
>
> I have no personal preference, but would suggest that if we change it to cut the pad, we keep the decryption case aligned.

What I like about this suggestion is that it allows unifying the repeated logic: the two blocks inside `if (encrypt)` and the corresponding `else` would become almost identical, allowing an additional abstraction. How about the following?


    private void convertCTSVariant(ByteBuffer ciphertextBuf,
            byte[] ciphertextArr, int end) {
        // [...]
        // [...] omitted code
        // [...]
        if (ciphertextBuf != null) {
            pad = pad == 0 ? blockSize : pad;
            if (encrypt) {
                // .... pp[pp] ffff -> .... ffff pp[pp]
                swapLastTwoBlocks(ciphertextBuf, end, pad, blockSize);
            } else {
                // .... ffff pp[pp] -> .... pp[pp] ffff
                swapLastTwoBlocks(ciphertextBuf, end, blockSize, pad);
            }
        }
    }

    private static void swapLastTwoBlocks(ByteBuffer buffer, int end,
            int prevBlockLen, int lastBlockLen) {
        // .... prevBlock lastBlock -> .... lastBlock prevBlock
        int prevBlockStart = end - prevBlockLen - lastBlockLen;
        byte[] prevBlockBackup = new byte[prevBlockLen];
        buffer.get(prevBlockStart, prevBlockBackup);
        buffer.put(prevBlockStart, buffer, end - lastBlockLen, lastBlockLen);
        buffer.put(end - prevBlockLen, prevBlockBackup);
    }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18898#discussion_r1627704892