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 results: i) Default (no JVM options, non-intrinsics) mode: a) Encryption: the new code performed better for both architectures tested (x86: +7.6%, arm64: +3.5%) Analysis: the new code makes some optimizations in the last cipher round with the T2 lookup table that Cryptix may not, hence the better performance in non-intrinsics. b) Decryption: the new code performed mixed between architectures tested (x86: +8.3%, arm64: -1.1%) Analysis: the new code performs predominately better, except for decryption on arm64, which we believe is negligible and acceptable to have noticeably better performance with x86 decryption. ii) Default (no JVM options, intrinsics) mode: a) Encryption and Decryption: as expected, both the new code and Cryptix code performed similarly (within the error margins) Analysis: this is from the fact that the intrinsics related code has not changed with the new changes. iii) Interpreted-only (-Xint) mode: a) Encryption: the new code performed better than the Cryptix code for both architectures (x86: +0.6%, arm64: +6.0%). b) Decryption: the new code performed slightly worse than the Cryptix code for both architectures (x86: -3.3%, arm64: -2.4%). Analysis: the design of the new code was focused on instruction efficiency; eliminating unnecessary index variables, rolling out the rounds loop, and using no objects for round and inverse round transforms. This is especially noticeable in arm64 encryption, but we believe that decryption's slight drop in performance is negligible. iv) JIT compiler (-Xcomp) mode: a) Encryption: in this mode, performance is mixed performant between the two architectures tested (x86: +11.7%, arm64: +1.5%). b) Decryption: performance is decreases for both of the architectures tested (x86: -4.9%, arm64: -3.2%). Analysis: As with the no options results, we believe that the increase in performance for both architectures in encryption is most likely from the T2 gadgetry that we've implemented. We believe that the slight performance drop in decryption is negligible. In any case, the -Xcomp option is primarily used for debugging purposes, ergo we are not as concerned about this slight drop. Resource utilization: ---------------------------- The new AES code uses similar resources to that of the existing Cryptix code. Memory allocation has the following characteristics: i) Peak allocation for both Cryptix and the new code is only a fraction of a percentage point different for both the 1 cipher object and 10 cipher objects test. Analysis: We believe that this is negligible given the difference in the 20ms to 50ms window of peak allocation. ii) Total GC pause for Cryptix and the new code only differs by less than 5% for both the 1 object and 10 objects test. Analysis: This is acceptable behavior given that the benchmark performance for the new code is better overall. iii) Peak pre-GC allocation for Cryptix and the new code is only a fraction of a percent more for the new code in the 1 object case and is only 2% more for the 10 objects case. Analysis: These differences indicate ~500 bytes per object discrepancy between the Cryptix and new code, which is also negligible. ------------- Commit messages: - Implement the rest of the class name changes in intrinsics - 8326609: New AES implementation with updates specified in FIPS 197 Changes: https://git.openjdk.org/jdk/pull/27807/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=27807&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8326609 Stats: 2989 lines in 5 files changed: 1506 ins; 1473 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/27807.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/27807/head:pull/27807 PR: https://git.openjdk.org/jdk/pull/27807
