On Tue, 25 Oct 2022 21:57:34 GMT, Jamil Nimeh <jni...@openjdk.org> wrote:
>> vpaprotsk has updated the pull request incrementally with one additional >> commit since the last revision: >> >> extra whitespace character > > src/java.base/share/classes/com/sun/crypto/provider/Poly1305.java line 296: > >> 294: keyBytes[12] &= (byte)252; >> 295: >> 296: // This should be enabled, but Poly1305KAT would fail > > I'm on the fence about this change. I have no problem with it in basic > terms. If we ever decided to make this a general purpose Mac in JCE then > this would definitely be good to do. As of right now, the only consumer is > ChaCha20 and it would submit a key through the process in the RFC. Seems > really unlikely to run afoul of these checks, but admittedly not impossible. > > I would agree with @sviswa7 that we could examine this in a separate change > and we could look at other approaches to getting around the KAT issue, > perhaps some package-private based way to disable the check. As long as > Poly1305 remains with package-private visibility, one could make another form > of the constructor with a boolean that would disable this check and that is > the constructor that the KAT would use. This is just an off-the-cuff idea, > but one way we might get the best of both worlds. > > If we move this down the road then we should remove the commenting. We can > refer back to this PR later. I think I will remove the check for now, dont want to hold up reviews. I wasn't sure how to 'inject a backdoor' to the commented out check either, or at least how to do it in an acceptable way. Your ideas do sound plausible, and if anyone does want this check, I can implement one of the ideas (package private boolean flag? turn it on in the test) while waiting for more reviews to come in. The comment about ChaCha being the only way in is also relevant, thanks. i.e. this is a private class today. ------------- PR: https://git.openjdk.org/jdk/pull/10582
