RFR: JDK-8338587 - Internal XOF Methods for SHAKE128 and SHAKE256

[Weijun Wang]

On Mon, 19 Aug 2024 19:38:37 GMT, Ferenc Rakoczi <duke at 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