RFR: 8293779: redundant checking in AESCrypt.makeSessionKey() method [v2]

Sean Mullan mullan at openjdk.org
Wed Sep 14 17:38:51 UTC 2022


On Wed, 14 Sep 2022 16:03:48 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> Hi,
>> 
>> Please review this simple code cleanup.
>> 
>> The following checking for key in the makeSessionKey() method is redundant as it the same checking has been performance before calling the method.
>> 
>> 
>>         if (k == null) {
>>             throw new InvalidKeyException("Empty key");
>>         }
>>         if (!isKeySizeValid(k.length)) {
>>              throw new InvalidKeyException("Invalid AES key length: " +
>>                                            k.length + " bytes");
>>         }
>> 
>> 
>> No new regression test, simple cleanup.
>> 
>> Thanks,
>> Xuelei
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - remove unused throws in comment
>  - remove unused throws

src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java line 605:

> 603:      */
> 604:     private void makeSessionKey(byte[] k) throws InvalidKeyException {
> 605:         int ROUNDS          = getRounds(k.length);

You could have left the check in here and removed the duplicate check from `init()`. Since the key is not referenced by `init` unless the key is different from the last key, it seems cleaner to leave it here, and I think the check would be invoked fewer times if the same key is reused.

-------------

PR: https://git.openjdk.org/jdk/pull/10263



More information about the security-dev mailing list