On Sun, 19 Oct 2025 02:18:43 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 Just have some minor questions and nit. src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 589: > 587: > 588: // Lookup table for inverse substitution transform of last round as > 589: // described in the international journal article referenced. Is there a link that I can look it up also? src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 1180: > 1178: ^ T3[(ti0 >> 16) & 0xFF] & 0xFF0000 > 1179: ^ T0[(ti1 >> 8) & 0xFF] & 0xFF00 > 1180: ^ T1[ti2 & 0xFF] & 0xFF ^ K[w + 3]; Is this last round processing also based on spec or some journal? ------------- PR Review: https://git.openjdk.org/jdk/pull/27807#pullrequestreview-3358325489 PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2446391365 PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2446389723
