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

[Martin Balao]

On Tue, 1 Mar 2022 14:54:11 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

>>> > I agree too, but let's wait for a comment from Martin.
>>> 
>>> You mean Martin Balao, right? He didn't react for 18 days now. 
>> 
>> I'm sure he never saw it. He's working on it at the moment.
>
>> I'm sure he never saw it. He's working on it at the moment.
> 
> Great, thank you!
> Reaching out to somebody is a bit problematic since github: Too many channels (PR, JBS, Mail), and too many mail notifications.  I missed things, too.

Hi @GoeLin,

Thanks for proposing this backports. Looks good to me and I agree with all the previous review comments and fixes.

One minor thing: it looks to me that HexToBytes does not exactly reflect HexFormat::parseHex behavior when it comes to handling null strings. While HexFormat::parseHex requires a non-null value (or throws a NPE) [1], HexToBytes will return an array of length 0. Returning an array of length 0, for HexFormat::parseHex, is reserved for the case in which the string is empty (non-null) only. There might be some performance differences too. Given that this is only used for tests, I believe that no changes are needed to the current PR. Something I was thinking is that HexFormat looks pretty self-contained so I wondered if it is worth moving it to an internal package of java.base (which could be for example jdk.internal.util) and exporting it to modules (such as jdk.crypto.cryptoki) on demand as backports use it. This would allow to use it even beyond SunPKCS11 in the future.

Martin.-

--
[1] - https://github.com/openjdk/jdk/blob/jdk-19+11/src/java.base/share/classes/java/util/HexFormat.java#L527

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

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