Skip to content

Commit

Permalink
[dev.boringcrypto] crypto/aes: panic on invalid dst, src overlap
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
rsc committed Sep 18, 2017
1 parent a929f3a commit 89ba9e3
Showing 1 changed file with 35 additions and 0 deletions.
35 changes: 35 additions & 0 deletions src/crypto/internal/boring/aes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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")
}
Expand All @@ -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")
}
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}

0 comments on commit 89ba9e3

Please sign in to comment.