On Mon, 19 Aug 2024 19:38:37 GMT, Ferenc Rakoczi <d...@openjdk.org> wrote:
> In preparation for the new PQC algorithms implementations, internal XOF
> (eXtendable Output Function) methods are added to the SHAKE128 and SHAKE256
> implementations.
Added some comments. Also, I wish the digest and squeeze methods can reuse more
code.
src/java.base/share/classes/sun/security/provider/SHA3.java line 72:
> 70: private final byte suffix;
> 71: private long[] state = new long[DM*DM];
> 72: private int squeezeOffset = -1;
Add some comment for `squeezeOffset`.
src/java.base/share/classes/sun/security/provider/SHA3.java line 123:
> 121: */
> 122: void implDigest(byte[] out, int ofs) {
> 123: byte[] byteState = new byte[8];
`byteState` can be moved to line 150.
src/java.base/share/classes/sun/security/provider/SHA3.java line 154:
> 152: System.arraycopy(byteState, 0,
> 153: out, ofs, numBytes - (numLongs - 1) * 8);
> 154: }
I still think you can set `numLongs` to be `numBytes/8`. Then there is no need
to do a single set on line 149. The code will be
int numLongs = numBytes / 8;
for (int i = 0; i < numLongs; i++) {
asLittleEndian.set(out, ofs, state[i]);
ofs += 8;
}
if (numBytes % 8 != 0) {
byte[] byteState = new byte[8];
asLittleEndian.set(byteState, 0, state[numLongs]);
System.arraycopy(byteState, 0, out, ofs, numBytes % 8);
}
src/java.base/share/classes/sun/security/provider/SHA3.java line 171:
> 169: int availableBytes = blockSize - squeezeOffset;
> 170:
> 171: if (availableBytes == 0) {
Is this check really necessary? It looks like in the `while (numBytes >
availableBytes)` loop below no byte will be copied in the 1st round and
`keccak` will be called anyway.
src/java.base/share/classes/sun/security/provider/SHA3.java line 211:
> 209: squeezeOffset += bytesToCopy;
> 210: }
> 211:
Is it possible to add a return here if `numBytes` is already zero?
src/java.base/share/classes/sun/security/provider/SHA3.java line 434:
> 432: /*
> 433: * The SHAKE128 extendable output function.
> 434: */
Please add a comment describing what would happen if `update` is called after
`squeeze`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/20631#pullrequestreview-2258682171
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1730098979
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1729948885
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1729981162
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1730108440
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1730122283
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1730093125
