quic: enforce AEAD integrity limit

Immediately close a connection after receiving too many
packets which fail authentication.

RFC 9001, section 6.6.

For golang/go#58547

Change-Id: I646b1e89d93fc013f35a2e7b751c4f7b578f42a9
Reviewed-on: https://go-review.googlesource.com/c/net/+/529596
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>

Author: neild

internal/quic/conn_recv.go

@@ -89,8 +89,13 @@ func (c *Conn) handle1RTT(now time.Time, buf []byte) int {
 	}
 
 	pnumMax := c.acks[appDataSpace].largestSeen()
-	p, n := parse1RTTPacket(buf, &c.keysAppData, connIDLen, pnumMax)
-	if n < 0 {
+	p, err := parse1RTTPacket(buf, &c.keysAppData, connIDLen, pnumMax)
+	if err != nil {
+		// A localTransportError terminates the connection.
+		// Other errors indicate an unparseable packet, but otherwise may be ignored.
+		if _, ok := err.(localTransportError); ok {
+			c.abort(now, err)
+		}
 		return -1
 	}
 	if buf[0]&reserved1RTTBits != 0 {

internal/quic/packet_codec_test.go

@@ -193,9 +193,9 @@ func TestRoundtripEncodeShortPacket(t *testing.T) {
 			w.b = append(w.b, test.payload...)
 			w.finish1RTTPacket(test.num, 0, connID, &test.k)
 			pkt := w.datagram()
-			p, n := parse1RTTPacket(pkt, &test.k, connIDLen, 0)
-			if n != len(pkt) {
-				t.Errorf("parse1RTTPacket: n=%v, want %v", n, len(pkt))
+			p, err := parse1RTTPacket(pkt, &test.k, connIDLen, 0)
+			if err != nil {
+				t.Errorf("parse1RTTPacket: err=%v, want nil", err)
 			}
 			if p.num != test.num || !bytes.Equal(p.payload, test.payload) {
 				t.Errorf("Round-trip encode/decode did not preserve packet.\nsent: num=%v, payload={%x}\ngot: num=%v, payload={%x}", test.num, test.payload, p.num, p.payload)

internal/quic/packet_parser.go

@@ -143,14 +143,14 @@ func skipLongHeaderPacket(pkt []byte) int {
 //
 // On input, pkt contains a short header packet, k the decryption keys for the packet,
 // and pnumMax the largest packet number seen in the number space of this packet.
-func parse1RTTPacket(pkt []byte, k *updatingKeyPair, dstConnIDLen int, pnumMax packetNumber) (p shortPacket, n int) {
+func parse1RTTPacket(pkt []byte, k *updatingKeyPair, dstConnIDLen int, pnumMax packetNumber) (p shortPacket, err error) {
 	pay, pnum, err := k.unprotect(pkt, 1+dstConnIDLen, pnumMax)
 	if err != nil {
-		return shortPacket{}, -1
+		return shortPacket{}, err
 	}
 	p.num = pnum
 	p.payload = pay
-	return p, len(pkt)
+	return p, nil
 }
 
 // Consume functions return n=-1 on conditions which result in FRAME_ENCODING_ERROR,

internal/quic/packet_protection.go

@@ -336,12 +336,13 @@ func updateSecret(suite uint16, secret []byte) (nextSecret []byte) {
 // of reordered packets before discarding the previous phase's keys after
 // an update.
 type updatingKeyPair struct {
-	phase       uint8 // current key phase (r.pkt[0], w.pkt[0])
-	updating    bool
-	minSent     packetNumber // min packet number sent since entering the updating state
-	minReceived packetNumber // min packet number received in the next phase
-	updateAfter packetNumber // packet number after which to initiate key update
-	r, w        updatingKeys
+	phase        uint8 // current key phase (r.pkt[0], w.pkt[0])
+	updating     bool
+	authFailures int64        // total packet unprotect failures
+	minSent      packetNumber // min packet number sent since entering the updating state
+	minReceived  packetNumber // min packet number received in the next phase
+	updateAfter  packetNumber // packet number after which to initiate key update
+	r, w         updatingKeys
 }
 
 func (k *updatingKeyPair) init() {
@@ -424,26 +425,45 @@ func (k *updatingKeyPair) unprotect(pkt []byte, pnumOff int, pnumMax packetNumbe
 	// Otherwise, use the next phase.
 	if hdr[0]&keyPhaseBit == k.phase && (!k.updating || pnum < k.minReceived) {
 		pay, err = k.r.pkt[0].unprotect(hdr, pay, pnum)
-		if err != nil {
-			return nil, 0, err
-		}
 	} else {
 		pay, err = k.r.pkt[1].unprotect(hdr, pay, pnum)
-		if err != nil {
-			return nil, 0, err
+		if err == nil {
+			if !k.updating {
+				// The peer has initiated a key update.
+				k.updating = true
+				k.minSent = maxPacketNumber
+				k.minReceived = pnum
+			} else {
+				k.minReceived = min(pnum, k.minReceived)
+			}
 		}
-		if !k.updating {
-			// The peer has initiated a key update.
-			k.updating = true
-			k.minSent = maxPacketNumber
-			k.minReceived = pnum
-		} else {
-			k.minReceived = min(pnum, k.minReceived)
+	}
+	if err != nil {
+		k.authFailures++
+		if k.authFailures >= aeadIntegrityLimit(k.r.suite) {
+			return nil, 0, localTransportError(errAEADLimitReached)
 		}
+		return nil, 0, err
 	}
 	return pay, pnum, nil
 }
 
+// aeadIntegrityLimit returns the integrity limit for an AEAD:
+// The maximum number of received packets that may fail authentication
+// before closing the connection.
+//
+// https://www.rfc-editor.org/rfc/rfc9001#section-6.6-4
+func aeadIntegrityLimit(suite uint16) int64 {
+	switch suite {
+	case tls.TLS_AES_128_GCM_SHA256, tls.TLS_AES_256_GCM_SHA384:
+		return 1 << 52
+	case tls.TLS_CHACHA20_POLY1305_SHA256:
+		return 1 << 36
+	default:
+		panic("BUG: unknown cipher suite")
+	}
+}
+
 // https://www.rfc-editor.org/rfc/rfc9001#section-5.2-2
 var initialSalt = []byte{0x38, 0x76, 0x2c, 0xf7, 0xf5, 0x59, 0x34, 0xb3, 0x4d, 0x17, 0x9a, 0xe6, 0xa4, 0xc8, 0x0c, 0xad, 0xcc, 0xbb, 0x7f, 0x0a}
 

internal/quic/tls_test.go

@@ -538,3 +538,66 @@ func TestConnCryptoBufferSizeExceeded(t *testing.T) {
 			code: errCryptoBufferExceeded,
 		})
 }
+
+func TestConnAEADLimitReached(t *testing.T) {
+	// "[...] endpoints MUST count the number of received packets that
+	// fail authentication during the lifetime of a connection.
+	// If the total number of received packets that fail authentication [...]
+	// exceeds the integrity limit for the selected AEAD,
+	// the endpoint MUST immediately close the connection [...]"
+	// https://www.rfc-editor.org/rfc/rfc9001#section-6.6-6
+	tc := newTestConn(t, clientSide)
+	tc.handshake()
+
+	var limit int64
+	switch suite := tc.conn.keysAppData.r.suite; suite {
+	case tls.TLS_AES_128_GCM_SHA256, tls.TLS_AES_256_GCM_SHA384:
+		limit = 1 << 52
+	case tls.TLS_CHACHA20_POLY1305_SHA256:
+		limit = 1 << 36
+	default:
+		t.Fatalf("conn.keysAppData.r.suite = %v, unknown suite", suite)
+	}
+
+	dstConnID := tc.conn.connIDState.local[0].cid
+	if tc.conn.connIDState.local[0].seq == -1 {
+		// Only use the transient connection ID in Initial packets.
+		dstConnID = tc.conn.connIDState.local[1].cid
+	}
+	invalid := tc.encodeTestPacket(&testPacket{
+		ptype:     packetType1RTT,
+		num:       1000,
+		frames:    []debugFrame{debugFramePing{}},
+		version:   1,
+		dstConnID: dstConnID,
+		srcConnID: tc.peerConnID,
+	}, 0)
+	invalid[len(invalid)-1] ^= 1
+	sendInvalid := func() {
+		t.Logf("<- conn under test receives invalid datagram")
+		tc.conn.sendMsg(&datagram{
+			b: invalid,
+		})
+		tc.wait()
+	}
+
+	// Set the conn's auth failure count to just before the AEAD integrity limit.
+	tc.conn.keysAppData.authFailures = limit - 1
+
+	tc.writeFrames(packetType1RTT, debugFramePing{})
+	tc.advanceToTimer()
+	tc.wantFrameType("auth failures less than limit: conn ACKs packet",
+		packetType1RTT, debugFrameAck{})
+
+	sendInvalid()
+	tc.writeFrames(packetType1RTT, debugFramePing{})
+	tc.advanceToTimer()
+	tc.wantFrameType("auth failures at limit: conn closes",
+		packetType1RTT, debugFrameConnectionCloseTransport{
+			code: errAEADLimitReached,
+		})
+
+	tc.writeFrames(packetType1RTT, debugFramePing{})
+	tc.advance(1 * time.Second)
+	tc.wantIdle("auth failures at limit: conn does not process additional packets")
+}