RFR: 8253821: Improve ByteBuffer performance with GCM [v4]

[Valerie Peng]

On Mon, 2 Nov 2020 22:30:45 GMT, Anthony Scarpino <ascarpino at 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