On Fri, 15 Nov 2024 18:55:44 GMT, Volodymyr Paprotski <vpaprot...@openjdk.org>
wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/CipherBlockChaining.java
>> line 65:
>>
>>> 63: // Should be large enough to provide intrinsic with optimization
>>> 64: // opportunities
>>> 65: private final int chunkSize = 1024*100;
>>
>> Shouldn't we call `getBlockSize()` of the parent class here rather than
>> using a hard-coded integer? Variable assignment to be done in constructor in
>> such case.
>
> I don't think this constant needs to be dynamic. The reason I mention
> blocksize, the intrinsic expects multiple of block size
> (`ArrayUtil.blockSizeCheck(plainLen, blockSize);` assert before-hand), but
> its otherwise unrelated to the block size. (i.e. "has to be large enough for
> optimizations").
>
> I believe this class can also be used by DES; iirc, blocksize=8. I suppose I
> could use `AESConstants.AES_BLOCK_SIZE*6400` as `chunkSize`. If that looks
> good, I will do that change, though I've seen '16' used as a raw number
> plenty of times in the package, without 'attribution' to AES_BLOCK_SIZE.
>
> And to be thorough, I should probably put an
> `ArrayUtil.blockSizeCheck(chunkSize, blockSize);` into the constructor? That
> way it is checked, but not on every call.
Yes, my concern is basically `intrinsic expects multiple of block size`. I
would simply declare the variable and then assign it in constructor when
`blockSize` is known, no need to call `blockSizeCheck`:
private final int chunkSize;
.........
CipherBlockChaining(SymmetricCipher embeddedCipher) {
super(embeddedCipher);
k = new byte[blockSize];
r = new byte[blockSize];
chunkSize = blockSize * 6400;
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22086#discussion_r1844375649
