[go] crypto: add tests for the cipher.BlockMode interface
2 views
Skip to first unread message
Manuel Sabin (Gerrit)
Jun 27, 2024, 9:07:15 AM (7 days ago) Jun 27
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Manuel Sabin has uploaded the change for review![]( "Open in Gerrit")
Commit message
crypto: add tests for the cipher.BlockMode interface
This CL creates tests for the cipher.BlockMode interface in the new internal/cryptotest package. This set of tests is called from the tests of implementations of the BlockMode interface e.g. cbc_test.go
Updates #25309
Change-Id: I3685bbee24d08d66f5bb4b7f001cbf520c844881
Change diff
diff --git a/src/crypto/cipher/cbc_test.go b/src/crypto/cipher/cbc_test.go
new file mode 100644
index 0000000..83f803e
--- /dev/null
+++ b/src/crypto/cipher/cbc_test.go
@@ -0,0 +1,11 @@
+package cipher_test
+
+import (
+ "crypto/cipher"
+ "crypto/internal/cryptotest"
+ "testing"
+)
+
+func TestCBCBlockMode(t *testing.T) {
+ cryptotest.TestBlockMode(t, cipher.NewCBCEncrypter, cipher.NewCBCDecrypter)
+}
diff --git a/src/crypto/internal/cryptotest/blockmode.go b/src/crypto/internal/cryptotest/blockmode.go
new file mode 100644
index 0000000..aebffff
--- /dev/null
+++ b/src/crypto/internal/cryptotest/blockmode.go
@@ -0,0 +1,221 @@
+package cryptotest
+
+import (
+ "bytes"
+ "crypto/aes"
+ "crypto/cipher"
+ "crypto/des"
+ "fmt"
+ "io"
+ "math/rand"
+ "strings"
+ "testing"
+ "time"
+)
+
+// MakeBlockMode returns a cipher.BlockMode instance.
+// It expects len(iv) == b.BlockSize().
+type MakeBlockMode func(b cipher.Block, iv []byte) cipher.BlockMode
+
+// TestBlockMode performs a set of tests on cipher.BlockMode implementations,
+// checking the documented requirements of CryptBlocks.
+func TestBlockMode(t *testing.T, makeEncrypter, makeDecrypter MakeBlockMode) {
+ // Create one random number generator to use for all BlockMode tests
+ rng := newRandReader(t)
+
+ // Test BlockMode using AES block cipher for each of its key lengths
+ for _, keylen := range []int{128, 192, 256} {
+ t.Run(fmt.Sprintf("AES-%d", keylen), func(t *testing.T) {
+ // Generate a random IV and key to instantiate an AES block cipher
+ iv := make([]byte, aes.BlockSize)
+ rng.Read(iv)
+ key := make([]byte, keylen/8)
+ rng.Read(key)
+ cipher, err := aes.NewCipher(key)
+ if err != nil {
+ panic(err)
+ }
+ testBlockModePair(t, rng, makeEncrypter, makeDecrypter, cipher, iv)
+ })
+ }
+
+ t.Run("DES", func(t *testing.T) {
+ // Generate a random IV and key to instantiate a DES block cipher
+ key := make([]byte, 8)
+ rng.Read(key)
+ iv := make([]byte, des.BlockSize)
+ rng.Read(iv)
+ cipher, err := des.NewCipher(key)
+ if err != nil {
+ panic(err)
+ }
+ testBlockModePair(t, rng, makeEncrypter, makeDecrypter, cipher, iv)
+ })
+}
+
+func testBlockModePair(t *testing.T, rng io.Reader, e, d MakeBlockMode, b cipher.Block, iv []byte) {
+ t.Run("Encryption", func(t *testing.T) {
+ testBlockMode(t, rng, e, b, iv)
+ })
+
+ t.Run("Decryption", func(t *testing.T) {
+ testBlockMode(t, rng, d, b, iv)
+ })
+
+ t.Run("Cycle", func(t *testing.T) {
+ bs := e(b, iv).BlockSize()
+ if d(b, iv).BlockSize() != bs {
+ t.Skip("mismatching encryption and decryption blocksizes")
+ }
+
+ before, dst, after := make([]byte, bs*2), make([]byte, bs*2), make([]byte, bs*2)
+ rng.Read(before)
+
+ e(b, iv).CryptBlocks(dst, before)
+ d(b, iv).CryptBlocks(after, dst)
+ expectEqual(t, after, before, "plaintext is different after a encrypt/decrypt cycle")
+ })
+}
+
+func testBlockMode(t *testing.T, rng io.Reader, bm MakeBlockMode, b cipher.Block, iv []byte) {
+ bs := bm(b, iv).BlockSize()
+
+ t.Run("WrongIVLen", func(t *testing.T) {
+ iv := make([]byte, b.BlockSize()+1)
+ mustPanic(t, "IV length must equal block size", func() { bm(b, iv) })
+ })
+
+ t.Run("Aliasing", func(t *testing.T) {
+ src, dst, before := make([]byte, bs*2), make([]byte, bs*2), make([]byte, bs*2)
+ for _, length := range []int{0, bs, bs * 2} {
+ rng.Read(src)
+ copy(before, src)
+
+ bm(b, iv).CryptBlocks(dst[:length], src[:length])
+ expectEqual(t, src, before, "CryptBlocks modified src")
+
+ // CryptBlocks on the same src data should yield same output even if dst=src
+ copy(src, before)
+ bm(b, iv).CryptBlocks(src[:length], src[:length])
+ expectEqual(t, src[:length], dst[:length], "CryptBlocks behaves differently when dst = src")
+
+ rng.Read(src)
+ rng.Read(dst)
+ copy(before, dst)
+ bm(b, iv).CryptBlocks(dst, src[:length])
+ expectEqual(t, dst[length:], before[length:], "CryptBlocks modified dst past len(src)")
+ }
+ })
+
+ t.Run("BufferOverlap", func(t *testing.T) {
+ src := make([]byte, bs+1)
+ rng.Read(src)
+
+ // Make src and dst slices point to same array with inexact overlap
+ dst := src[1:]
+ src = src[:len(src)-1]
+
+ mustPanic(t, "invalid buffer overlap", func() { bm(b, iv).CryptBlocks(dst, src) })
+ })
+
+ // Input to CryptBlocks should be a multiple of BlockSize
+ t.Run("PartialBlocks", func(t *testing.T) {
+ // Check a few cases of not being a multiple of BlockSize
+ for _, srcSize := range []int{bs - 1, bs + 1, 2*bs - 1, 2*bs + 1} {
+ src := make([]byte, srcSize)
+ dst := make([]byte, 3*bs) // Make a dst large enough all src
+ mustPanic(t, "input not full blocks", func() { bm(b, iv).CryptBlocks(dst, src) })
+ }
+ })
+
+ t.Run("OutOfBoundsWrite", func(t *testing.T) { // Issue 21104
+ src, dst := make([]byte, bs*3), make([]byte, bs*3)
+ rng.Read(src)
+ copy(dst, src)
+ mustPanic(t, "output smaller than input", func() { bm(b, iv).CryptBlocks(dst[bs:bs*2], src) })
+
+ expectEqual(t, dst[bs*2:], src[bs*2:], "CryptBlocks did out of bounds write after end of dst slice")
+ expectEqual(t, dst[:bs], src[:bs], "CryptBlocks did out of bounds write before beginning of dst slice")
+ })
+
+ // Check that output of cipher isn't affected by adjacent data beyond input
+ // slice scope.
+ t.Run("OutOfBoundsRead", func(t *testing.T) {
+ src := make([]byte, bs)
+ rng.Read(src)
+ ctrlDst := make([]byte, bs) // Record a control output
+ bm(b, iv).CryptBlocks(ctrlDst, src)
+
+ // Make a buffer with src in the middle and data on either end
+ buff := make([]byte, bs*3)
+ copy(buff[bs:bs*2], src)
+ rng.Read(buff[:bs])
+ rng.Read(buff[bs*2:])
+
+ testDst := make([]byte, bs)
+ bm(b, iv).CryptBlocks(testDst, buff[bs:bs*2])
+
+ expectEqual(t, testDst, ctrlDst, "CryptBlocks affected by data outside of src slice bounds")
+ })
+
+ t.Run("KeepState", func(t *testing.T) {
+ src, serialDst, compositeDst := make([]byte, bs*4), make([]byte, bs*4), make([]byte, bs*4)
+ rng.Read(src)
+
+ length, block := 2*bs, bm(b, iv)
+ block.CryptBlocks(serialDst, src[:length])
+ block.CryptBlocks(serialDst[length:], src[length:])
+
+ bm(b, iv).CryptBlocks(compositeDst, src)
+
+ expectEqual(t, serialDst, compositeDst, "two successive CryptBlocks calls returned a different result than a single one")
+ })
+}
+
+func newRandReader(t *testing.T) io.Reader {
+ seed := time.Now().UnixNano()
+ t.Logf("Deterministic RNG seed: 0x%x", seed)
+ return rand.New(rand.NewSource(seed))
+}
+
+func expectEqual(t *testing.T, got, want []byte, msg string) {
+ if !bytes.Equal(got, want) {
+ t.Errorf("%s; got %x, want %x", msg, got, want)
+ }
+}
+
+// Check if function f panics with message msg and error otherwise.
+func mustPanic(t *testing.T, msg string, f func()) {
+ t.Helper()
+
+ defer func() {
+ t.Helper()
+
+ err := recover()
+
+ if err == nil {
+ t.Errorf("function did not panic, wanted %q", msg)
+ } else {
+ // Cast err to string
+ err := fmt.Sprintf("%v", err)
+
+ // Split message of form "path/to/block/directory: error message" at colon
+ panicMsg := strings.SplitN(err, ": ", 2)
+
+ // If the message didn't have a colon, it should
+ if len(panicMsg) == 1 {
+ t.Errorf(
+ "got panic %q, wanted %q",
+ panicMsg[0], "path/to/block/directory: "+msg)
+ return
+ }
+
+ panicFrom := panicMsg[0]
+ panicGot := panicMsg[1]
+ if panicGot != msg {
+ t.Errorf("%v got panic %q, wanted %q", panicFrom, panicGot, msg)
+ }
+ }
+ }()
+ f()
+}
Change information
Files:
- A src/crypto/cipher/cbc_test.go
- A src/crypto/internal/cryptotest/blockmode.go
Change size: M
Delta: 2 files changed, 232 insertions(+), 0 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manuel Sabin (Gerrit)
Jun 27, 2024, 9:29:10 AM (6 days ago) Jun 27
to goph...@pubsubhelper.golang.org, Russell Webb, Esra Al, Clide Stefani, Garrett, Roland Shoemaker, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Filippo Valsorda, Roland Shoemaker and Russell Webb
Manuel Sabin added 5 comments![]( "Open in Gerrit")
File src/crypto/internal/cryptotest/blockmode.go
Line 2, Patchset 1 (Latest):
Manuel Sabin . unresolved
Much of this code is modified from Filippo's older CL here, so this code will have some comments throughout with my own questions on the code
Line 42, Patchset 1 (Latest):
t.Run("DES", func(t *testing.T) {Manuel Sabin . unresolved
Should this include a test for TripleDES being used in BlockMode as well?
Line 68, Patchset 1 (Latest):
t.Skip("mismatching encryption and decryption blocksizes")Manuel Sabin . unresolved
Why is this a skip?
Line 97, Patchset 1 (Latest):
// CryptBlocks on the same src data should yield same output even if dst=srcManuel Sabin . unresolved
This check is direcly related to Aliasing. What do the other two expectEqual checks in this loop, while important, have to do with aliasing?
Line 137, Patchset 1 (Latest):
expectEqual(t, dst[bs*2:], src[bs*2:], "CryptBlocks did out of bounds write after end of dst slice")Manuel Sabin . unresolved
How is this different than the test in Aliasing that errors with message "CryptBlocks modified dst past len(src)"? Seems to be addressing a specific Issue # (22104), but what's the difference? Is it that this checks for writing outside of the slice's length itself?
Related details
Attention is currently required from:
- Filippo Valsorda
- Roland Shoemaker
- Russell Webb
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manuel Sabin (Gerrit)
Jun 27, 2024, 10:02:07 AM (6 days ago) Jun 27
to goph...@pubsubhelper.golang.org, Roland Shoemaker, Russell Webb, Esra Al, Clide Stefani, Garrett, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Filippo Valsorda and Russell Webb
Manuel Sabin removed Roland Shoemaker from this change![]( "Open in Gerrit")
Attention is currently required from:
- Filippo Valsorda
- Russell Webb
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manuel Sabin (Gerrit)
Jun 28, 2024, 11:07:52 AM (5 days ago) Jun 28
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Filippo Valsorda and Russell Webb
Manuel Sabin uploaded new patchset![]( "Open in Gerrit")
Manuel Sabin uploaded patch set #2 to this change.
Related details
Attention is currently required from:
- Filippo Valsorda
- Russell Webb
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manuel Sabin (Gerrit)
5:35 PM (4 hours ago) 5:35 PM
to goph...@pubsubhelper.golang.org, Russell Webb, Esra Al, Clide Stefani, Garrett, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Filippo Valsorda and Russell Webb
Manuel Sabin added 1 comment![]( "Open in Gerrit")
File src/crypto/internal/cryptotest/blockmode.go
Line 97, Patchset 1:
// CryptBlocks on the same src data should yield same output even if dst=srcManuel Sabin . resolved
This check is direcly related to Aliasing. What do the other two expectEqual checks in this loop, while important, have to do with aliasing?
Manuel Sabin
Done
Related details
Attention is currently required from:
- Filippo Valsorda
- Russell Webb
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Manuel Sabin (Gerrit)
5:35 PM (4 hours ago) 5:35 PM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Filippo Valsorda and Russell Webb
Manuel Sabin uploaded new patchset![]( "Open in Gerrit")
Manuel Sabin uploaded patch set #3 to this change.
Related details
Attention is currently required from:
- Filippo Valsorda
- Russell Webb
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages