From a968e40d31039a22286881cb3d17010ef6cf5a98 Mon Sep 17 00:00:00 2001 From: Davis Goodin Date: Tue, 9 Jan 2024 10:44:43 -0800 Subject: [PATCH] Fix TLS 1.3 additionalData length check --- cng/aes.go | 15 ++++++++++++--- cng/aes_test.go | 28 ++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/cng/aes.go b/cng/aes.go index e399eba..7fda49a 100644 --- a/cng/aes.go +++ b/cng/aes.go @@ -203,7 +203,14 @@ func (x *cbcCipher) SetIV(iv []byte) { const ( gcmTagSize = 16 gcmStandardNonceSize = 12 - gcmTlsAddSize = 13 + // TLS 1.2 additional data is constructed as: + // + // additional_data = seq_num(8) + TLSCompressed.type(1) + TLSCompressed.version(2) + TLSCompressed.length(2); + gcmTls12AddSize = 13 + // TLS 1.3 additional data is constructed as: + // + // additional_data = TLSCiphertext.opaque_type(1) || TLSCiphertext.legacy_record_version(2) || TLSCiphertext.length(2) + gcmTls13AddSize = 5 gcmTlsFixedNonceSize = 4 ) @@ -261,8 +268,10 @@ func (g *aesGCM) Seal(dst, nonce, plaintext, additionalData []byte) []byte { panic("cipher: message too large for buffer") } if g.tls != cipherGCMTLSNone { - if len(additionalData) != gcmTlsAddSize { - panic("cipher: incorrect additional data length given to GCM TLS") + if g.tls == cipherGCMTLS12 && len(additionalData) != gcmTls12AddSize { + panic("cipher: incorrect additional data length given to GCM TLS 1.2") + } else if g.tls == cipherGCMTLS13 && len(additionalData) != gcmTls13AddSize { + panic("cipher: incorrect additional data length given to GCM TLS 1.3") } counter := bigUint64(nonce[gcmTlsFixedNonceSize:]) if g.tls == cipherGCMTLS13 { diff --git a/cng/aes_test.go b/cng/aes_test.go index f8f4fec..c5c9039 100644 --- a/cng/aes_test.go +++ b/cng/aes_test.go @@ -73,12 +73,12 @@ func TestSealAndOpen(t *testing.T) { func TestSealAndOpenTLS(t *testing.T) { tests := []struct { name string - new func(c cipher.Block) (cipher.AEAD, error) + tls string mask func(n *[12]byte) }{ - {"1.2", NewGCMTLS, nil}, - {"1.3", NewGCMTLS13, nil}, - {"1.3_masked", NewGCMTLS13, func(n *[12]byte) { + {"1.2", "1.2", nil}, + {"1.3", "1.3", nil}, + {"1.3_masked", "1.3", func(n *[12]byte) { // Arbitrary mask in the high bits. n[9] ^= 0x42 // Mask the very first bit. This makes sure that if Seal doesn't @@ -93,7 +93,13 @@ func TestSealAndOpenTLS(t *testing.T) { if err != nil { t.Fatal(err) } - gcm, err := tt.new(ci) + var gcm cipher.AEAD + switch tt.tls { + case "1.2": + gcm, err = NewGCMTLS(ci) + case "1.3": + gcm, err = NewGCMTLS13(ci) + } if err != nil { t.Fatal(err) } @@ -108,9 +114,15 @@ func TestSealAndOpenTLS(t *testing.T) { } } plainText := []byte{0x01, 0x02, 0x03} - additionalData := make([]byte, 13) - additionalData[11] = byte(len(plainText) >> 8) - additionalData[12] = byte(len(plainText)) + var additionalData []byte + switch tt.tls { + case "1.2": + additionalData = make([]byte, 13) + case "1.3": + additionalData = []byte{23, 3, 3, 0, 0} + } + additionalData[len(additionalData)-2] = byte(len(plainText) >> 8) + additionalData[len(additionalData)-1] = byte(len(plainText)) sealed := gcm.Seal(nil, nonce[:], plainText, additionalData) assertPanic(t, func() { gcm.Seal(nil, nonce[:], plainText, additionalData)