On Tue, 28 Nov 2023 08:35:29 GMT, Daniel Jeliński <[email protected]> wrote:
>> Hi,
>>
>> I need a review for a new internal buffer class called AEADBufferStream.
>> AEADBufferStream extends ByteArrayOutputStream, but eliminates some data
>> checking and copying that are not necessary for what GaloisCounterMode.java
>> and ChaCha20Cipher.java need.
>>
>> The changes greatest benefit is with decryption operations.
>> ChaCha20-Poly1305 had larger performance gains by adopting similar
>> techniques that AES/GCM already uses.
>>
>> The new buffer shows up to 21% bytes/sec performance increase for decryption
>> for ChaCha20-Poly1305 and 12% for AES/GCM. 16K data sizes saw a memory
>> usage reduction of 46% with and 83% with ChaCha20-Poly1305. These results
>> come from the JMH tests updated in this request and memory usage using the
>> JMH gc profile gc.alloc.rate.norm entry
>>
>> thanks
>>
>> Tony
>
> src/java.base/share/classes/com/sun/crypto/provider/ChaCha20Cipher.java line
> 859:
>
>> 857: try {
>> 858: outArray = engineDoFinal(inArray, inOfs, inLen);
>> 859: } catch (BadPaddingException e) {
>
> Should we really be hiding the `BadPaddingException` here and in other places
> in this method? `AEADBadTagException` extends `BadPaddingException`, and I'm
> pretty sure we don't want to hide it.
CipherSpi:engineUpdate() only throws ShortBufferException. That forces many
exceptions into a ProviderException with extends RuntimeException. The
BadPaddingException doesn't matter because CC20 doesn't have padding.
AEADTagException should be specified and thrown, not wrapped.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16487#discussion_r1409787566
