On Wed, 26 Oct 2022 15:27:55 GMT, vpaprotsk <d...@openjdk.org> wrote:
>> 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. I flipped-flopped on this.. I already had the code for the exception.. and already described the potential fix. So rather then remove the code, pushed the described fix. Its always easier to remove the extra field I added. Let me know what you think about the 'backdoor' field. ------------- PR: https://git.openjdk.org/jdk/pull/10582
