Skip to content

cksum: add length support for shake128 and shake256 validation#11320

Merged
RenjiSann merged 1 commit intouutils:mainfrom
AldanTanneo:shake_validation
Mar 15, 2026
Merged

cksum: add length support for shake128 and shake256 validation#11320
RenjiSann merged 1 commit intouutils:mainfrom
AldanTanneo:shake_validation

Conversation

@AldanTanneo
Copy link
Copy Markdown
Contributor

@AldanTanneo AldanTanneo commented Mar 13, 2026

As pure XOF hashes, shake* algos don't make much sense if we don't specify the length. This follows the current behaviour of the blake2b algo, where the length is appended (when not the default) in the same way as with the sha3 family.

It is unfortunate that bit and byte length are used in the same variable for different algos right now. I'm tempted to have another PR to make this uniform.

@AldanTanneo AldanTanneo changed the title add length support for shake128 and shake256 validation cksum: add length support for shake128 and shake256 validation Mar 13, 2026
@github-actions
Copy link
Copy Markdown

GNU testsuite comparison:

GNU test failed: tests/tail/retry. tests/tail/retry is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented Mar 14, 2026

I think length's unit is coming from https://codeberg.org/maandree/sha3sum .

@RenjiSann
Copy link
Copy Markdown
Collaborator

I agree the length option being undecidedly in bits or bytes is an issue, and we should try to stay coherent. The only issue I have with that is that we should make sure we are not going against strongly established existing implementations

On another topic, tbh I am not a fan of not writing the length in the tagged output because it is the default length, and I think we should make our output as explicit as possible (while still being able to decode tagged lines that are missing the length.

WDYT ?

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented Mar 15, 2026

Just follow with

$ cksum -a blake2b -l 512 /dev/null
BLAKE2b (/dev/null) = 786a02f742015903c6c6fd852552d272912f4740e15847618a86e217f71f5419d25e1031afee585313896444934eb04b903a685b1448b755d56f701afe9be2ce
$ cksum -a blake2b -l 512 /dev/null > /tmp/b
$ cat /tmp/b | cksum -c
/dev/null: OK
$ sed "s/BLAKE2b/BLAKE2b-512/" /tmp/b | cksum -c -
/dev/null: OK

?

@RenjiSann
Copy link
Copy Markdown
Collaborator

Just follow with

$ cksum -a blake2b -l 512 /dev/null
BLAKE2b (/dev/null) = 786a02f742015903c6c6fd852552d272912f4740e15847618a86e217f71f5419d25e1031afee585313896444934eb04b903a685b1448b755d56f701afe9be2ce
$ cksum -a blake2b -l 512 /dev/null > /tmp/b
$ cat /tmp/b | cksum -c
/dev/null: OK
$ sed "s/BLAKE2b/BLAKE2b-512/" /tmp/b | cksum -c -
/dev/null: OK

?

The more I think of it, the less I'm convinced.
Not writing down the XOF length in tagged mode when the length is default isn't a good practice imo, as it prevents the validation step afterwards from easily parsing the length. Also, it adds unnecessary overhead in the code to handle this specific case.

I think we can deviate from GNU behavior here and provide the XOF length every time. The output will still be compatible with GNU, though, so it's not a big deal.

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented Mar 15, 2026

Since GNU does not have definition for them yet, including length always is useful.

@github-actions
Copy link
Copy Markdown

GNU testsuite comparison:

Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/symlink (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/seq/seq-epipe is now being skipped but was previously passing.
Congrats! The gnu test tests/tail/tail-n0f is now passing!

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 15, 2026

Merging this PR will not alter performance

✅ 298 untouched benchmarks
⏩ 48 skipped benchmarks1


Comparing AldanTanneo:shake_validation (53bcdb9) with main (0f891cc)

Open in CodSpeed

Footnotes

  1. 48 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions
Copy link
Copy Markdown

GNU testsuite comparison:

Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/symlink (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/tail/tail-n0f is now passing!

@github-actions
Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/symlink (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/dd/no-allocate is now being skipped but was previously passing.
Note: The gnu test tests/seq/seq-epipe is now being skipped but was previously passing.

@RenjiSann RenjiSann merged commit d66817f into uutils:main Mar 15, 2026
160 checks passed
@AldanTanneo AldanTanneo deleted the shake_validation branch March 15, 2026 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants