From 37a17fe027db43f76fd88b056ddf588563fc8722 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Tue, 8 May 2018 20:09:24 -0400 Subject: [PATCH] internal/subtle: add Any/InexactOverlap (new package) and apply them across packages AnyOverlap and InexactOverlap implement checks for the aliasing requirements defined by the crypto/cipher interfaces. Apply them to all implementations as the actual requirement could be architecture-dependent and user code should not rely on undefined behavior. Updates golang/go#21624 Change-Id: I465de02fb3fec4e0c6f1fdee1ef6ae7ed5abff10 Reviewed-on: https://go-review.googlesource.com/112236 Reviewed-by: Brad Fitzpatrick --- chacha20poly1305/chacha20poly1305_amd64.go | 7 +++ chacha20poly1305/chacha20poly1305_generic.go | 7 +++ internal/chacha20/chacha_generic.go | 9 ++++ internal/subtle/aliasing.go | 30 ++++++++++++ internal/subtle/aliasing_test.go | 48 ++++++++++++++++++++ nacl/secretbox/secretbox.go | 9 +++- nacl/sign/sign.go | 7 +++ salsa20/salsa20.go | 4 ++ xts/xts.go | 8 ++++ 9 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 internal/subtle/aliasing.go create mode 100644 internal/subtle/aliasing_test.go diff --git a/chacha20poly1305/chacha20poly1305_amd64.go b/chacha20poly1305/chacha20poly1305_amd64.go index 07d18a3..ec13d13 100644 --- a/chacha20poly1305/chacha20poly1305_amd64.go +++ b/chacha20poly1305/chacha20poly1305_amd64.go @@ -9,6 +9,7 @@ package chacha20poly1305 import ( "encoding/binary" + "golang.org/x/crypto/internal/subtle" "golang.org/x/sys/cpu" ) @@ -55,6 +56,9 @@ func (c *chacha20poly1305) seal(dst, nonce, plaintext, additionalData []byte) [] setupState(&state, &c.key, nonce) ret, out := sliceForAppend(dst, len(plaintext)+16) + if subtle.InexactOverlap(out, plaintext) { + panic("chacha20poly1305: invalid buffer overlap") + } chacha20Poly1305Seal(out[:], state[:], plaintext, additionalData) return ret } @@ -69,6 +73,9 @@ func (c *chacha20poly1305) open(dst, nonce, ciphertext, additionalData []byte) ( ciphertext = ciphertext[:len(ciphertext)-16] ret, out := sliceForAppend(dst, len(ciphertext)) + if subtle.InexactOverlap(out, ciphertext) { + panic("chacha20poly1305: invalid buffer overlap") + } if !chacha20Poly1305Open(out, state[:], ciphertext, additionalData) { for i := range out { out[i] = 0 diff --git a/chacha20poly1305/chacha20poly1305_generic.go b/chacha20poly1305/chacha20poly1305_generic.go index 8d28ce2..c279712 100644 --- a/chacha20poly1305/chacha20poly1305_generic.go +++ b/chacha20poly1305/chacha20poly1305_generic.go @@ -8,6 +8,7 @@ import ( "encoding/binary" "golang.org/x/crypto/internal/chacha20" + "golang.org/x/crypto/internal/subtle" "golang.org/x/crypto/poly1305" ) @@ -17,6 +18,9 @@ func roundTo16(n int) int { func (c *chacha20poly1305) sealGeneric(dst, nonce, plaintext, additionalData []byte) []byte { ret, out := sliceForAppend(dst, len(plaintext)+poly1305.TagSize) + if subtle.InexactOverlap(out, plaintext) { + panic("chacha20poly1305: invalid buffer overlap") + } var polyKey [32]byte s := chacha20.New(c.key, [3]uint32{ @@ -62,6 +66,9 @@ func (c *chacha20poly1305) openGeneric(dst, nonce, ciphertext, additionalData [] binary.LittleEndian.PutUint64(polyInput[len(polyInput)-8:], uint64(len(ciphertext))) ret, out := sliceForAppend(dst, len(ciphertext)) + if subtle.InexactOverlap(out, ciphertext) { + panic("chacha20poly1305: invalid buffer overlap") + } if !poly1305.Verify(&tag, polyInput, &polyKey) { for i := range out { out[i] = 0 diff --git a/internal/chacha20/chacha_generic.go b/internal/chacha20/chacha_generic.go index 7ed1cd9..523751f 100644 --- a/internal/chacha20/chacha_generic.go +++ b/internal/chacha20/chacha_generic.go @@ -9,6 +9,8 @@ package chacha20 import ( "crypto/cipher" "encoding/binary" + + "golang.org/x/crypto/internal/subtle" ) // assert that *Cipher implements cipher.Stream @@ -41,6 +43,13 @@ func New(key [8]uint32, nonce [3]uint32) *Cipher { // the src buffers was passed in a single run. That is, Cipher // maintains state and does not reset at each XORKeyStream call. func (s *Cipher) XORKeyStream(dst, src []byte) { + if len(dst) < len(src) { + panic("chacha20: output smaller than input") + } + if subtle.InexactOverlap(dst[:len(src)], src) { + panic("chacha20: invalid buffer overlap") + } + // xor src with buffered keystream first if s.len != 0 { buf := s.buf[len(s.buf)-s.len:] diff --git a/internal/subtle/aliasing.go b/internal/subtle/aliasing.go new file mode 100644 index 0000000..61b67bf --- /dev/null +++ b/internal/subtle/aliasing.go @@ -0,0 +1,30 @@ +// Copyright 2018 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 subtle implements functions that are often useful in cryptographic +// code but require careful thought to use correctly. +package subtle // import "golang.org/x/crypto/internal/subtle" + +import "unsafe" + +// AnyOverlap reports whether x and y share memory at any (not necessarily +// corresponding) index. The memory beyond the slice length is ignored. +func AnyOverlap(x, y []byte) bool { + return len(x) > 0 && len(y) > 0 && + uintptr(unsafe.Pointer(&x[0])) <= uintptr(unsafe.Pointer(&y[len(y)-1])) && + uintptr(unsafe.Pointer(&y[0])) <= uintptr(unsafe.Pointer(&x[len(x)-1])) +} + +// InexactOverlap reports whether x and y share memory at any non-corresponding +// index. The memory beyond the slice length is ignored. Note that x and y can +// have different lengths and still not have any inexact overlap. +// +// InexactOverlap can be used to implement the requirements of the crypto/cipher +// AEAD, Block, BlockMode and Stream interfaces. +func InexactOverlap(x, y []byte) bool { + if len(x) == 0 || len(y) == 0 || &x[0] == &y[0] { + return false + } + return AnyOverlap(x, y) +} diff --git a/internal/subtle/aliasing_test.go b/internal/subtle/aliasing_test.go new file mode 100644 index 0000000..dfa7368 --- /dev/null +++ b/internal/subtle/aliasing_test.go @@ -0,0 +1,48 @@ +// Copyright 2018 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 subtle_test + +import ( + "testing" + + "golang.org/x/crypto/internal/subtle" +) + +var a, b [100]byte + +var aliasingTests = []struct { + x, y []byte + anyOverlap, inexactOverlap bool +}{ + {a[:], b[:], false, false}, + {a[:], b[:0], false, false}, + {a[:], b[:50], false, false}, + {a[40:50], a[50:60], false, false}, + {a[40:50], a[60:70], false, false}, + {a[:51], a[50:], true, true}, + {a[:], a[:], true, false}, + {a[:50], a[:60], true, false}, + {a[:], nil, false, false}, + {nil, nil, false, false}, + {a[:], a[:0], false, false}, +} + +func testAliasing(t *testing.T, i int, x, y []byte, anyOverlap, inexactOverlap bool) { + any := subtle.AnyOverlap(x, y) + if any != anyOverlap { + t.Errorf("%d: wrong AnyOverlap result, expected %v, got %v", i, anyOverlap, any) + } + inexact := subtle.InexactOverlap(x, y) + if inexact != inexactOverlap { + t.Errorf("%d: wrong InexactOverlap result, expected %v, got %v", i, inexactOverlap, any) + } +} + +func TestAliasing(t *testing.T) { + for i, tt := range aliasingTests { + testAliasing(t, i, tt.x, tt.y, tt.anyOverlap, tt.inexactOverlap) + testAliasing(t, i, tt.y, tt.x, tt.anyOverlap, tt.inexactOverlap) + } +} diff --git a/nacl/secretbox/secretbox.go b/nacl/secretbox/secretbox.go index 53ee83c..a98d1bd 100644 --- a/nacl/secretbox/secretbox.go +++ b/nacl/secretbox/secretbox.go @@ -35,6 +35,7 @@ This package is interoperable with NaCl: https://nacl.cr.yp.to/secretbox.html. package secretbox // import "golang.org/x/crypto/nacl/secretbox" import ( + "golang.org/x/crypto/internal/subtle" "golang.org/x/crypto/poly1305" "golang.org/x/crypto/salsa20/salsa" ) @@ -87,6 +88,9 @@ func Seal(out, message []byte, nonce *[24]byte, key *[32]byte) []byte { copy(poly1305Key[:], firstBlock[:]) ret, out := sliceForAppend(out, len(message)+poly1305.TagSize) + if subtle.AnyOverlap(out, message) { + panic("nacl: invalid buffer overlap") + } // We XOR up to 32 bytes of message with the keystream generated from // the first block. @@ -118,7 +122,7 @@ func Seal(out, message []byte, nonce *[24]byte, key *[32]byte) []byte { // Open authenticates and decrypts a box produced by Seal and appends the // message to out, which must not overlap box. The output will be Overhead // bytes smaller than box. -func Open(out []byte, box []byte, nonce *[24]byte, key *[32]byte) ([]byte, bool) { +func Open(out, box []byte, nonce *[24]byte, key *[32]byte) ([]byte, bool) { if len(box) < Overhead { return nil, false } @@ -143,6 +147,9 @@ func Open(out []byte, box []byte, nonce *[24]byte, key *[32]byte) ([]byte, bool) } ret, out := sliceForAppend(out, len(box)-Overhead) + if subtle.AnyOverlap(out, box) { + panic("nacl: invalid buffer overlap") + } // We XOR up to 32 bytes of box with the keystream generated from // the first block. diff --git a/nacl/sign/sign.go b/nacl/sign/sign.go index a9ac0a7..d076270 100644 --- a/nacl/sign/sign.go +++ b/nacl/sign/sign.go @@ -24,6 +24,7 @@ import ( "io" "golang.org/x/crypto/ed25519" + "golang.org/x/crypto/internal/subtle" ) // Overhead is the number of bytes of overhead when signing a message. @@ -47,6 +48,9 @@ func GenerateKey(rand io.Reader) (publicKey *[32]byte, privateKey *[64]byte, err func Sign(out, message []byte, privateKey *[64]byte) []byte { sig := ed25519.Sign(ed25519.PrivateKey((*privateKey)[:]), message) ret, out := sliceForAppend(out, Overhead+len(message)) + if subtle.AnyOverlap(out, message) { + panic("nacl: invalid buffer overlap") + } copy(out, sig) copy(out[Overhead:], message) return ret @@ -63,6 +67,9 @@ func Open(out, signedMessage []byte, publicKey *[32]byte) ([]byte, bool) { return nil, false } ret, out := sliceForAppend(out, len(signedMessage)-Overhead) + if subtle.AnyOverlap(out, signedMessage) { + panic("nacl: invalid buffer overlap") + } copy(out, signedMessage[Overhead:]) return ret, true } diff --git a/salsa20/salsa20.go b/salsa20/salsa20.go index 3ca6748..6f9bb10 100644 --- a/salsa20/salsa20.go +++ b/salsa20/salsa20.go @@ -24,6 +24,7 @@ package salsa20 // import "golang.org/x/crypto/salsa20" // TODO(agl): implement XORKeyStream12 and XORKeyStream8 - the reduced round variants of Salsa20. import ( + "golang.org/x/crypto/internal/subtle" "golang.org/x/crypto/salsa20/salsa" ) @@ -34,6 +35,9 @@ func XORKeyStream(out, in []byte, nonce []byte, key *[32]byte) { if len(out) < len(in) { panic("salsa20: output smaller than input") } + if subtle.InexactOverlap(out[:len(in)], in) { + panic("salsa20: invalid buffer overlap") + } var subNonce [16]byte diff --git a/xts/xts.go b/xts/xts.go index 92cbce9..9654e1f 100644 --- a/xts/xts.go +++ b/xts/xts.go @@ -25,6 +25,8 @@ import ( "crypto/cipher" "encoding/binary" "errors" + + "golang.org/x/crypto/internal/subtle" ) // Cipher contains an expanded key structure. It doesn't contain mutable state @@ -64,6 +66,9 @@ func (c *Cipher) Encrypt(ciphertext, plaintext []byte, sectorNum uint64) { if len(plaintext)%blockSize != 0 { panic("xts: plaintext is not a multiple of the block size") } + if subtle.InexactOverlap(ciphertext[:len(plaintext)], plaintext) { + panic("xts: invalid buffer overlap") + } var tweak [blockSize]byte binary.LittleEndian.PutUint64(tweak[:8], sectorNum) @@ -95,6 +100,9 @@ func (c *Cipher) Decrypt(plaintext, ciphertext []byte, sectorNum uint64) { if len(ciphertext)%blockSize != 0 { panic("xts: ciphertext is not a multiple of the block size") } + if subtle.InexactOverlap(plaintext[:len(ciphertext)], ciphertext) { + panic("xts: invalid buffer overlap") + } var tweak [blockSize]byte binary.LittleEndian.PutUint64(tweak[:8], sectorNum)