RFR: 8326609: New AES implementation with updates specified in FIPS 197 [v8]

[Shawn M Emery]

On Fri, 17 Oct 2025 05:41:25 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Shawn M Emery has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Updates for code review comments from @valeriepeng
>
> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 911:
> 
>> 909:             }
>> 910:             sessionK[0] = genRoundKeys(key, rounds);
>> 911:             sessionK[1] = invGenRoundKeys();
> 
> Given the decryption round keys are somewhat based on the encryption round keys, we could combine these two methods into one, e.g.
> 
>  private static int[][] genRoundKeys(byte[] key, int rounds) {
>         int[][] ks = new int[2][]; // key schedule
> 
>         int wLen = (rounds + 1) * WB;
>         int nk = key.length / WB;
> 
>         // generate the round keys for encryption
>         int[] w = new int[wLen];
>         for (int i = 0, j = 0; i < nk; i++, j+=4) {
>             w[i] = ((key[j] & 0xFF) << 24)
>                     | ((key[j + 1] & 0xFF) << 16)
>                     | ((key[j + 2] & 0xFF) << 8)
>                     | (key[j + 3] & 0xFF);
>         }
>         for (int i = nk; i < wLen; i++) {
>             int tmp = w[i - 1];
>             if (i % nk == 0) {
>                 int rW = (tmp << 8) & 0xFFFFFF00 | (tmp >>> 24);
>                 tmp = subWord(rW) ^ RCON[(i / nk) - 1];
>             } else if ((nk > 6) && ((i % nk) == WB)) {
>                 tmp = subWord(tmp);
>             }
>             w[i] = w[i - nk] ^ tmp;
>         }
>         ks[0] = w;
> 
>         // generate the decryption round keys based on encryption ones
>         int[] dw = new int[wLen];
>         int[] temp = new int[WB];
> 
>         // Intrinsics requires the inverse key expansion to be reverse order
>         // except for the first and last round key as the first two round keys
>         // are without a mix column transform.
>         for (int i = 1; i < rounds; i++) {
>             System.arraycopy(w, i * WB, temp, 0, WB);
>             invMixRKey(temp);
>             System.arraycopy(temp, 0, dw, wLen - (i * WB), WB);
>         }
>         // dw[0...3] <- w[0...3] AND dw[4...7] <- w[(wLen - 4)...(wLen -1)]
>         System.arraycopy(w, 0, dw, 0, WB);
>         System.arraycopy(w, wLen - WB, dw, WB, WB);
>         ks[1] = dw;
>         Arrays.fill(temp, 0);
> 
>         return ks;
>     }

These two methods were only the few that I was able to make that were compact and singular in purpose (gen round key, gen inverse round key) code as the coding style guidelines espouse.  The rest of the methods' construction were dictated by performance improvements, where compactness came at the cost of interpreter speed.

> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 958:
> 
>> 956:      * @return the processed round key row.
>> 957:      */
>> 958:     private static int invMix(int[] state, int idx) {
> 
> It seems that we can just use an `int` argument and make the callers do the array dereferencing. This way we can get rid of the temporary buffer inside `invMixRKey(int[])` as passing an integer to `invMix(int)` method will not affect the array, e.g.
> 
>     private static void invMixRKey(int[] state) {
>         state[0] = invMix(state[0]);
>         state[1] = invMix(state[1]);
>         state[2] = invMix(state[2]);
>         state[3] = invMix(state[3]);
>     }

I've removed this method and inlined this logic in the invGenRoundKeys method.

> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 976:
> 
>> 974:      * @param state [in, out] the round key for inverse mix column processing.
>> 975:      */
>> 976:     private static void invMixRKey(int[] state) {
> 
> nit: name the method "invMixColumns(int[])". This name matches the spec psuedo code and goes better with the "state" argument name. Or use "invMixRoundKey(int[] roundKey)"?

I've removed this method and inlined this logic in the invGenRoundKeys method.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2438587221
PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2438587085
PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2438586207