Skip to content

Commit 770149e

Browse files
committed
quic: pad ack-eliciting server Initial datagrams
UDP datagrams containing Initial packets are expanded to 1200 bytes to validate that the path is capable of supporting the smallest allowed maximum QUIC datagram size. (In addition, client Initial packets must be sent in datagrams of at least 1200 bytes, to defend against amplification attacks.) We were expanding client datagrams containing Initial packets, but not server datagrams. Fix this. (More specifically, server datagrams must be expanded to 1200 bytes when they contain ack-eliciting Initial packets.) RFC 9000, Section 14.1. Change-Id: I0c0c36321c055e960be3e29a49d7cb7620640b82 Reviewed-on: https://go-review.googlesource.com/c/net/+/538776 Reviewed-by: Jonathan Amsterdam <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 05086a7 commit 770149e

File tree

7 files changed

+16
-13
lines changed

7 files changed

+16
-13
lines changed

internal/quic/conn_recv.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func (c *Conn) handleDatagram(now time.Time, dgram *datagram) {
2424
ptype := getPacketType(buf)
2525
switch ptype {
2626
case packetTypeInitial:
27-
if c.side == serverSide && len(dgram.b) < minimumClientInitialDatagramSize {
27+
if c.side == serverSide && len(dgram.b) < paddedInitialDatagramSize {
2828
// Discard client-sent Initial packets in too-short datagrams.
2929
// https://www.rfc-editor.org/rfc/rfc9000#section-14.1-4
3030
return

internal/quic/conn_send.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,11 @@ func (c *Conn) maybeSend(now time.Time) (next time.Time) {
7777
}
7878
sentInitial = c.w.finishProtectedLongHeaderPacket(pnumMaxAcked, c.keysInitial.w, p)
7979
if sentInitial != nil {
80-
// Client initial packets need to be sent in a datagram padded to
81-
// at least 1200 bytes. We can't add the padding yet, however,
82-
// since we may want to coalesce additional packets with this one.
83-
if c.side == clientSide {
80+
// Client initial packets and ack-eliciting server initial packaets
81+
// need to be sent in a datagram padded to at least 1200 bytes.
82+
// We can't add the padding yet, however, since we may want to
83+
// coalesce additional packets with this one.
84+
if c.side == clientSide || sentInitial.ackEliciting {
8485
pad = true
8586
}
8687
}
@@ -123,7 +124,7 @@ func (c *Conn) maybeSend(now time.Time) (next time.Time) {
123124
// 1-RTT packets have no length field and extend to the end
124125
// of the datagram, so if we're sending a datagram that needs
125126
// padding we need to add it inside the 1-RTT packet.
126-
c.w.appendPaddingTo(minimumClientInitialDatagramSize)
127+
c.w.appendPaddingTo(paddedInitialDatagramSize)
127128
pad = false
128129
}
129130
if logPackets {
@@ -149,7 +150,7 @@ func (c *Conn) maybeSend(now time.Time) (next time.Time) {
149150
// Pad out the datagram with zeros, coalescing the Initial
150151
// packet with invalid packets that will be ignored by the peer.
151152
// https://www.rfc-editor.org/rfc/rfc9000.html#section-14.1-1
152-
for len(buf) < minimumClientInitialDatagramSize {
153+
for len(buf) < paddedInitialDatagramSize {
153154
buf = append(buf, 0)
154155
// Technically this padding isn't in any packet, but
155156
// account it to the Initial packet in this datagram

internal/quic/listener.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ func (l *Listener) handleUnknownDestinationDatagram(m *datagram) {
269269
return
270270
}
271271
p, ok := parseGenericLongHeaderPacket(m.b)
272-
if !ok || len(m.b) < minimumClientInitialDatagramSize {
272+
if !ok || len(m.b) < paddedInitialDatagramSize {
273273
return
274274
}
275275
switch p.version {
@@ -382,7 +382,7 @@ func (l *Listener) sendConnectionClose(in genericLongPacket, addr netip.AddrPort
382382
srcConnID: in.dstConnID,
383383
}
384384
const pnumMaxAcked = 0
385-
w.reset(minimumClientInitialDatagramSize)
385+
w.reset(paddedInitialDatagramSize)
386386
w.startProtectedLongHeaderPacket(pnumMaxAcked, p)
387387
w.appendConnectionCloseTransportFrame(code, 0, "")
388388
w.finishProtectedLongHeaderPacket(pnumMaxAcked, keys.w, p)

internal/quic/listener_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ func (tl *testListener) writeDatagram(d *testDatagram) {
172172
}
173173
pad := 0
174174
if p.ptype == packetType1RTT {
175-
pad = d.paddedSize
175+
pad = d.paddedSize - len(buf)
176176
}
177177
buf = append(buf, encodeTestPacket(tl.t, tc, p, pad)...)
178178
}

internal/quic/quic.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,10 @@ const (
5858
// https://www.rfc-editor.org/rfc/rfc9002.html#section-6.1.2-6
5959
const timerGranularity = 1 * time.Millisecond
6060

61-
// Minimum size of a UDP datagram sent by a client carrying an Initial packet.
61+
// Minimum size of a UDP datagram sent by a client carrying an Initial packet,
62+
// or a server containing an ack-eliciting Initial packet.
6263
// https://www.rfc-editor.org/rfc/rfc9000#section-14.1
63-
const minimumClientInitialDatagramSize = 1200
64+
const paddedInitialDatagramSize = 1200
6465

6566
// Maximum number of streams of a given type which may be created.
6667
// https://www.rfc-editor.org/rfc/rfc9000.html#section-4.6-2

internal/quic/tls_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ func handshakeDatagrams(tc *testConn) (dgrams []*testDatagram) {
148148
},
149149
},
150150
}},
151+
paddedSize: 1200,
151152
}, {
152153
// Client Initial + Handshake + 1-RTT
153154
packets: []*testPacket{{

internal/quic/version_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestVersionNegotiationServerReceivesUnknownVersion(t *testing.T) {
3030
pkt = append(pkt, dstConnID...)
3131
pkt = append(pkt, byte(len(srcConnID)))
3232
pkt = append(pkt, srcConnID...)
33-
for len(pkt) < minimumClientInitialDatagramSize {
33+
for len(pkt) < paddedInitialDatagramSize {
3434
pkt = append(pkt, 0)
3535
}
3636

0 commit comments

Comments
 (0)