On Mon, 3 Jun 2024 21:49:58 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> 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
>> ten additional commits since the last revision:
>>
>> - 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 <fferr...@redhat.com>
>> Co-authored-by: Martin Balao <mba...@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 <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,...
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line
> 621:
>
>> 619: int flushFromPadBuffer;
>> 620: int fillLen = getBytesToCompleteBlock(padBufferLen);
>> 621: if (dataForP11Update >= padBufferLen + fillLen) {
>
> Maybe use `if (inLen >= fillLen)` ?
`dataForP11Update >= padBufferLen + fillLen` is not the same as `inLen >=
fillLen` (the equivalent would be `inLen - newPadBufferLen >= fillLen`, but I
personally find the proposed condition more clear). We will flush the entire
`padBuffer` only if there are enough bytes in `inLen` to fill `padBuffer` with
whatever we need (0 or more bytes) and fulfill the new buffering requirement.
Regarding `fillLen > 0`, that is not strictly needed to flush the entire
`padBuffer`. If we are buffering 3 blocks (e.g. for NSS), we may have 1 block
buffered in `padBuffer` and `fillLen == 0` (no need to borrow to complete
`padBuffer` to a multiple of a block size).
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java line
> 782:
>
>> 780: int flushFromPadBuffer;
>> 781: int fillLen =
>> getBytesToCompleteBlock(padBufferLen);
>> 782: if (dataForP11Update >= padBufferLen + fillLen)
>> {
>
> Same suggestion as before.
Replied above.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18898#discussion_r1625121859
PR Review Comment: https://git.openjdk.org/jdk/pull/18898#discussion_r1625122157
