RFR: 8326609: New AES implementation with updates specified in FIPS 197 [v5]
Shawn M Emery
duke at openjdk.org
Thu Oct 16 04:04:25 UTC 2025
On Thu, 16 Oct 2025 00:18:08 GMT, Valerie Peng <valeriep at openjdk.org> wrote:
>> Shawn M Emery has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Updates for code review comments from @valeriepeng
>
> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 896:
>
>> 894: if (key.length == AESConstants.AES_KEYSIZES[0]) {
>> 895: rounds = AES_128_ROUNDS;
>> 896: nk = AESConstants.AES_KEYSIZES[0]/WB;
>
> Looks like we can get rid of `nk` as the `genRKeys(byte[])` method can calculate/derive it based on the specified `key` argument, i.e. `key.length >> 2` or `key.length / WB`
Sounds reasonable. Fixed.
> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 932:
>
>> 930: * @return w the cipher round keys.
>> 931: */
>> 932: private int[] genRKeys(byte[] key, int nk) {
>
> nit: The spec names this "keyExpansion" and documents it under section 5.2. Could we include this in the method javadoc? Also, how about "genRoundKeys" or "doKeyExpansion" as method name? `nk` can be derived from `key` and maybe no need for extra argument?
Actually I used Stallings' cryptography book specifically for the round key concepts, hence the variable name mismatch at least for 128 bit keys. But after your suggestions is does look more like FIPS 192-upd 1 so I will reference the section ;) Fixed.
> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 932:
>
>> 930: * @return w the cipher round keys.
>> 931: */
>> 932: private int[] genRKeys(byte[] key, int nk) {
>
> This method can be static if you pass the `rounds` value as a method argument.
Yes and subWord() would also need to be static for this change. Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2434497086
PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2434497821
PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2434498001
More information about the security-dev
mailing list