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

Weijun Wang weijun at openjdk.org
Wed Aug 28 14:05:22 UTC 2024


On Tue, 27 Aug 2024 13:05:42 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.
>
> Ferenc Rakoczi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Code style changes suggested by Andrey Turbanov

Mostly wording. One simplification suggestion.

src/java.base/share/classes/sun/security/provider/SHA3.java line 74:

> 72:     private final byte suffix;
> 73: 
> 74:     // the stae matrix flattened into an array

Typo: `stat` should be `state`.

src/java.base/share/classes/sun/security/provider/SHA3.java line 84:

> 82:     // calls) will set it to 0 at its start.
> 83:     // When a squeeze() call uses up all available bytes from this state
> 84:     // and so a new keccak() call is made, squeezeOffset is reset to 0.

The paragraph only mentions `set to 0` and `reset to 0`, and has not talked about other values. Maybe you can say when the squeeze call returns, it's set to a position that states before it have been used.

src/java.base/share/classes/sun/security/provider/SHA3.java line 136:

> 134:      */
> 135:     void implDigest(byte[] out, int ofs) {
> 136:         // Moving this allocation to the block where t is used causes a little

Typo, `t` should be `it`.

src/java.base/share/classes/sun/security/provider/SHA3.java line 170:

> 168: 
> 169:     void implSqueeze(byte[]output, int offset, int numBytes) {
> 170:         // Moving this allocation to the block where t is used causes a little

Typo. `t` should be `it`.

src/java.base/share/classes/sun/security/provider/SHA3.java line 225:

> 223:                 ((squeezeOffset + numBytes - longOffset * 8) + 7) / 8;
> 224: 
> 225:         int limit = longOffset + longsToConvert - (numBytes % 8 == 0 ? 0 : 1);

Can we also do the same simplification here as in `implDigest`?

        int longsToConvert =
                (squeezeOffset + numBytes - longOffset * 8) / 8;

        int limit = longOffset + longsToConvert;

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

PR Review: https://git.openjdk.org/jdk/pull/20631#pullrequestreview-2266256436
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1734672509
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1734678099
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1734732266
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1734735206
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1734743245


More information about the security-dev mailing list