On Fri, 13 Nov 2020 22:25:13 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> Anthony Scarpino has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Code review comment update
>> Major change to test to detect corruption with incremental buffers test
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
> line 777:
>
>> 775: if ((src.remaining() + ((ibuffer != null) ? ibuffer.size() : 0)
>> -
>> 776: tagLenBytes) > dst.remaining()) {
>> 777: throw new RuntimeException("output buffer too small");
>
> Shouldn't this be ShortBufferException instead of RuntimeException?
I thought so too, but that isn't what GCTR returns. All the GCTR checks are
RuntimeExceptions. This check was original inside of GCTR, but I had to bring
it out because of the ibuffer lengths. I don't mind changing, it's just a
strange inconsistency.
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
> line 822:
>
>> 820: if (len > dst.remaining()) {
>> 821: throw new ShortBufferException("Output buffer too small");
>> 822: }
>
> How is this different from the check at line 775-778 at the beginning of this
> method?
Ah.. good.. I'll remove the one above from GCTR as I liked this check better
and it protects the GCTR update ()
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java
> line 818:
>
>> 816: // do this check here can also catch the potential integer
>> overflow
>> 817: // scenario for the subsequent output buffer capacity check.
>> 818: checkDataLength(0, len);
>
> Perhaps this can be moved up to the beginning of this method just like the
> dst size check?
You don't know what 'len', which includes ibuffer data, until this point in the
code.
-------------
PR: https://git.openjdk.java.net/jdk/pull/411
