On Fri, 29 Aug 2025 17:16:45 GMT, Anthony Scarpino <[email protected]>
wrote:
>> This is a draft PR for early review with the following intent:
>>
>> i) This work is to replace the existing AES cipher under the Cryptix license
>> with an Oracle version.
>>
>> ii) The lookup tables are employed for performance, but also for operating
>> in constant time.
>>
>> iii) Several blocks statements are flattened for optimization purposes.
>>
>> Note: I have not seen the original Cryptix code, so please don't refer to
>> the deltas, but rather provide references based on the new AESCrypt.java
>> code itself.
>>
>> Updates in this delta:
>> Phase 2: Optimization - SW
>> Phase 3: Optimization - HW
>> Fix round key ordering for inverse
>> Cleanup comments and style
>> Remove extraneous code
>> Create constant-time execution - including inverse multiplication Remove
>> sensitive information - including temporary round key attributes
>
> src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java line 629:
>
>> 627: throw new InvalidKeyException ("Invalid algorithm name.");
>> 628: }
>> 629: if (key.length == AES_128_NKEYS) {
>
> A suggestion is the new style switch
>
> switch (key.length) {
> case AES_128_NKEYS -> { ... }
> ...
> default -> throws ...
> }
Thank you for the suggestion, I will make them in the next commit. See f819d9f.
> src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java line 642:
>
>> 640: "Invalid key length (" + key.length + ").");
>> 641: }
>> 642: if (!Arrays.equals(prevKey, key)) {
>
> You will want to use `MessageDigest.isEqual(...)` here as it's a constant
> time. I realize the usage of `MessageDigest` is strange, but the check
> method has never been moved to a generic utility java class.
Good catch! I will make this change in the next commit. See f819d9f.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26912#discussion_r2311809449
PR Review Comment: https://git.openjdk.org/jdk/pull/26912#discussion_r2311809957
