On Thu, 16 Oct 2025 20:22:20 GMT, Shawn M Emery <[email protected]> wrote:
>> General:
>> -----------
>> i) This work is to replace the existing AES cipher under the Cryptix license.
>>
>> ii) The lookup tables are employed for performance, but also for operating
>> in constant time.
>>
>> iii) Several loops have been unrolled for optimization purposes, but are
>> harder to read and don't meet coding style guidelines.
>>
>> iv) None of the AES related intrinsics has been modified in this PR, but the
>> new code has been updated to use the intrinsics related hooks for the AES
>> block and key table arguments.
>>
>> Note: I've purposefully not seen the original Cryptix code, so when making a
>> code review comment please don't quote the code in the AESCrypt.java file.
>>
>> Correctness:
>> -----------------
>> The following AES-specific regression tests have passed in intrinsics
>> (default) and non-intrinsic (-Xint) modes:
>>
>> i) test/jdk/com/sun/crypto/provider/Cipher/AES: all 27 tests pass
>>
>> -intrinsics mode for:
>>
>> ii) test/hotspot/jtreg/compiler/codegen/aes: all 4 tests pass
>>
>> iii) jck:api/java_security, jck:api/javax_crypto, jck:api/javax_net,
>> jck:api/javax_security, jck:api/org_ietf, and jck:api/javax_xml/crypto:
>> passed, with 10 known failures
>>
>> iv) jdk_security_infra: passed, with 48 known failures
>>
>> v) tier1 and tier2: all 110257 tests pass
>>
>> Security:
>> -----------
>> In order to prevent side-channel (timing and differential power analysis)
>> attacks the code has been constructed to operate in constant time and does
>> not use conditionals based on the key or key expansion table. This is
>> accomplished by using lookup tables in both the cipher and inverse cipher of
>> AES.
>>
>> Performance:
>> ------------------
>> All AES related benchmarks have been executed against the new and original
>> Cryptix code:
>>
>> micro:org.openjdk.bench.javax.crypto.AES
>>
>> micro:org.openjdk.bench.javax.crypto.full.AESBench
>>
>> micro:org.openjdk.bench.javax.crypto.full.AESExtraBench
>>
>> micro:org.openjdk.bench.javax.crypto.full.AESGCMBench
>>
>> micro:org.openjdk.bench.javax.crypto.full.AESGCMByteBuffer
>>
>> micro:org.openjdk.bench.javax.crypto.full.AESGCMCipherInputStream
>>
>> micro:org.openjdk.bench.javax.crypto.full.AESGCMCipherOutputStream
>>
>> micro:org.openjdk.bench.javax.crypto.full.AESKeyWrapBench.
>>
>> micro:org.openjdk.bench.java.security.CipherSuiteBench (AES)
>>
>> The benchmarks were executed in different compiler modes (default (no
>> compiler options), -Xint, and -Xcomp) and on two different architectures
>> (x86 and arm64) with the following encryption re...
>
> 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;
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2438441223
