[jdk11u-dev] RFR: 8255410: Add ChaCha20 and Poly1305 support to SunPKCS11 provider [v2]

[Martin Doerr]

On Thu, 10 Feb 2022 13:22:39 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

>> I backport this for parity with 11.0.15-oracle.
>> 
>> I had to do a row of adaptions. There are some implementation
>> differences, and many Java 17 usages that had to be changed.
>> 
>> In the original change in SunPKCS11.java, dA() is used 
>> to give alias names of algorithms. It calls to 
>> SecurityProviderConstants.getAliases().
>> In SunPKCS11.java of jdk11 neither dA() nore getAliases()
>> are available. It uses d() instead of da() and calls a 
>> s() with a list of literal strings to give the alias names.
>> SecurityProviderConstants.java is in java.base. The jdk11 
>> version does not contain the list of aliases that can be 
>> found in 17.
>> 
>> I looked up whether there are aliases listed for 
>> "ChaCha20-Poly1305" in 17, but found none, so I added
>> an empty call s() in SunPKCS11.java.
>> 
>> In P11AEADCipher.java I had to modify the syntax of 
>> a switch statement.
>> 
>> In CK_SALSA20_CHACHA20_POLY1305_PARAMS.java
>> I had to replace HexFormat.of().formatHex(...)
>> which is a java.util class not in 11.
>> 
>> The tests use HexFormat heavily.
>> I replaced it by HexToBytes() from TestKATForGCM and 
>> by toHexString() from TestLeadingZeroesP11.java.
>> To make these methods available I moved them to the 
>> superclass PKCS11Test.java. 
>> This way they can be used for potential later backports, too.
>> I also had to adapt a switch statement using '->' to syntax
>> know to 11.
>
> Goetz Lindenmaier has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
> 
>  - Fixes proposed by Martin. CK_SALSA: also don't capitalize Hex
>  - Backport 5d8c1cc8a05e0d9aedd6d54b8147d374c2290024

Please consider my proposal. Otherwise, good. Thanks for the backport work!

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line 635:

> 633:         d(AGP, "ChaCha20-Poly1305",
> 634:                 "com.sun.crypto.provider.ChaCha20Poly1305Parameters",
> 635:                 s("1.2.840.113549.1.9.16.3.18", "CHACHA20-POLY1305"),

I've looked a bit more through the code and it seems like the algorithm name is compared case insensitive (`algorithm = algorithm.toUpperCase(Locale.ENGLISH);` in P11SecretKeyFactory.java). So, it seems like "CHACHA20-POLY1305" is redundant because it matches the original name (case insensitive). Other places have an extra entry starting with "OID". I'd use: `s("1.2.840.113549.1.9.16.3.18", "OID.1.2.840.113549.1.9.16.3.18")` which looks consistent with the remaining code. Maybe another reviewer can confirm this.

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line 725:

> 723:         d(CIP, "ChaCha20-Poly1305",             P11AEADCipher,
> 724:                 s("1.2.840.113549.1.9.16.3.18", "CHACHA20-POLY1305"),
> 725:                 m(CKM_CHACHA20_POLY1305));

like above

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

Marked as reviewed by mdoerr (Reviewer).

PR: https://git.openjdk.java.net/jdk11u-dev/pull/805