RFR: 8255557: Decouple GCM from CipherCore [v4]

Valerie Peng valeriep at openjdk.java.net
Thu Jun 3 22:34:07 UTC 2021


On Wed, 2 Jun 2021 03:18:49 GMT, Anthony Scarpino <ascarpino at openjdk.org> wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 942:
>> 
>>> 940: 
>>> 941:             System.arraycopy(out, originalOutOfs, originalOut, originalOutOfs,
>>> 942:                 len);
>> 
>> Don't you need to do `originalOut = null;` after copying the bytes over? Otherwise, if you have two overlapping calls with the same engine, the 2nd restoreOut(...) call may lead to data corruption, i.e. it will duplicate the output bytes to the original output buffer (in the 1st overlapping call).
>> Same applies for the ByteBuffer case, i.e. restoreDst(...).
>> If time permits, please add a regression test to cover this.
>
> A engine is a one time use, so setting originalOut to null isn't necessary.  engineDoFinal() sets engine = null.

engine is one time use per encryption/decryption. But 'originalOut' is for overlap detection/protection which may be used multiple times during multi-part encryption/decrypion. For each overlapDetection()/restoreOut() pair, the 'originalOut' value should be cleared, otherwise there may be cases where the old value of 'originalOut' gets used?

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

PR: https://git.openjdk.java.net/jdk/pull/4072



More information about the security-dev mailing list