On Fri, 17 Oct 2025 06:56: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
test/micro/org/openjdk/bench/javax/crypto/AESDecrypt.java line 44:
> 42:
> 43: @Param("10000000")
> 44: private int count;
Drive-by comment / question:
Did you do all benchmarking with this single (quite large) size? How are the
results for much smaller sizes? It may be worth it to just get a nice plot that
goes over a range of sizes, to see if it behaves as expected.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2439948821
