[dev.boringcrypto] crypto/aes: panic on invalid dst, src overlap

I've now debugged multiple mysterious "inability to communicate"
bugs that manifest as a silent unexplained authentication failure but are
really crypto.AEAD.Open being invoked with badly aligned buffers.
In #21624 I suggested using a panic as the consequence of bad alignment,
so that this kind of failure is loud and clearly different from, say, a
corrupted or invalid message signature. Adding the panic here made
my failure very easy to track down, once I realized that was the problem.
I don't want to debug another one of these.

Also using this CL as an experiment to get data about the impact of
maybe applying this change more broadly in the master branch.

Change-Id: Id2e2d8e980439f8acacac985fc2674f7c96c5032
Reviewed-on: https://go-review.googlesource.com/63915
Reviewed-by: Adam Langley <agl@golang.org>

diff --git a/src/crypto/internal/boring/aes.go b/src/crypto/internal/boring/aes.go
index a977158c189faab466aaa8a824b62bae60959b96..cd7064e68679534253368f1597b31a9a71730cd3 100644 (file)
--- a/src/crypto/internal/boring/aes.go
+++ b/src/crypto/internal/boring/aes.go
@@ -59,6 +59,9 @@ func NewAESCipher(key []byte) (cipher.Block, error) {
 func (c *aesCipher) BlockSize() int { return aesBlockSize }
 
 func (c *aesCipher) Encrypt(dst, src []byte) {
+       if inexactOverlap(dst, src) {
+               panic("crypto/cipher: invalid buffer overlap")
+       }
        if len(src) < aesBlockSize {
                panic("crypto/aes: input not full block")
        }
@@ -72,6 +75,9 @@ func (c *aesCipher) Encrypt(dst, src []byte) {
 }
 
 func (c *aesCipher) Decrypt(dst, src []byte) {
+       if inexactOverlap(dst, src) {
+               panic("crypto/cipher: invalid buffer overlap")
+       }
        if len(src) < aesBlockSize {
                panic("crypto/aes: input not full block")
        }
@@ -93,6 +99,9 @@ type aesCBC struct {
 func (x *aesCBC) BlockSize() int { return aesBlockSize }
 
 func (x *aesCBC) CryptBlocks(dst, src []byte) {
+       if inexactOverlap(dst, src) {
+               panic("crypto/cipher: invalid buffer overlap")
+       }
        if len(src)%aesBlockSize != 0 {
                panic("crypto/cipher: input not full blocks")
        }
@@ -135,6 +144,9 @@ type aesCTR struct {
 }
 
 func (x *aesCTR) XORKeyStream(dst, src []byte) {
+       if inexactOverlap(dst, src) {
+               panic("crypto/cipher: invalid buffer overlap")
+       }
        if len(dst) < len(src) {
                panic("crypto/cipher: output smaller than input")
        }
@@ -262,6 +274,11 @@ func (g *aesGCM) Seal(dst, nonce, plaintext, additionalData []byte) []byte {
        }
        dst = dst[:n+len(plaintext)+gcmTagSize]
 
+       // Check delayed until now to make sure len(dst) is accurate.
+       if inexactOverlap(dst[n:], plaintext) {
+               panic("cipher: invalid buffer overlap")
+       }
+
        var outLen C.size_t
        ok := C._goboringcrypto_EVP_AEAD_CTX_seal(
                &g.ctx,
@@ -298,6 +315,11 @@ func (g *aesGCM) Open(dst, nonce, ciphertext, additionalData []byte) ([]byte, er
        }
        dst = dst[:n+len(ciphertext)-gcmTagSize]
 
+       // Check delayed until now to make sure len(dst) is accurate.
+       if inexactOverlap(dst[n:], ciphertext) {
+               panic("cipher: invalid buffer overlap")
+       }
+
        var outLen C.size_t
        ok := C._goboringcrypto_EVP_AEAD_CTX_open(
                &g.ctx,
@@ -313,3 +335,16 @@ func (g *aesGCM) Open(dst, nonce, ciphertext, additionalData []byte) ([]byte, er
        }
        return dst[:n+int(outLen)], nil
 }
+
+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]))
+}
+
+func inexactOverlap(x, y []byte) bool {
+       if len(x) == 0 || len(y) == 0 || &x[0] == &y[0] {
+               return false
+       }
+       return anyOverlap(x, y)
+}