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

Xue-Lei Andrew Fan xuelei at openjdk.org
Wed Sep 14 18:27:42 UTC 2022


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

>> Actually, NM, init still has to call MessageDigest.isEqual so eliminating keys of invalid length before that is probably more efficient.
>
> Good point.  Modified to use less checking.
> 
> If the key is null, the following condition could bypass the checking, and result in NPE.
> 
> `        if (!MessageDigest.isEqual(key, lastKey)) {
> `
> 
> Although it is unlikely to happen as the caller should has already been checked that the key cannot be null, but the code logic here is not that clear to read.  In the new patch, I have the null checking in the init() method, and the validity checking in the makeSessionKey() method.

> Actually, NM, init still has to call MessageDigest.isEqual so eliminating keys of invalid length before that is probably more efficient.

The key should be valid for common cases.  For valid key, it is more efficient to have the checking in makeSessionKey() as there is less checking.  For invalid key, it is more efficient to have the checking before calling MessageDigest.isEqual().  Exception itself is costly, I would prefer to have better performance for common cases (valid key).

I updated the patch before I read the comment.  Please let me know your preference.  I'm fine to rollback if you prefer the old patch.

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

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



More information about the security-dev mailing list