From 874414cbe6e30955450e13f29999fdec11af5f08 Mon Sep 17 00:00:00 2001 From: yperbasis Date: Fri, 14 Aug 2026 17:10:53 +0200 Subject: [PATCH 1/3] keccak: defer squeeze permutation until needed --- keccak_asm.go | 6 +++--- keccak_squeeze_asm_test.go | 39 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 keccak_squeeze_asm_test.go diff --git a/keccak_asm.go b/keccak_asm.go index 37ce284..0687d4d 100644 --- a/keccak_asm.go +++ b/keccak_asm.go @@ -95,13 +95,13 @@ func (s *sponge) Read(out []byte) (int, error) { n := len(out) for len(out) > 0 { - x := copy(out, s.state[s.readIdx:rate]) - s.readIdx += x - out = out[x:] if s.readIdx == rate { keccakF1600(&s.state) s.readIdx = 0 } + x := copy(out, s.state[s.readIdx:rate]) + s.readIdx += x + out = out[x:] } return n, nil } diff --git a/keccak_squeeze_asm_test.go b/keccak_squeeze_asm_test.go new file mode 100644 index 0000000..2823569 --- /dev/null +++ b/keccak_squeeze_asm_test.go @@ -0,0 +1,39 @@ +//go:build (amd64 || arm64) && !purego + +package keccak + +import ( + "strconv" + "testing" +) + +func TestSpongeReadDefersBoundaryPermutation(t *testing.T) { + var h sponge + h.Write([]byte("input")) + h.Read(make([]byte, rate)) + + if h.readIdx != rate { + t.Fatalf("read index after one full block = %d, want %d", h.readIdx, rate) + } + + var next [1]byte + h.Read(next[:]) + if h.readIdx != 1 { + t.Fatalf("read index after next byte = %d, want 1", h.readIdx) + } +} + +func BenchmarkSpongeRead(b *testing.B) { + for _, size := range []int{rate - 1, rate, rate + 1} { + b.Run(strconv.Itoa(size), func(b *testing.B) { + out := make([]byte, size) + b.SetBytes(int64(size)) + b.ReportAllocs() + for b.Loop() { + var h sponge + h.Write([]byte("input")) + h.Read(out) + } + }) + } +} From ff347a3088713323276f14f8e87ab409bdfb1355 Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Wed, 2 Sep 2026 11:14:43 +0700 Subject: [PATCH 2/3] keccak: guard the squeeze tests and check the squeezed bytes Both new cases drove sponge directly, so they reached the platform assembly with no useASM check and would SIGILL on a CPU without it. Fold them into keccak_asm_test.go, which already carries that convention, and compare Read(rate)+Read(k) against x/crypto instead of only pinning readIdx. --- keccak_asm_test.go | 55 ++++++++++++++++++++++++++++++++++++++ keccak_squeeze_asm_test.go | 39 --------------------------- 2 files changed, 55 insertions(+), 39 deletions(-) delete mode 100644 keccak_squeeze_asm_test.go diff --git a/keccak_asm_test.go b/keccak_asm_test.go index 44a8f64..630fbc6 100644 --- a/keccak_asm_test.go +++ b/keccak_asm_test.go @@ -3,8 +3,12 @@ package keccak import ( + "bytes" "math/rand" + "strconv" "testing" + + "golang.org/x/crypto/sha3" ) // TestXorAndPermute verifies the fused XOR+permute assembly entry point @@ -34,3 +38,54 @@ func TestXorAndPermute(t *testing.T) { } } } + +// TestSpongeReadDefersBoundaryPermutation covers Read(rate) followed by +// Read(k): the only sequence where moving the block permutation to the read +// that needs it can change the output. TestReadMatchesXCrypto reads once per +// hasher and TestReadMultipleCalls chunks by 37, so neither lands a call +// boundary on rate. +func TestSpongeReadDefersBoundaryPermutation(t *testing.T) { + if !useASM { + t.Skip("hardware acceleration unavailable on this CPU") + } + const input = "input" + for _, k := range []int{1, 7, rate - 1, rate, rate + 1} { + t.Run(strconv.Itoa(k), func(t *testing.T) { + ref := sha3.NewLegacyKeccak256().(KeccakState) + ref.Write([]byte(input)) + want := make([]byte, rate+k) + ref.Read(want) + + var s sponge + s.Write([]byte(input)) + got := make([]byte, rate+k) + s.Read(got[:rate]) + if s.readIdx != rate { + t.Fatalf("read index after one full block = %d, want %d", s.readIdx, rate) + } + s.Read(got[rate:]) + + if !bytes.Equal(got, want) { + t.Fatalf("Read(%d)+Read(%d) = %x, want %x", rate, k, got, want) + } + }) + } +} + +func BenchmarkSpongeRead(b *testing.B) { + if !useASM { + b.Skip("hardware acceleration unavailable on this CPU") + } + for _, size := range []int{rate - 1, rate, rate + 1} { + b.Run(strconv.Itoa(size), func(b *testing.B) { + out := make([]byte, size) + b.SetBytes(int64(size)) + b.ReportAllocs() + for b.Loop() { + var h sponge + h.Write([]byte("input")) + h.Read(out) + } + }) + } +} diff --git a/keccak_squeeze_asm_test.go b/keccak_squeeze_asm_test.go deleted file mode 100644 index 2823569..0000000 --- a/keccak_squeeze_asm_test.go +++ /dev/null @@ -1,39 +0,0 @@ -//go:build (amd64 || arm64) && !purego - -package keccak - -import ( - "strconv" - "testing" -) - -func TestSpongeReadDefersBoundaryPermutation(t *testing.T) { - var h sponge - h.Write([]byte("input")) - h.Read(make([]byte, rate)) - - if h.readIdx != rate { - t.Fatalf("read index after one full block = %d, want %d", h.readIdx, rate) - } - - var next [1]byte - h.Read(next[:]) - if h.readIdx != 1 { - t.Fatalf("read index after next byte = %d, want 1", h.readIdx) - } -} - -func BenchmarkSpongeRead(b *testing.B) { - for _, size := range []int{rate - 1, rate, rate + 1} { - b.Run(strconv.Itoa(size), func(b *testing.B) { - out := make([]byte, size) - b.SetBytes(int64(size)) - b.ReportAllocs() - for b.Loop() { - var h sponge - h.Write([]byte("input")) - h.Read(out) - } - }) - } -} From e610be511c0fd7bfb0e2ec456f795e0584e9af6d Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Wed, 2 Sep 2026 11:47:28 +0700 Subject: [PATCH 3/3] keccak: say what the squeeze test actually pins Deferring the permutation cannot change the output, so the docstring's claim was wrong: readIdx is the discriminating assertion. --- keccak_asm_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/keccak_asm_test.go b/keccak_asm_test.go index 630fbc6..46b890a 100644 --- a/keccak_asm_test.go +++ b/keccak_asm_test.go @@ -40,10 +40,10 @@ func TestXorAndPermute(t *testing.T) { } // TestSpongeReadDefersBoundaryPermutation covers Read(rate) followed by -// Read(k): the only sequence where moving the block permutation to the read -// that needs it can change the output. TestReadMatchesXCrypto reads once per -// hasher and TestReadMultipleCalls chunks by 37, so neither lands a call -// boundary on rate. +// Read(k), the call boundary neither TestReadMatchesXCrypto (one read per +// hasher) nor TestReadMultipleCalls (chunks of 37) lands on. Deferring the +// permutation is output-neutral, so readIdx is what pins the deferral; the +// byte comparison catches a reorder that permutes the wrong number of times. func TestSpongeReadDefersBoundaryPermutation(t *testing.T) { if !useASM { t.Skip("hardware acceleration unavailable on this CPU")