From b3d5ee068cec1396ba97116fe39a2012e6caafd5 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 27 Sep 2024 15:30:00 +0200 Subject: [PATCH 1/6] aes: encrypt and decrypt one block at a time --- cng/aes.go | 21 +++++++++++++++------ cng/aes_test.go | 12 ++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/cng/aes.go b/cng/aes.go index 7fda49a..654cb71 100644 --- a/cng/aes.go +++ b/cng/aes.go @@ -41,15 +41,20 @@ func (c *aesCipher) finalize() { func (c *aesCipher) BlockSize() int { return aesBlockSize } func (c *aesCipher) Encrypt(dst, src []byte) { - if subtle.InexactOverlap(dst, src) { - panic("crypto/cipher: invalid buffer overlap") - } if len(src) < aesBlockSize { panic("crypto/aes: input not full block") } if len(dst) < aesBlockSize { panic("crypto/aes: output not full block") } + + // cypher.Block.Encrypt() is documented to encrypt one full block + // at a time, so we truncate the input and output to the block size. + dst, src = dst[:aesBlockSize], src[:aesBlockSize] + if subtle.InexactOverlap(dst, src) { + panic("crypto/cipher: invalid buffer overlap") + } + var ret uint32 err := bcrypt.Encrypt(c.kh, src, nil, nil, dst, &ret, 0) if err != nil { @@ -62,9 +67,6 @@ func (c *aesCipher) Encrypt(dst, src []byte) { } func (c *aesCipher) Decrypt(dst, src []byte) { - if subtle.InexactOverlap(dst, src) { - panic("crypto/cipher: invalid buffer overlap") - } if len(src) < aesBlockSize { panic("crypto/aes: input not full block") } @@ -72,6 +74,13 @@ func (c *aesCipher) Decrypt(dst, src []byte) { panic("crypto/aes: output not full block") } + // cypher.Block.Decrypt() is documented to decrypt one full block + // at a time, so we truncate the input and output to the block size. + dst, src = dst[:aesBlockSize], src[:aesBlockSize] + if subtle.InexactOverlap(dst, src) { + panic("crypto/cipher: invalid buffer overlap") + } + var ret uint32 err := bcrypt.Decrypt(c.kh, src, nil, nil, dst, &ret, 0) if err != nil { diff --git a/cng/aes_test.go b/cng/aes_test.go index c5c9039..379c628 100644 --- a/cng/aes_test.go +++ b/cng/aes_test.go @@ -9,7 +9,10 @@ package cng import ( "bytes" "crypto/cipher" + "fmt" "testing" + + "github.com/microsoft/go-crypto-winnative/internal/cryptotest" ) var key = []byte("D249BF6DEC97B1EBD69BC4D6B3A3C49D") @@ -375,3 +378,12 @@ func TestCBCDecryptSimple(t *testing.T) { t.Errorf("decryption incorrect\nexp %v, got %v\n", plainText, decrypted) } } + +// Test AES against the general cipher.Block interface tester. +func TestAESBlock(t *testing.T) { + for _, keylen := range []int{128, 192, 256} { + t.Run(fmt.Sprintf("AES-%d", keylen), func(t *testing.T) { + cryptotest.TestBlock(t, keylen/8, NewAESCipher) + }) + } +} From 06efdad6c09b751f46dc0619455db7a9af2d0ebe Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 27 Sep 2024 15:51:48 +0200 Subject: [PATCH 2/6] test AES block mode --- cng/aes_test.go | 27 ++++ internal/cryptotest/blockmode.go | 224 +++++++++++++++++++++++++++++++ 2 files changed, 251 insertions(+) create mode 100644 internal/cryptotest/blockmode.go diff --git a/cng/aes_test.go b/cng/aes_test.go index 379c628..183e9fd 100644 --- a/cng/aes_test.go +++ b/cng/aes_test.go @@ -10,7 +10,10 @@ import ( "bytes" "crypto/cipher" "fmt" + "io" + "math/rand" "testing" + "time" "github.com/microsoft/go-crypto-winnative/internal/cryptotest" ) @@ -387,3 +390,27 @@ func TestAESBlock(t *testing.T) { }) } } + +func TestAESBlockMode(t *testing.T) { + for _, keylen := range []int{128, 192, 256} { + t.Run(fmt.Sprintf("AES-%d", keylen), func(t *testing.T) { + rng := newRandReader(t) + + key := make([]byte, keylen/8) + rng.Read(key) + + block, err := NewAESCipher(key) + if err != nil { + panic(err) + } + + cryptotest.TestBlockMode(t, block, cipher.NewCBCEncrypter, cipher.NewCBCDecrypter) + }) + } +} + +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)) +} diff --git a/internal/cryptotest/blockmode.go b/internal/cryptotest/blockmode.go new file mode 100644 index 0000000..d3271e5 --- /dev/null +++ b/internal/cryptotest/blockmode.go @@ -0,0 +1,224 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cryptotest + +import ( + "bytes" + "crypto/cipher" + "testing" +) + +// 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, block cipher.Block, makeEncrypter, makeDecrypter MakeBlockMode) { + rng := newRandReader(t) + iv := make([]byte, block.BlockSize()) + rng.Read(iv) + + testBlockModePair(t, block, makeEncrypter, makeDecrypter, iv) +} + +func testBlockModePair(t *testing.T, b cipher.Block, enc, dec MakeBlockMode, iv []byte) { + t.Run("Encryption", func(t *testing.T) { + testBlockMode(t, enc, b, iv) + }) + + t.Run("Decryption", func(t *testing.T) { + testBlockMode(t, dec, b, iv) + }) + + t.Run("Roundtrip", func(t *testing.T) { + rng := newRandReader(t) + + blockSize := enc(b, iv).BlockSize() + if decBlockSize := dec(b, iv).BlockSize(); decBlockSize != blockSize { + t.Errorf("decryption blocksize different than encryption's; got %d, want %d", decBlockSize, blockSize) + } + + before, dst, after := make([]byte, blockSize*2), make([]byte, blockSize*2), make([]byte, blockSize*2) + rng.Read(before) + + enc(b, iv).CryptBlocks(dst, before) + dec(b, iv).CryptBlocks(after, dst) + if !bytes.Equal(after, before) { + t.Errorf("plaintext is different after an encrypt/decrypt cycle; got %x, want %x", after, before) + } + }) +} + +func testBlockMode(t *testing.T, bm MakeBlockMode, b cipher.Block, iv []byte) { + blockSize := 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("AlterInput", func(t *testing.T) { + rng := newRandReader(t) + + src, dst, before := make([]byte, blockSize*2), make([]byte, blockSize*2), make([]byte, blockSize*2) + + for _, length := range []int{0, blockSize, blockSize * 2} { + rng.Read(src) + copy(before, src) + + bm(b, iv).CryptBlocks(dst[:length], src[:length]) + if !bytes.Equal(src, before) { + t.Errorf("CryptBlocks modified src; got %x, want %x", src, before) + } + } + }) + + t.Run("Aliasing", func(t *testing.T) { + rng := newRandReader(t) + + buff, expectedOutput := make([]byte, blockSize*2), make([]byte, blockSize*2) + + for _, length := range []int{0, blockSize, blockSize * 2} { + // Record what output is when src and dst are different + rng.Read(buff) + bm(b, iv).CryptBlocks(expectedOutput[:length], buff[:length]) + + // Check that the same output is generated when src=dst alias to the same + // memory + bm(b, iv).CryptBlocks(buff[:length], buff[:length]) + if !bytes.Equal(buff[:length], expectedOutput[:length]) { + t.Errorf("block cipher produced different output when dst = src; got %x, want %x", buff[:length], expectedOutput[:length]) + } + } + }) + + t.Run("OutOfBoundsWrite", func(t *testing.T) { // Issue 21104 + rng := newRandReader(t) + + src := make([]byte, blockSize) + rng.Read(src) + + // Make a buffer with dst in the middle and data on either end + buff := make([]byte, blockSize*3) + endOfPrefix, startOfSuffix := blockSize, blockSize*2 + rng.Read(buff[:endOfPrefix]) + rng.Read(buff[startOfSuffix:]) + dst := buff[endOfPrefix:startOfSuffix] + + // Record the prefix and suffix data to make sure they aren't written to + initPrefix, initSuffix := make([]byte, blockSize), make([]byte, blockSize) + copy(initPrefix, buff[:endOfPrefix]) + copy(initSuffix, buff[startOfSuffix:]) + + // Write to dst (the middle of the buffer) and make sure it doesn't write + // beyond the dst slice on a valid CryptBlocks call + bm(b, iv).CryptBlocks(dst, src) + if !bytes.Equal(buff[startOfSuffix:], initSuffix) { + t.Errorf("block cipher did out of bounds write after end of dst slice; got %x, want %x", buff[startOfSuffix:], initSuffix) + } + if !bytes.Equal(buff[:endOfPrefix], initPrefix) { + t.Errorf("block cipher did out of bounds write before beginning of dst slice; got %x, want %x", buff[:endOfPrefix], initPrefix) + } + + // Check that dst isn't written to beyond len(src) even if there is room in + // the slice + dst = buff[endOfPrefix:] // Extend dst to include suffix + bm(b, iv).CryptBlocks(dst, src) + if !bytes.Equal(buff[startOfSuffix:], initSuffix) { + t.Errorf("CryptBlocks modified dst past len(src); got %x, want %x", buff[startOfSuffix:], initSuffix) + } + + // Issue 21104: Shouldn't write to anything outside of dst even if src is bigger + src = make([]byte, blockSize*3) + rng.Read(src) + + mustPanic(t, "output smaller than input", func() { + bm(b, iv).CryptBlocks(dst, src) + }) + + if !bytes.Equal(buff[startOfSuffix:], initSuffix) { + t.Errorf("block cipher did out of bounds write after end of dst slice; got %x, want %x", buff[startOfSuffix:], initSuffix) + } + if !bytes.Equal(buff[:endOfPrefix], initPrefix) { + t.Errorf("block cipher did out of bounds write before beginning of dst slice; got %x, want %x", buff[:endOfPrefix], initPrefix) + } + }) + + // Check that output of cipher isn't affected by adjacent data beyond input + // slice scope + t.Run("OutOfBoundsRead", func(t *testing.T) { + rng := newRandReader(t) + + src := make([]byte, blockSize) + rng.Read(src) + expectedDst := make([]byte, blockSize) + bm(b, iv).CryptBlocks(expectedDst, src) + + // Make a buffer with src in the middle and data on either end + buff := make([]byte, blockSize*3) + endOfPrefix, startOfSuffix := blockSize, blockSize*2 + + copy(buff[endOfPrefix:startOfSuffix], src) + rng.Read(buff[:endOfPrefix]) + rng.Read(buff[startOfSuffix:]) + + testDst := make([]byte, blockSize) + bm(b, iv).CryptBlocks(testDst, buff[endOfPrefix:startOfSuffix]) + + if !bytes.Equal(testDst, expectedDst) { + t.Errorf("CryptBlocks affected by data outside of src slice bounds; got %x, want %x", testDst, expectedDst) + } + }) + + t.Run("BufferOverlap", func(t *testing.T) { + rng := newRandReader(t) + + buff := make([]byte, blockSize*2) + rng.Read(buff) + + // Make src and dst slices point to same array with inexact overlap + src := buff[:blockSize] + dst := buff[1 : blockSize+1] + mustPanic(t, "invalid buffer overlap", func() { bm(b, iv).CryptBlocks(dst, src) }) + + // Only overlap on one byte + src = buff[:blockSize] + dst = buff[blockSize-1 : 2*blockSize-1] + mustPanic(t, "invalid buffer overlap", func() { bm(b, iv).CryptBlocks(dst, src) }) + + // src comes after dst with one byte overlap + src = buff[blockSize-1 : 2*blockSize-1] + dst = buff[:blockSize] + 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{blockSize - 1, blockSize + 1, 2*blockSize - 1, 2*blockSize + 1} { + src := make([]byte, srcSize) + dst := make([]byte, 3*blockSize) // Make a dst large enough for all src + mustPanic(t, "input not full blocks", func() { bm(b, iv).CryptBlocks(dst, src) }) + } + }) + + t.Run("KeepState", func(t *testing.T) { + rng := newRandReader(t) + + src, serialDst, compositeDst := make([]byte, blockSize*4), make([]byte, blockSize*4), make([]byte, blockSize*4) + rng.Read(src) + + length, block := 2*blockSize, bm(b, iv) + block.CryptBlocks(serialDst, src[:length]) + block.CryptBlocks(serialDst[length:], src[length:]) + + bm(b, iv).CryptBlocks(compositeDst, src) + + if !bytes.Equal(serialDst, compositeDst) { + t.Errorf("two successive CryptBlocks calls returned a different result than a single one; got %x, want %x", serialDst, compositeDst) + } + }) +} From c52d8885135a9cd77384ed9ff73d0448cf552786 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 27 Sep 2024 15:55:28 +0200 Subject: [PATCH 3/6] test AEAD --- cng/aes_test.go | 37 ++++ internal/cryptotest/aead.go | 387 ++++++++++++++++++++++++++++++++++++ 2 files changed, 424 insertions(+) create mode 100644 internal/cryptotest/aead.go diff --git a/cng/aes_test.go b/cng/aes_test.go index 183e9fd..b96d077 100644 --- a/cng/aes_test.go +++ b/cng/aes_test.go @@ -409,6 +409,43 @@ func TestAESBlockMode(t *testing.T) { } } +// Test GCM against the general cipher.AEAD interface tester. +func TestAESGCMAEAD(t *testing.T) { + minTagSize := 12 + + for _, keySize := range []int{128, 192, 256} { + // Use AES as underlying block cipher at different key sizes for GCM. + t.Run(fmt.Sprintf("AES-%d", keySize), func(t *testing.T) { + rng := newRandReader(t) + + key := make([]byte, keySize/8) + rng.Read(key) + + block, err := NewAESCipher(key) + if err != nil { + panic(err) + } + + // Test GCM with the current AES block with the standard nonce and tag + // sizes. + cryptotest.TestAEAD(t, func() (cipher.AEAD, error) { return cipher.NewGCM(block) }) + + // Test non-standard tag sizes. + t.Run("MinTagSize", func(t *testing.T) { + cryptotest.TestAEAD(t, func() (cipher.AEAD, error) { return cipher.NewGCMWithTagSize(block, minTagSize) }) + }) + + // Test non-standard nonce sizes. + for _, nonceSize := range []int{1, 16, 100} { + t.Run(fmt.Sprintf("NonceSize-%d", nonceSize), func(t *testing.T) { + + cryptotest.TestAEAD(t, func() (cipher.AEAD, error) { return cipher.NewGCMWithNonceSize(block, nonceSize) }) + }) + } + }) + } +} + func newRandReader(t *testing.T) io.Reader { seed := time.Now().UnixNano() t.Logf("Deterministic RNG seed: 0x%x", seed) diff --git a/internal/cryptotest/aead.go b/internal/cryptotest/aead.go new file mode 100644 index 0000000..a6107e5 --- /dev/null +++ b/internal/cryptotest/aead.go @@ -0,0 +1,387 @@ +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cryptotest + +import ( + "bytes" + "crypto/cipher" + "fmt" + "testing" +) + +var lengths = []int{0, 156, 8192, 8193, 8208} + +// MakeAEAD returns a cipher.AEAD instance. +// +// Multiple calls to MakeAEAD must return equivalent instances, so for example +// the key must be fixed. +type MakeAEAD func() (cipher.AEAD, error) + +// TestAEAD performs a set of tests on cipher.AEAD implementations, checking +// the documented requirements of NonceSize, Overhead, Seal and Open. +func TestAEAD(t *testing.T, mAEAD MakeAEAD) { + aead, err := mAEAD() + if err != nil { + t.Fatal(err) + } + + t.Run("Roundtrip", func(t *testing.T) { + + // Test all combinations of plaintext and additional data lengths. + for _, ptLen := range lengths { + for _, adLen := range lengths { + t.Run(fmt.Sprintf("Plaintext-Length=%d,AddData-Length=%d", ptLen, adLen), func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + before, addData := make([]byte, adLen), make([]byte, ptLen) + rng.Read(before) + rng.Read(addData) + + ciphertext := sealMsg(t, aead, nil, nonce, before, addData) + after := openWithoutError(t, aead, nil, nonce, ciphertext, addData) + + if !bytes.Equal(after, before) { + t.Errorf("plaintext is different after a seal/open cycle; got %s, want %s", truncateHex(after), truncateHex(before)) + } + }) + } + } + }) + + t.Run("InputNotModified", func(t *testing.T) { + + // Test all combinations of plaintext and additional data lengths. + for _, ptLen := range lengths { + for _, adLen := range lengths { + t.Run(fmt.Sprintf("Plaintext-Length=%d,AddData-Length=%d", ptLen, adLen), func(t *testing.T) { + t.Run("Seal", func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + src, before := make([]byte, ptLen), make([]byte, ptLen) + rng.Read(src) + copy(before, src) + + addData := make([]byte, adLen) + rng.Read(addData) + + sealMsg(t, aead, nil, nonce, src, addData) + if !bytes.Equal(src, before) { + t.Errorf("Seal modified src; got %s, want %s", truncateHex(src), truncateHex(before)) + } + }) + + t.Run("Open", func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + plaintext, addData := make([]byte, ptLen), make([]byte, adLen) + rng.Read(plaintext) + rng.Read(addData) + + // Record the ciphertext that shouldn't be modified as the input of + // Open. + ciphertext := sealMsg(t, aead, nil, nonce, plaintext, addData) + before := make([]byte, len(ciphertext)) + copy(before, ciphertext) + + openWithoutError(t, aead, nil, nonce, ciphertext, addData) + if !bytes.Equal(ciphertext, before) { + t.Errorf("Open modified src; got %s, want %s", truncateHex(ciphertext), truncateHex(before)) + } + }) + }) + } + } + }) + + t.Run("BufferOverlap", func(t *testing.T) { + + // Test all combinations of plaintext and additional data lengths. + for _, ptLen := range lengths { + if ptLen <= 1 { // We need enough room for an overlap to occur. + continue + } + for _, adLen := range lengths { + t.Run(fmt.Sprintf("Plaintext-Length=%d,AddData-Length=%d", ptLen, adLen), func(t *testing.T) { + t.Run("Seal", func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + // Make a buffer that can hold a plaintext and ciphertext as we + // overlap their slices to check for panic on inexact overlaps. + ctLen := ptLen + aead.Overhead() + buff := make([]byte, ptLen+ctLen) + rng.Read(buff) + + addData := make([]byte, adLen) + rng.Read(addData) + + // Make plaintext and dst slices point to same array with inexact overlap. + plaintext := buff[:ptLen] + dst := buff[1:1] // Shift dst to not start at start of plaintext. + mustPanic(t, "invalid buffer overlap", func() { sealMsg(t, aead, dst, nonce, plaintext, addData) }) + + // Only overlap on one byte + plaintext = buff[:ptLen] + dst = buff[ptLen-1 : ptLen-1] + mustPanic(t, "invalid buffer overlap", func() { sealMsg(t, aead, dst, nonce, plaintext, addData) }) + }) + + t.Run("Open", func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + // Create a valid ciphertext to test Open with. + plaintext := make([]byte, ptLen) + rng.Read(plaintext) + addData := make([]byte, adLen) + rng.Read(addData) + validCT := sealMsg(t, aead, nil, nonce, plaintext, addData) + + // Make a buffer that can hold a plaintext and ciphertext as we + // overlap their slices to check for panic on inexact overlaps. + buff := make([]byte, ptLen+len(validCT)) + + // Make ciphertext and dst slices point to same array with inexact overlap. + ciphertext := buff[:len(validCT)] + copy(ciphertext, validCT) + dst := buff[1:1] // Shift dst to not start at start of ciphertext. + mustPanic(t, "invalid buffer overlap", func() { aead.Open(dst, nonce, ciphertext, addData) }) + + // Only overlap on one byte. + ciphertext = buff[:len(validCT)] + copy(ciphertext, validCT) + // Make sure it is the actual ciphertext being overlapped and not + // the hash digest which might be extracted/truncated in some + // implementations: Go one byte past the hash digest/tag and into + // the ciphertext. + beforeTag := len(validCT) - aead.Overhead() + dst = buff[beforeTag-1 : beforeTag-1] + mustPanic(t, "invalid buffer overlap", func() { aead.Open(dst, nonce, ciphertext, addData) }) + }) + }) + } + } + }) + + t.Run("AppendDst", func(t *testing.T) { + + // Test all combinations of plaintext and additional data lengths. + for _, ptLen := range lengths { + for _, adLen := range lengths { + t.Run(fmt.Sprintf("Plaintext-Length=%d,AddData-Length=%d", ptLen, adLen), func(t *testing.T) { + + t.Run("Seal", func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + shortBuff := []byte("a") + longBuff := make([]byte, 512) + rng.Read(longBuff) + prefixes := [][]byte{shortBuff, longBuff} + + // Check each prefix gets appended to by Seal with altering them. + for _, prefix := range prefixes { + plaintext, addData := make([]byte, ptLen), make([]byte, adLen) + rng.Read(plaintext) + rng.Read(addData) + out := sealMsg(t, aead, prefix, nonce, plaintext, addData) + + // Check that Seal didn't alter the prefix + if !bytes.Equal(out[:len(prefix)], prefix) { + t.Errorf("Seal alters dst instead of appending; got %s, want %s", truncateHex(out[:len(prefix)]), truncateHex(prefix)) + } + + ciphertext := out[len(prefix):] + // Check that the appended ciphertext wasn't affected by the prefix + if expectedCT := sealMsg(t, aead, nil, nonce, plaintext, addData); !bytes.Equal(ciphertext, expectedCT) { + t.Errorf("Seal behavior affected by pre-existing data in dst; got %s, want %s", truncateHex(ciphertext), truncateHex(expectedCT)) + } + } + }) + + t.Run("Open", func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + shortBuff := []byte("a") + longBuff := make([]byte, 512) + rng.Read(longBuff) + prefixes := [][]byte{shortBuff, longBuff} + + // Check each prefix gets appended to by Open with altering them. + for _, prefix := range prefixes { + before, addData := make([]byte, adLen), make([]byte, ptLen) + rng.Read(before) + rng.Read(addData) + ciphertext := sealMsg(t, aead, nil, nonce, before, addData) + + out := openWithoutError(t, aead, prefix, nonce, ciphertext, addData) + + // Check that Open didn't alter the prefix + if !bytes.Equal(out[:len(prefix)], prefix) { + t.Errorf("Open alters dst instead of appending; got %s, want %s", truncateHex(out[:len(prefix)]), truncateHex(prefix)) + } + + after := out[len(prefix):] + // Check that the appended plaintext wasn't affected by the prefix + if !bytes.Equal(after, before) { + t.Errorf("Open behavior affected by pre-existing data in dst; got %s, want %s", truncateHex(after), truncateHex(before)) + } + } + }) + }) + } + } + }) + + t.Run("WrongNonce", func(t *testing.T) { + + // Test all combinations of plaintext and additional data lengths. + for _, ptLen := range lengths { + for _, adLen := range lengths { + t.Run(fmt.Sprintf("Plaintext-Length=%d,AddData-Length=%d", ptLen, adLen), func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + plaintext, addData := make([]byte, ptLen), make([]byte, adLen) + rng.Read(plaintext) + rng.Read(addData) + + ciphertext := sealMsg(t, aead, nil, nonce, plaintext, addData) + + // Perturb the nonce and check for an error when Opening + alterNonce := make([]byte, aead.NonceSize()) + copy(alterNonce, nonce) + alterNonce[len(alterNonce)-1] += 1 + _, err := aead.Open(nil, alterNonce, ciphertext, addData) + + if err == nil { + t.Errorf("Open did not error when given different nonce than Sealed with") + } + }) + } + } + }) + + t.Run("WrongAddData", func(t *testing.T) { + + // Test all combinations of plaintext and additional data lengths. + for _, ptLen := range lengths { + for _, adLen := range lengths { + if adLen == 0 { + continue + } + + t.Run(fmt.Sprintf("Plaintext-Length=%d,AddData-Length=%d", ptLen, adLen), func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + plaintext, addData := make([]byte, ptLen), make([]byte, adLen) + rng.Read(plaintext) + rng.Read(addData) + + ciphertext := sealMsg(t, aead, nil, nonce, plaintext, addData) + + // Perturb the Additional Data and check for an error when Opening + alterAD := make([]byte, adLen) + copy(alterAD, addData) + alterAD[len(alterAD)-1] += 1 + _, err := aead.Open(nil, nonce, ciphertext, alterAD) + + if err == nil { + t.Errorf("Open did not error when given different Additional Data than Sealed with") + } + }) + } + } + }) + + t.Run("WrongCiphertext", func(t *testing.T) { + + // Test all combinations of plaintext and additional data lengths. + for _, ptLen := range lengths { + for _, adLen := range lengths { + + t.Run(fmt.Sprintf("Plaintext-Length=%d,AddData-Length=%d", ptLen, adLen), func(t *testing.T) { + rng := newRandReader(t) + + nonce := make([]byte, aead.NonceSize()) + rng.Read(nonce) + + plaintext, addData := make([]byte, ptLen), make([]byte, adLen) + rng.Read(plaintext) + rng.Read(addData) + + ciphertext := sealMsg(t, aead, nil, nonce, plaintext, addData) + + // Perturb the ciphertext and check for an error when Opening + alterCT := make([]byte, len(ciphertext)) + copy(alterCT, ciphertext) + alterCT[len(alterCT)-1] += 1 + _, err := aead.Open(nil, nonce, alterCT, addData) + + if err == nil { + t.Errorf("Open did not error when given different ciphertext than was produced by Seal") + } + }) + } + } + }) +} + +// Helper function to Seal a plaintext with additional data. Checks that +// ciphertext isn't bigger than the plaintext length plus Overhead() +func sealMsg(t *testing.T, aead cipher.AEAD, ciphertext, nonce, plaintext, addData []byte) []byte { + t.Helper() + + initialLen := len(ciphertext) + + ciphertext = aead.Seal(ciphertext, nonce, plaintext, addData) + + lenCT := len(ciphertext) - initialLen + + // Appended ciphertext shouldn't ever be longer than the length of the + // plaintext plus Overhead + if lenCT > len(plaintext)+aead.Overhead() { + t.Errorf("length of ciphertext from Seal exceeds length of plaintext by more than Overhead(); got %d, want <=%d", lenCT, len(plaintext)+aead.Overhead()) + } + + return ciphertext +} + +// Helper function to Open and authenticate ciphertext. Checks that Open +// doesn't error (assuming ciphertext was well-formed with corresponding nonce +// and additional data). +func openWithoutError(t *testing.T, aead cipher.AEAD, plaintext, nonce, ciphertext, addData []byte) []byte { + t.Helper() + + plaintext, err := aead.Open(plaintext, nonce, ciphertext, addData) + if err != nil { + t.Fatalf("Open returned error on properly formed ciphertext; got \"%s\", want \"nil\"", err) + } + + return plaintext +} From ace0e9bc19f3b54bbca05316ccc63edbea55c771 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 27 Sep 2024 21:40:44 +0200 Subject: [PATCH 4/6] use t.Fatal instead of panic --- cng/aes_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cng/aes_test.go b/cng/aes_test.go index b96d077..afbac19 100644 --- a/cng/aes_test.go +++ b/cng/aes_test.go @@ -401,7 +401,7 @@ func TestAESBlockMode(t *testing.T) { block, err := NewAESCipher(key) if err != nil { - panic(err) + t.Fatal(err) } cryptotest.TestBlockMode(t, block, cipher.NewCBCEncrypter, cipher.NewCBCDecrypter) From b47dc27ec40430e2ac2ce0a9efe0e51fe31a4de7 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Fri, 27 Sep 2024 21:53:40 +0200 Subject: [PATCH 5/6] use black box testing in all test files --- cng/aes_test.go | 96 +++++++++++++++++++----------------------------- cng/cng_test.go | 9 +++++ cng/hmac_test.go | 30 ++++++++------- cng/rand_test.go | 8 ++-- 4 files changed, 67 insertions(+), 76 deletions(-) diff --git a/cng/aes_test.go b/cng/aes_test.go index afbac19..381f2aa 100644 --- a/cng/aes_test.go +++ b/cng/aes_test.go @@ -4,28 +4,34 @@ //go:build windows // +build windows -package cng +package cng_test import ( "bytes" "crypto/cipher" "fmt" - "io" - "math/rand" + "strings" "testing" - "time" + "github.com/microsoft/go-crypto-winnative/cng" "github.com/microsoft/go-crypto-winnative/internal/cryptotest" ) var key = []byte("D249BF6DEC97B1EBD69BC4D6B3A3C49D") +const ( + gcmTagSize = 16 + gcmStandardNonceSize = 12 +) + func TestNewGCMNonce(t *testing.T) { - ci, err := NewAESCipher(key) + ci, err := cng.NewAESCipher(key) if err != nil { t.Fatal(err) } - c := ci.(*aesCipher) + c := ci.(interface { + NewGCM(nonceSize, tagSize int) (cipher.AEAD, error) + }) _, err = c.NewGCM(gcmStandardNonceSize-1, gcmTagSize-1) if err == nil { t.Error("expected error for non-standard tag and nonce size at the same time, got none") @@ -45,12 +51,11 @@ func TestNewGCMNonce(t *testing.T) { } func TestSealAndOpen(t *testing.T) { - ci, err := NewAESCipher(key) + ci, err := cng.NewAESCipher(key) if err != nil { t.Fatal(err) } - c := ci.(*aesCipher) - gcm, err := c.NewGCM(gcmStandardNonceSize, gcmTagSize) + gcm, err := cipher.NewGCMWithTagSize(ci, gcmTagSize) if err != nil { t.Fatal(err) } @@ -95,16 +100,16 @@ func TestSealAndOpenTLS(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ci, err := NewAESCipher(key) + ci, err := cng.NewAESCipher(key) if err != nil { t.Fatal(err) } var gcm cipher.AEAD switch tt.tls { case "1.2": - gcm, err = NewGCMTLS(ci) + gcm, err = cng.NewGCMTLS(ci) case "1.3": - gcm, err = NewGCMTLS13(ci) + gcm, err = cng.NewGCMTLS13(ci) } if err != nil { t.Fatal(err) @@ -163,12 +168,11 @@ func TestSealAndOpenTLS(t *testing.T) { } func TestSealAndOpenAuthenticationError(t *testing.T) { - ci, err := NewAESCipher(key) + ci, err := cng.NewAESCipher(key) if err != nil { t.Fatal(err) } - c := ci.(*aesCipher) - gcm, err := c.NewGCM(gcmStandardNonceSize, gcmTagSize) + gcm, err := cipher.NewGCMWithTagSize(ci, gcmTagSize) if err != nil { t.Fatal(err) } @@ -177,7 +181,7 @@ func TestSealAndOpenAuthenticationError(t *testing.T) { additionalData := []byte{0x05, 0x05, 0x07} sealed := gcm.Seal(nil, nonce, plainText, additionalData) _, err = gcm.Open(nil, nonce, sealed, nil) - if err != errOpen { + if !strings.Contains(err.Error(), "cipher: message authentication failed") { t.Errorf("expected authentication error, got: %#v", err) } } @@ -193,12 +197,11 @@ func assertPanic(t *testing.T, f func()) { } func TestSealPanic(t *testing.T) { - ci, err := NewAESCipher(key) + ci, err := cng.NewAESCipher(key) if err != nil { t.Fatal(err) } - c := ci.(*aesCipher) - gcm, err := c.NewGCM(gcmStandardNonceSize, gcmTagSize) + gcm, err := cipher.NewGCMWithTagSize(ci, gcmTagSize) if err != nil { t.Fatal(err) } @@ -215,14 +218,14 @@ func TestSealPanic(t *testing.T) { } func TestAESInvalidKeySize(t *testing.T) { - _, err := NewAESCipher([]byte{1}) + _, err := cng.NewAESCipher([]byte{1}) if err == nil { t.Error("error expected") } } func TestEncryptAndDecrypt(t *testing.T) { - ci, err := NewAESCipher(key) + ci, err := cng.NewAESCipher(key) if err != nil { t.Fatal(err) } @@ -242,7 +245,7 @@ func TestCBCBlobEncryptBasicBlockEncryption(t *testing.T) { key := []byte{0x24, 0xcd, 0x8b, 0x13, 0x37, 0xc5, 0xc1, 0xb1, 0x0, 0xbb, 0x27, 0x40, 0x4f, 0xab, 0x5f, 0x7b, 0x2d, 0x0, 0x20, 0xf5, 0x1, 0x84, 0x4, 0xbf, 0xe3, 0xbd, 0xa1, 0xc4, 0xbf, 0x61, 0x2f, 0xc5} iv := []byte{0x91, 0xc7, 0xa7, 0x54, 0x52, 0xef, 0x10, 0xdb, 0x91, 0xa8, 0x6c, 0xf9, 0x79, 0xd5, 0xac, 0x74} - block, err := NewAESCipher(key) + block, err := cng.NewAESCipher(key) if err != nil { t.Errorf("expected no error for aes.NewCipher, got: %s", err) } @@ -251,19 +254,14 @@ func TestCBCBlobEncryptBasicBlockEncryption(t *testing.T) { if blockSize != 16 { t.Errorf("unexpected block size, expected 16 got: %d", blockSize) } - var encryptor cipher.BlockMode - if c, ok := block.(*aesCipher); ok { - encryptor = c.NewCBCEncrypter(iv) - if encryptor == nil { - t.Error("unable to create new CBC encrypter") - } - } + + encrypter := cipher.NewCBCEncrypter(block, iv) encrypted := make([]byte, 32) // First block. 16 bytes. srcBlock1 := bytes.Repeat([]byte{0x01}, 16) - encryptor.CryptBlocks(encrypted, srcBlock1) + encrypter.CryptBlocks(encrypted, srcBlock1) if !bytes.Equal([]byte{ 0x14, 0xb7, 0x3e, 0x2f, 0xd9, 0xe7, 0x69, 0x7e, 0xb7, 0xd2, 0xc3, 0x5b, 0x31, 0x9c, 0xf0, 0x59, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -273,7 +271,7 @@ func TestCBCBlobEncryptBasicBlockEncryption(t *testing.T) { // Second block. 16 bytes. srcBlock2 := bytes.Repeat([]byte{0x02}, 16) - encryptor.CryptBlocks(encrypted[16:], srcBlock2) + encrypter.CryptBlocks(encrypted[16:], srcBlock2) if !bytes.Equal([]byte{ 0x14, 0xb7, 0x3e, 0x2f, 0xd9, 0xe7, 0x69, 0x7e, 0xb7, 0xd2, 0xc3, 0x5b, 0x31, 0x9c, 0xf0, 0x59, 0xbb, 0xd4, 0x95, 0x25, 0x21, 0x56, 0x87, 0x3b, 0xe6, 0x22, 0xe8, 0xd0, 0x19, 0xa8, 0xed, 0xcd, @@ -281,13 +279,7 @@ func TestCBCBlobEncryptBasicBlockEncryption(t *testing.T) { t.Error("unexpected CryptBlocks result for second block") } - var decrypter cipher.BlockMode - if c, ok := block.(*aesCipher); ok { - decrypter = c.NewCBCDecrypter(iv) - if decrypter == nil { - t.Error("unable to create new CBC decrypter") - } - } + decrypter := cipher.NewCBCDecrypter(block, iv) plainText := append(srcBlock1, srcBlock2...) decrypted := make([]byte, len(plainText)) decrypter.CryptBlocks(decrypted, encrypted[:16]) @@ -305,7 +297,7 @@ func TestCBCDecryptSimple(t *testing.T) { 0xe3, 0xbd, 0xa1, 0xc4, 0xbf, 0x61, 0x2f, 0xc5, } - block, err := NewAESCipher(key) + block, err := cng.NewAESCipher(key) if err != nil { t.Fatal(err) } @@ -314,17 +306,9 @@ func TestCBCDecryptSimple(t *testing.T) { 0x91, 0xc7, 0xa7, 0x54, 0x52, 0xef, 0x10, 0xdb, 0x91, 0xa8, 0x6c, 0xf9, 0x79, 0xd5, 0xac, 0x74, } - var encrypter, decrypter cipher.BlockMode - if c, ok := block.(*aesCipher); ok { - encrypter = c.NewCBCEncrypter(iv) - if encrypter == nil { - t.Error("unable to create new CBC encrypter") - } - decrypter = c.NewCBCDecrypter(iv) - if decrypter == nil { - t.Error("unable to create new CBC decrypter") - } - } + + encrypter := cipher.NewCBCEncrypter(block, iv) + decrypter := cipher.NewCBCDecrypter(block, iv) plainText := []byte{ 0x54, 0x68, 0x65, 0x72, 0x65, 0x20, 0x69, 0x73, @@ -386,7 +370,7 @@ func TestCBCDecryptSimple(t *testing.T) { func TestAESBlock(t *testing.T) { for _, keylen := range []int{128, 192, 256} { t.Run(fmt.Sprintf("AES-%d", keylen), func(t *testing.T) { - cryptotest.TestBlock(t, keylen/8, NewAESCipher) + cryptotest.TestBlock(t, keylen/8, cng.NewAESCipher) }) } } @@ -399,7 +383,7 @@ func TestAESBlockMode(t *testing.T) { key := make([]byte, keylen/8) rng.Read(key) - block, err := NewAESCipher(key) + block, err := cng.NewAESCipher(key) if err != nil { t.Fatal(err) } @@ -421,7 +405,7 @@ func TestAESGCMAEAD(t *testing.T) { key := make([]byte, keySize/8) rng.Read(key) - block, err := NewAESCipher(key) + block, err := cng.NewAESCipher(key) if err != nil { panic(err) } @@ -445,9 +429,3 @@ func TestAESGCMAEAD(t *testing.T) { }) } } - -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)) -} diff --git a/cng/cng_test.go b/cng/cng_test.go index bfe0960..c702e9d 100644 --- a/cng/cng_test.go +++ b/cng/cng_test.go @@ -8,9 +8,12 @@ package cng_test import ( "fmt" + "io" + "math/rand" "os" "strconv" "testing" + "time" "github.com/microsoft/go-crypto-winnative/cng" ) @@ -39,3 +42,9 @@ func TestFIPS(t *testing.T) { } } } + +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)) +} diff --git a/cng/hmac_test.go b/cng/hmac_test.go index 713727c..1758a35 100644 --- a/cng/hmac_test.go +++ b/cng/hmac_test.go @@ -4,13 +4,15 @@ //go:build windows // +build windows -package cng +package cng_test import ( "bytes" "fmt" "hash" "testing" + + "github.com/microsoft/go-crypto-winnative/cng" ) func TestHMAC_EmptyKey(t *testing.T) { @@ -20,14 +22,14 @@ func TestHMAC_EmptyKey(t *testing.T) { fn func() hash.Hash out string }{ - {"sha1", NewSHA1, "d5d1ed05121417247616cfc8378f360a39da7cfa"}, - {"sha256", NewSHA256, "eb08c1f56d5ddee07f7bdf80468083da06b64cf4fac64fe3a90883df5feacae4"}, - {"sha384", NewSHA384, "a1302a8028a419bb834bfae53c5e98ab48e07aed9ef8b980a821df28685902003746ade315072edd8ce009a1d23705ec"}, - {"sha512", NewSHA512, "08fce52f6395d59c2a3fb8abb281d74ad6f112b9a9c787bcea290d94dadbc82b2ca3e5e12bf2277c7fedbb0154d5493e41bb7459f63c8e39554ea3651b812492"}, + {"sha1", cng.NewSHA1, "d5d1ed05121417247616cfc8378f360a39da7cfa"}, + {"sha256", cng.NewSHA256, "eb08c1f56d5ddee07f7bdf80468083da06b64cf4fac64fe3a90883df5feacae4"}, + {"sha384", cng.NewSHA384, "a1302a8028a419bb834bfae53c5e98ab48e07aed9ef8b980a821df28685902003746ade315072edd8ce009a1d23705ec"}, + {"sha512", cng.NewSHA512, "08fce52f6395d59c2a3fb8abb281d74ad6f112b9a9c787bcea290d94dadbc82b2ca3e5e12bf2277c7fedbb0154d5493e41bb7459f63c8e39554ea3651b812492"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - h := NewHMAC(tt.fn, nil) + h := cng.NewHMAC(tt.fn, nil) h.Write([]byte(payload)) sum := fmt.Sprintf("%x", h.Sum(nil)) if sum != tt.out { @@ -44,26 +46,26 @@ func TestHMAC(t *testing.T) { fn func() hash.Hash key []byte }{ - {"sha1", NewSHA1, key}, - {"sha256", NewSHA256, key}, - {"sha256-big", NewSHA256, append(key, make([]byte, 1000)...)}, - {"sha384", NewSHA384, key}, - {"sha512", NewSHA512, key}, + {"sha1", cng.NewSHA1, key}, + {"sha256", cng.NewSHA256, key}, + {"sha256-big", cng.NewSHA256, append(key, make([]byte, 1000)...)}, + {"sha384", cng.NewSHA384, key}, + {"sha512", cng.NewSHA512, key}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - h := NewHMAC(tt.fn, tt.key) + h := cng.NewHMAC(tt.fn, tt.key) h.Write([]byte("hello")) sumHello := h.Sum(nil) - h = NewHMAC(tt.fn, tt.key) + h = cng.NewHMAC(tt.fn, tt.key) h.Write([]byte("hello world")) sumHelloWorld := h.Sum(nil) // Test that Sum has no effect on future Sum or Write operations. // This is a bit unusual as far as usage, but it's allowed // by the definition of Go hash.Hash, and some clients expect it to work. - h = NewHMAC(tt.fn, tt.key) + h = cng.NewHMAC(tt.fn, tt.key) h.Write([]byte("hello")) if sum := h.Sum(nil); !bytes.Equal(sum, sumHello) { t.Fatalf("1st Sum after hello = %x, want %x", sum, sumHello) diff --git a/cng/rand_test.go b/cng/rand_test.go index 80d1758..9654f6e 100644 --- a/cng/rand_test.go +++ b/cng/rand_test.go @@ -4,16 +4,18 @@ //go:build windows // +build windows -package cng +package cng_test import ( "io" "testing" + + "github.com/microsoft/go-crypto-winnative/cng" ) func TestRand(t *testing.T) { b := make([]byte, 5) - n, err := RandReader.Read(b) + n, err := cng.RandReader.Read(b) if err != nil { t.Fatal(err) } @@ -28,7 +30,7 @@ func TestRandBig(t *testing.T) { t.Skip("skipping test in short mode.") } b := make([]byte, 1<<32+60) - n, err := io.ReadFull(RandReader, b) + n, err := io.ReadFull(cng.RandReader, b) if err != nil { t.Fatal(err) } From 145cfd330146826869d7648a1e45d39214b2ba05 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Sun, 29 Sep 2024 09:39:30 +0200 Subject: [PATCH 6/6] factor out aes params validation --- cng/aes.go | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/cng/aes.go b/cng/aes.go index 654cb71..caac632 100644 --- a/cng/aes.go +++ b/cng/aes.go @@ -40,20 +40,27 @@ func (c *aesCipher) finalize() { func (c *aesCipher) BlockSize() int { return aesBlockSize } -func (c *aesCipher) Encrypt(dst, src []byte) { +// validateAndClipInputs checks that dst and src meet the [cipher.Block] +// interface requirements and clips them to a single block. +func (c *aesCipher) validateAndClipInputs(dst, src []byte) (d, s []byte) { if len(src) < aesBlockSize { panic("crypto/aes: input not full block") } if len(dst) < aesBlockSize { panic("crypto/aes: output not full block") } - - // cypher.Block.Encrypt() is documented to encrypt one full block - // at a time, so we truncate the input and output to the block size. - dst, src = dst[:aesBlockSize], src[:aesBlockSize] - if subtle.InexactOverlap(dst, src) { - panic("crypto/cipher: invalid buffer overlap") + // cypher.Block methods are documented to operate on + // one block at a time, so we truncate the input and output + // to the block size. + d, s = dst[:aesBlockSize], src[:aesBlockSize] + if subtle.InexactOverlap(d, s) { + panic("crypto/aes: invalid buffer overlap") } + return d, s +} + +func (c *aesCipher) Encrypt(dst, src []byte) { + dst, src = c.validateAndClipInputs(dst, src) var ret uint32 err := bcrypt.Encrypt(c.kh, src, nil, nil, dst, &ret, 0) @@ -67,19 +74,7 @@ func (c *aesCipher) Encrypt(dst, src []byte) { } func (c *aesCipher) Decrypt(dst, src []byte) { - if len(src) < aesBlockSize { - panic("crypto/aes: input not full block") - } - if len(dst) < aesBlockSize { - panic("crypto/aes: output not full block") - } - - // cypher.Block.Decrypt() is documented to decrypt one full block - // at a time, so we truncate the input and output to the block size. - dst, src = dst[:aesBlockSize], src[:aesBlockSize] - if subtle.InexactOverlap(dst, src) { - panic("crypto/cipher: invalid buffer overlap") - } + dst, src = c.validateAndClipInputs(dst, src) var ret uint32 err := bcrypt.Decrypt(c.kh, src, nil, nil, dst, &ret, 0)