On Tue, 4 Jun 2024 02:55:30 GMT, Martin Balao <[email protected]> wrote:
>> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java
>> line 922:
>>
>>> 920: 0, out, outOfs, outLen);
>>> 921: }
>>> 922: if (paddingObj != null) {
>>
>> Why not put this if-block inside an else-block of the `if (blockMode ==
>> Mode.CTS),` so it's clear that CTS mode won't use paddingObj?
>
> I'd rather not add a level of indentation to the else-block, but perhaps we
> can add an else-if block to the `paddingObj != null` block. @franferrax, what
> do you think?
I wouldn't increase indentation either. Regarding adding an `else if`, I'm
neither for nor against it, so I can do that if that's your preference.
I agree that added here in line **922** of encryption case, it would make it
clearer that CTS mode won't use `paddingObj`:
https://github.com/openjdk/jdk/blob/997777e86c6fa03f070dcf0f219813c11cb480ce/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java#L917-L940
However, it would also introduce an asymmetry with the decryption case in line
**965**, where we can't do the same, since the code inside the `else` of line
**983** must also be executed in CTS mode:
https://github.com/openjdk/jdk/blob/997777e86c6fa03f070dcf0f219813c11cb480ce/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java#L957-L987
----
NOTE: this discussion also applies to the [same block in the `ByteBuffer`
version of
`P11Cipher::implDoFinal()`](https://github.com/openjdk/jdk/blob/997777e86c6fa03f070dcf0f219813c11cb480ce/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java#L1030-L1104):
an `else if` can be introduced line **1036** but not in line **1079**.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18898#discussion_r1625948565
