quic: handle PATH_CHALLENGE and PATH_RESPONSE frames

We do not support path migration yet, and will ignore packets
sent from anything other than the peer's original address.
Handle PATH_CHALLENGE frames by sending a PATH_RESPONSE.
Handle PATH_RESPONSE frames by closing the connection
(since we never send a challenge to respond to).

For golang/go#58547

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

Author: neild

internal/quic/conn.go

@@ -37,6 +37,7 @@ type Conn struct {
 	connIDState connIDState
 	loss        lossState
 	streams     streamsState
+	path        pathState
 
 	// Packet protection keys, CRYPTO streams, and TLS state.
 	keysInitial   fixedKeyPair

internal/quic/conn_loss_test.go

@@ -663,6 +663,29 @@ func TestLostRetireConnectionIDFrame(t *testing.T) {
 	})
 }
 
+func TestLostPathResponseFrame(t *testing.T) {
+	// "Responses to path validation using PATH_RESPONSE frames are sent just once."
+	// https://www.rfc-editor.org/rfc/rfc9000.html#section-13.3-3.12
+	lostFrameTest(t, func(t *testing.T, pto bool) {
+		tc := newTestConn(t, clientSide)
+		tc.handshake()
+		tc.ignoreFrame(frameTypeAck)
+		tc.ignoreFrame(frameTypePing)
+
+		data := pathChallengeData{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}
+		tc.writeFrames(packetType1RTT, debugFramePathChallenge{
+			data: data,
+		})
+		tc.wantFrame("response to PATH_CHALLENGE",
+			packetType1RTT, debugFramePathResponse{
+				data: data,
+			})
+
+		tc.triggerLossOrPTO(packetType1RTT, pto)
+		tc.wantIdle("lost PATH_RESPONSE frame is not retransmitted")
+	})
+}
+
 func TestLostHandshakeDoneFrame(t *testing.T) {
 	// "The HANDSHAKE_DONE frame MUST be retransmitted until it is acknowledged."
 	// https://www.rfc-editor.org/rfc/rfc9000.html#section-13.3-3.16

internal/quic/conn_recv.go

@@ -46,11 +46,11 @@ func (c *Conn) handleDatagram(now time.Time, dgram *datagram) (handled bool) {
 				// https://www.rfc-editor.org/rfc/rfc9000#section-14.1-4
 				return false
 			}
-			n = c.handleLongHeader(now, ptype, initialSpace, c.keysInitial.r, buf)
+			n = c.handleLongHeader(now, dgram, ptype, initialSpace, c.keysInitial.r, buf)
 		case packetTypeHandshake:
-			n = c.handleLongHeader(now, ptype, handshakeSpace, c.keysHandshake.r, buf)
+			n = c.handleLongHeader(now, dgram, ptype, handshakeSpace, c.keysHandshake.r, buf)
 		case packetType1RTT:
-			n = c.handle1RTT(now, buf)
+			n = c.handle1RTT(now, dgram, buf)
 		case packetTypeRetry:
 			c.handleRetry(now, buf)
 			return true
@@ -86,7 +86,7 @@ func (c *Conn) handleDatagram(now time.Time, dgram *datagram) (handled bool) {
 	return true
 }
 
-func (c *Conn) handleLongHeader(now time.Time, ptype packetType, space numberSpace, k fixedKeys, buf []byte) int {
+func (c *Conn) handleLongHeader(now time.Time, dgram *datagram, ptype packetType, space numberSpace, k fixedKeys, buf []byte) int {
 	if !k.isSet() {
 		return skipLongHeaderPacket(buf)
 	}
@@ -125,7 +125,7 @@ func (c *Conn) handleLongHeader(now time.Time, ptype packetType, space numberSpa
 		c.logLongPacketReceived(p, buf[:n])
 	}
 	c.connIDState.handlePacket(c, p.ptype, p.srcConnID)
-	ackEliciting := c.handleFrames(now, ptype, space, p.payload)
+	ackEliciting := c.handleFrames(now, dgram, ptype, space, p.payload)
 	c.acks[space].receive(now, space, p.num, ackEliciting)
 	if p.ptype == packetTypeHandshake && c.side == serverSide {
 		c.loss.validateClientAddress()
@@ -138,7 +138,7 @@ func (c *Conn) handleLongHeader(now time.Time, ptype packetType, space numberSpa
 	return n
 }
 
-func (c *Conn) handle1RTT(now time.Time, buf []byte) int {
+func (c *Conn) handle1RTT(now time.Time, dgram *datagram, buf []byte) int {
 	if !c.keysAppData.canRead() {
 		// 1-RTT packets extend to the end of the datagram,
 		// so skip the remainder of the datagram if we can't parse this.
@@ -175,7 +175,7 @@ func (c *Conn) handle1RTT(now time.Time, buf []byte) int {
 	if c.logEnabled(QLogLevelPacket) {
 		c.log1RTTPacketReceived(p, buf)
 	}
-	ackEliciting := c.handleFrames(now, packetType1RTT, appDataSpace, p.payload)
+	ackEliciting := c.handleFrames(now, dgram, packetType1RTT, appDataSpace, p.payload)
 	c.acks[appDataSpace].receive(now, appDataSpace, p.num, ackEliciting)
 	return len(buf)
 }
@@ -252,7 +252,7 @@ func (c *Conn) handleVersionNegotiation(now time.Time, pkt []byte) {
 	c.abortImmediately(now, errVersionNegotiation)
 }
 
-func (c *Conn) handleFrames(now time.Time, ptype packetType, space numberSpace, payload []byte) (ackEliciting bool) {
+func (c *Conn) handleFrames(now time.Time, dgram *datagram, ptype packetType, space numberSpace, payload []byte) (ackEliciting bool) {
 	if len(payload) == 0 {
 		// "An endpoint MUST treat receipt of a packet containing no frames
 		// as a connection error of type PROTOCOL_VIOLATION."
@@ -373,6 +373,16 @@ func (c *Conn) handleFrames(now time.Time, ptype packetType, space numberSpace,
 				return
 			}
 			n = c.handleRetireConnectionIDFrame(now, space, payload)
+		case frameTypePathChallenge:
+			if !frameOK(c, ptype, __01) {
+				return
+			}
+			n = c.handlePathChallengeFrame(now, dgram, space, payload)
+		case frameTypePathResponse:
+			if !frameOK(c, ptype, ___1) {
+				return
+			}
+			n = c.handlePathResponseFrame(now, space, payload)
 		case frameTypeConnectionCloseTransport:
 			// Transport CONNECTION_CLOSE is OK in all spaces.
 			n = c.handleConnectionCloseTransportFrame(now, payload)
@@ -546,6 +556,24 @@ func (c *Conn) handleRetireConnectionIDFrame(now time.Time, space numberSpace, p
 	return n
 }
 
+func (c *Conn) handlePathChallengeFrame(now time.Time, dgram *datagram, space numberSpace, payload []byte) int {
+	data, n := consumePathChallengeFrame(payload)
+	if n < 0 {
+		return -1
+	}
+	c.handlePathChallenge(now, dgram, data)
+	return n
+}
+
+func (c *Conn) handlePathResponseFrame(now time.Time, space numberSpace, payload []byte) int {
+	data, n := consumePathResponseFrame(payload)
+	if n < 0 {
+		return -1
+	}
+	c.handlePathResponse(now, data)
+	return n
+}
+
 func (c *Conn) handleConnectionCloseTransportFrame(now time.Time, payload []byte) int {
 	code, _, reason, n := consumeConnectionCloseTransportFrame(payload)
 	if n < 0 {

internal/quic/conn_send.go

@@ -271,6 +271,13 @@ func (c *Conn) appendFrames(now time.Time, space numberSpace, pnum packetNumber,
 			return
 		}
 
+		// PATH_RESPONSE
+		if pad, ok := c.appendPathFrames(); !ok {
+			return
+		} else if pad {
+			defer c.w.appendPaddingTo(smallestMaxDatagramSize)
+		}
+
 		// All stream-related frames. This should come last in the packet,
 		// so large amounts of STREAM data don't crowd out other frames
 		// we may need to send.

internal/quic/conn_test.go

@@ -168,6 +168,7 @@ type testConn struct {
 	sentDatagrams [][]byte
 	sentPackets   []*testPacket
 	sentFrames    []debugFrame
+	lastDatagram  *testDatagram
 	lastPacket    *testPacket
 
 	recvDatagram chan *datagram
@@ -576,6 +577,7 @@ func (tc *testConn) readDatagram() *testDatagram {
 		}
 		p.frames = frames
 	}
+	tc.lastDatagram = d
 	return d
 }
 

internal/quic/frame_debug.go

@@ -77,6 +77,7 @@ func parseDebugFrame(b []byte) (f debugFrame, n int) {
 // debugFramePadding is a sequence of PADDING frames.
 type debugFramePadding struct {
 	size int
+	to   int // alternate for writing packets: pad to
 }
 
 func parseDebugFramePadding(b []byte) (f debugFramePadding, n int) {
@@ -95,6 +96,10 @@ func (f debugFramePadding) write(w *packetWriter) bool {
 	if w.avail() == 0 {
 		return false
 	}
+	if f.to > 0 {
+		w.appendPaddingTo(f.to)
+		return true
+	}
 	for i := 0; i < f.size && w.avail() > 0; i++ {
 		w.b = append(w.b, frameTypePadding)
 	}
@@ -584,7 +589,7 @@ func (f debugFrameRetireConnectionID) LogValue() slog.Value {
 
 // debugFramePathChallenge is a PATH_CHALLENGE frame.
 type debugFramePathChallenge struct {
-	data uint64
+	data pathChallengeData
 }
 
 func parseDebugFramePathChallenge(b []byte) (f debugFramePathChallenge, n int) {
@@ -593,7 +598,7 @@ func parseDebugFramePathChallenge(b []byte) (f debugFramePathChallenge, n int) {
 }
 
 func (f debugFramePathChallenge) String() string {
-	return fmt.Sprintf("PATH_CHALLENGE Data=%016x", f.data)
+	return fmt.Sprintf("PATH_CHALLENGE Data=%x", f.data)
 }
 
 func (f debugFramePathChallenge) write(w *packetWriter) bool {
@@ -603,13 +608,13 @@ func (f debugFramePathChallenge) write(w *packetWriter) bool {
 func (f debugFramePathChallenge) LogValue() slog.Value {
 	return slog.GroupValue(
 		slog.String("frame_type", "path_challenge"),
-		slog.String("data", fmt.Sprintf("%016x", f.data)),
+		slog.String("data", fmt.Sprintf("%x", f.data)),
 	)
 }
 
 // debugFramePathResponse is a PATH_RESPONSE frame.
 type debugFramePathResponse struct {
-	data uint64
+	data pathChallengeData
 }
 
 func parseDebugFramePathResponse(b []byte) (f debugFramePathResponse, n int) {
@@ -618,7 +623,7 @@ func parseDebugFramePathResponse(b []byte) (f debugFramePathResponse, n int) {
 }
 
 func (f debugFramePathResponse) String() string {
-	return fmt.Sprintf("PATH_RESPONSE Data=%016x", f.data)
+	return fmt.Sprintf("PATH_RESPONSE Data=%x", f.data)
 }
 
 func (f debugFramePathResponse) write(w *packetWriter) bool {
@@ -628,7 +633,7 @@ func (f debugFramePathResponse) write(w *packetWriter) bool {
 func (f debugFramePathResponse) LogValue() slog.Value {
 	return slog.GroupValue(
 		slog.String("frame_type", "path_response"),
-		slog.String("data", fmt.Sprintf("%016x", f.data)),
+		slog.String("data", fmt.Sprintf("%x", f.data)),
 	)
 }
 

internal/quic/packet_codec_test.go

@@ -517,7 +517,7 @@ func TestFrameEncodeDecode(t *testing.T) {
 		s: "PATH_CHALLENGE Data=0123456789abcdef",
 		j: `{"frame_type":"path_challenge","data":"0123456789abcdef"}`,
 		f: debugFramePathChallenge{
-			data: 0x0123456789abcdef,
+			data: pathChallengeData{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef},
 		},
 		b: []byte{
 			0x1a,                                           // Type (i) = 0x1a,
@@ -527,7 +527,7 @@ func TestFrameEncodeDecode(t *testing.T) {
 		s: "PATH_RESPONSE Data=0123456789abcdef",
 		j: `{"frame_type":"path_response","data":"0123456789abcdef"}`,
 		f: debugFramePathResponse{
-			data: 0x0123456789abcdef,
+			data: pathChallengeData{0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef},
 		},
 		b: []byte{
 			0x1b,                                           // Type (i) = 0x1b,

internal/quic/packet_parser.go

@@ -463,18 +463,17 @@ func consumeRetireConnectionIDFrame(b []byte) (seq int64, n int) {
 	return seq, n
 }
 
-func consumePathChallengeFrame(b []byte) (data uint64, n int) {
+func consumePathChallengeFrame(b []byte) (data pathChallengeData, n int) {
 	n = 1
-	var nn int
-	data, nn = consumeUint64(b[n:])
-	if nn < 0 {
-		return 0, -1
+	nn := copy(data[:], b[n:])
+	if nn != len(data) {
+		return data, -1
 	}
 	n += nn
 	return data, n
 }
 
-func consumePathResponseFrame(b []byte) (data uint64, n int) {
+func consumePathResponseFrame(b []byte) (data pathChallengeData, n int) {
 	return consumePathChallengeFrame(b) // identical frame format
 }
 

internal/quic/packet_writer.go

@@ -243,10 +243,7 @@ func (w *packetWriter) appendPingFrame() (added bool) {
 		return false
 	}
 	w.b = append(w.b, frameTypePing)
-	// Mark this packet as ack-eliciting and in-flight,
-	// but there's no need to record the presence of a PING frame in it.
-	w.sent.ackEliciting = true
-	w.sent.inFlight = true
+	w.sent.markAckEliciting() // no need to record the frame itself
 	return true
 }
 
@@ -495,23 +492,23 @@ func (w *packetWriter) appendRetireConnectionIDFrame(seq int64) (added bool) {
 	return true
 }
 
-func (w *packetWriter) appendPathChallengeFrame(data uint64) (added bool) {
+func (w *packetWriter) appendPathChallengeFrame(data pathChallengeData) (added bool) {
 	if w.avail() < 1+8 {
 		return false
 	}
 	w.b = append(w.b, frameTypePathChallenge)
-	w.b = binary.BigEndian.AppendUint64(w.b, data)
-	w.sent.appendAckElicitingFrame(frameTypePathChallenge)
+	w.b = append(w.b, data[:]...)
+	w.sent.markAckEliciting() // no need to record the frame itself
 	return true
 }
 
-func (w *packetWriter) appendPathResponseFrame(data uint64) (added bool) {
+func (w *packetWriter) appendPathResponseFrame(data pathChallengeData) (added bool) {
 	if w.avail() < 1+8 {
 		return false
 	}
 	w.b = append(w.b, frameTypePathResponse)
-	w.b = binary.BigEndian.AppendUint64(w.b, data)
-	w.sent.appendAckElicitingFrame(frameTypePathResponse)
+	w.b = append(w.b, data[:]...)
+	w.sent.markAckEliciting() // no need to record the frame itself
 	return true
 }