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

[Francisco Ferrari Bihurriet]

On Wed, 5 Jun 2024 19:21:02 GMT, Valerie Peng <valeriep at 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 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 Ciph...
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Config.java line 647:
> 
>> 645:             return Enum.valueOf(enumClass, value);
>> 646:         } catch (IllegalArgumentException ignored) {
>> 647:             throw excToken(keyword + " must be one of " +
> 
> nit: since we are using the `value` for enum conversion, maybe using it instead of `keyword` for the error message? This has the benefit of showing what the parsed value is.

The `value` is completed form the current token by `excToken()`.

As mentioned in https://github.com/openjdk/jdk/pull/18898#issuecomment-2138360201, if we pass `CS4` (an invalid value), we get the following error:

cipherTextStealingVariant must be one of [CS1, CS2, CS3], read: Token[CS4], line 33


Other attributes are parsed in the same way, with similar error messages.

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

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