On Wed, 15 Oct 2025 05:51:40 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: > > Add remaining files to be staged src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 43: > 41: * > https://www.internationaljournalcorner.com/index.php/ijird_ojs/article/view/134688 > 42: */ > 43: public final class AES_Crypt extends SymmetricCipher { This internal class does not need to be public? I'd assume it's only used within the same package? test/micro/org/openjdk/bench/javax/crypto/AESDecrypt.java line 53: > 51: public void setup() throws Exception { > 52: SecretKeySpec keySpec = new SecretKeySpec(new byte[]{-80, -103, > -1, 68, -29, -94, 61, -52, 93, -59, -128, 105, 110, 88, 44, 105}, "AES"); > 53: IvParameterSpec iv = new IvParameterSpec(new byte[]{0x00, 0x00, > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > 0x00}); Is the all-0s IV intentional? test/micro/org/openjdk/bench/javax/crypto/AESDecrypt.java line 82: > 80: public byte[] testUseAesIntrinsics() throws Exception { > 81: return cipher.doFinal(ct); > 82: } These 3 methods look same to me except for the method names? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2433490853 PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2433487319 PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2433483607
