On Mon, 2 Nov 2020 22:30:45 GMT, Anthony Scarpino <ascarp...@openjdk.org> wrote:
>>> * It was my expectation that checkOutputCapacity() is making sure all
>>> the inOfs ==<> outOfs are going to work. Does that not catch all cases?
>> checkOutputCapacity() as the name has, only changes that the output buffer
>> is large enough.
>>
>>> * outWithPadding" is excessive because it doubles the allocation for
>>> every operation followed by a copy to the original buffer, even if the
>>> original buffer was adequate. I'm ok with doing the extra alloc & copy in
>>> certain situations, but not everytime. Can you be more specific what
>>> things may fail that we don't already check for in the regression tests?
>>
>> For example,
>> 1) various input/output offset combinations, e.g. inOfs <,=,> outOfs,
>> 2) Given a output buffer of 200-byte and outOfs == 0, assuming the returned
>> decrypted data length is 160 bytes, the bytes from index 160 and onward
>> should not be overwritten. GCM has no padding, so you may not notice any
>> difference if this is what you are testing. This is generic CipherCore code
>> and changes impact all ciphers.
>
> checkOutputCapacity: Yes.. The method includes the offsets for the output
> buffer, which I believe would verify that the output area in the buffer with
> offsets is large enough.
>
> outWithPadding: I understand the situation and I am assuming there are tests
> that cover this case. Given it's a generic situation.
Have you tested the outWithPadding situation? Given that the existing impl only
write out the final result, I don't think you can assume that existing tests
cover it. I have wrote a simple test to check it if you have not done so, can
you try it out to be sure?
import java.io.PrintStream;
import java.util.*;
import java.security.*;
import java.security.spec.*;
import javax.crypto.*;
import javax.crypto.spec.*;
public class TestDoFinal {
private static String ALGO = "AES";
private static int BLK_SIZE = 16;
public static void main(String args[]) throws Exception {
byte[] in = new byte[32];
Arrays.fill(in, (byte)8);
KeyGenerator kg = KeyGenerator.getInstance(ALGO, "SunJCE");
SecretKey skey = kg.generateKey();
Cipher ci = Cipher.getInstance(ALGO + "/CBC/PKCS5Padding", "SunJCE");
ci.init(Cipher.ENCRYPT_MODE, skey);
int inLen = in.length - BLK_SIZE;
byte[] out = ci.doFinal(in, 0, inLen);
System.out.println("=> enc " + inLen + " bytes, ret " +
(out == null? "null":(out.length + " byte")));
AlgorithmParameters param = ci.getParameters();
ci.init(Cipher.DECRYPT_MODE, skey, param);
int rLen = ci.doFinal(out, 0, out.length, in);
System.out.println("=> dec " + out.length + " bytes, ret " +
rLen + " byte");
// check if more than rLen bytes are written into 'in'
for (int j = rLen; j < in.length; j++) {
if (in[j] != (byte)8) {
throw new Exception("Value check failed at index " + j);
}
}
System.out.println("Test Passed");
}
}
-------------
PR: https://git.openjdk.java.net/jdk/pull/411
