Subject: [quicwg/base-drafts] Remove some cruft from packet size section (#3262)

From: Martin Thomson <[email-masked]>
Date: Mon, 18 Nov 2019 16:46:51 -0800

This section had a bunch of text that wasn&#39;t updated as the result of
other changes elsewhere.  There was a suggestion that this could
reference other sections, but as this is in some ways the authoritative
section on the subject, I decided to keep the requirements.  That&#39;s
duplicative, but I couldn&#39;t see a simple way to deal with that.

Closes #3255.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/3262

-- Commit Summary --

  * Remove some cruft from packet size section

-- File Changes --

    M draft-ietf-quic-transport.md (29)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/3262.patch
https://github.com/quicwg/base-drafts/pull/3262.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3262

---

From: Nick Banks <[email-masked]>
Date: Mon, 18 Nov 2019 17:02:18 -0800

nibanks approved this pull request.

Everything looks good, except for the comment I have about already existing text.

>  A server MAY send a CONNECTION_CLOSE frame with error code PROTOCOL_VIOLATION in
-response to the first Initial packet it receives from a client if the UDP
-datagram is smaller than 1200 bytes. It MUST NOT send any other frame type in
-response, or otherwise behave as if any part of the offending packet was
-processed as valid.
+response to an Initial packet it receives from a client if the UDP datagram is
+smaller than 1200 bytes. It MUST NOT send any other frame type in response, or

I know it's not part of this PR, but why do we let what is essentially an unauthenticated packet to kill the connection? Shouldn't the packet just be dropped and ignored?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3262#pullrequestreview-318727533

---

From: Martin Thomson <[email-masked]>
Date: Mon, 18 Nov 2019 17:33:14 -0800

martinthomson commented on this pull request.

At that point, this is not creating the connection as opposed to removing an existing connection.  So this is functionally equivalent to dropping, but adds the friendly signal.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3262#discussion_r347693426

---

From: ianswett <[email-masked]>
Date: Mon, 18 Nov 2019 19:21:59 -0800

ianswett approved this pull request.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3262#pullrequestreview-318764195

---

From: Nick Banks <[email-masked]>
Date: Mon, 18 Nov 2019 20:44:11 -0800

nibanks commented on this pull request.

I'm more worried about this as an attack vector. Now that we support multiple initial packets for the client initial, there is more of a windows for an attacker to race a bad initial packet. Sending a 1 byte initial packet to kill your connection would be pretty easy; especially since the size validation check is going to happen pre-decryption, so the attacker doesn't even need to craft a well-formed packet.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3262#discussion_r347728735

---

From: Martin Thomson <[email-masked]>
Date: Tue, 19 Nov 2019 18:58:06 -0800

Ahh, I see what you mean.  Opening an issue.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3262#discussion_r348271289

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 19 Nov 2019 23:00:55 -0800

janaiyengar commented on this pull request.

some editorializing

> @@ -3410,27 +3410,22 @@ later time in the connection.
 The QUIC packet size includes the QUIC header and protected payload, but not the
 UDP or IP header.
 
-Clients MUST ensure they send the first Initial packet in a single IP packet.
-Similarly, the first Initial packet sent after receiving a Retry packet MUST be
-sent in a single IP packet.
-
-The payload of a UDP datagram carrying the first Initial packet MUST be expanded
-to at least 1200 bytes, by adding PADDING frames to the Initial packet and/or by
-coalescing the Initial packet (see {{packet-coalesce}}). Sending a UDP datagram
-of this size ensures that the network path supports a reasonable Maximum
-Transmission Unit (MTU), and helps reduce the amplitude of amplification attacks
-caused by server responses toward an unverified client address; see
+The payload of a UDP datagram carrying Initial packets from a client MUST be

I would suggest making this active. "A client MUST expand the payload of all UDP datagrams carrying Initial packets to at least 1200 bytes, by adding PADDING ..."

> @@ -3410,27 +3410,22 @@ later time in the connection.
 The QUIC packet size includes the QUIC header and protected payload, but not the
 UDP or IP header.
 
-Clients MUST ensure they send the first Initial packet in a single IP packet.
-Similarly, the first Initial packet sent after receiving a Retry packet MUST be
-sent in a single IP packet.
-
-The payload of a UDP datagram carrying the first Initial packet MUST be expanded
-to at least 1200 bytes, by adding PADDING frames to the Initial packet and/or by
-coalescing the Initial packet (see {{packet-coalesce}}). Sending a UDP datagram
-of this size ensures that the network path supports a reasonable Maximum
-Transmission Unit (MTU), and helps reduce the amplitude of amplification attacks
-caused by server responses toward an unverified client address; see
+The payload of a UDP datagram carrying Initial packets from a client MUST be
+expanded to at least 1200 bytes, by adding PADDING frames to the Initial packet
+and/or by coalescing the Initial packet (see {{packet-coalesce}}). Sending a UDP
+datagram of this size ensures that the network path supports a reasonable

```suggestion
datagram of this size ensures that the network path from the client to the server supports a reasonable
```

> @@ -3410,27 +3410,22 @@ later time in the connection.
 The QUIC packet size includes the QUIC header and protected payload, but not the
 UDP or IP header.
 
-Clients MUST ensure they send the first Initial packet in a single IP packet.
-Similarly, the first Initial packet sent after receiving a Retry packet MUST be
-sent in a single IP packet.
-
-The payload of a UDP datagram carrying the first Initial packet MUST be expanded
-to at least 1200 bytes, by adding PADDING frames to the Initial packet and/or by
-coalescing the Initial packet (see {{packet-coalesce}}). Sending a UDP datagram
-of this size ensures that the network path supports a reasonable Maximum
-Transmission Unit (MTU), and helps reduce the amplitude of amplification attacks
-caused by server responses toward an unverified client address; see
+The payload of a UDP datagram carrying Initial packets from a client MUST be
+expanded to at least 1200 bytes, by adding PADDING frames to the Initial packet
+and/or by coalescing the Initial packet (see {{packet-coalesce}}). Sending a UDP
+datagram of this size ensures that the network path supports a reasonable
+Maximum Transmission Unit (MTU), and helps reduce the amplitude of amplification

```suggestion
Maximum Transmission Unit (MTU). It also helps reduce the amplitude of amplification
```

>  {{address-validation}}.
 
-The datagram containing the first Initial packet from a client MAY exceed 1200
-bytes if the client believes that the Path Maximum Transmission Unit (PMTU)
-supports the size that it chooses.
+These datagrams containing Initial packets MAY exceed 1200 bytes if the client

```suggestion
Datagrams containing Initial packets MAY exceed 1200 bytes if the client
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3262#pullrequestreview-319580505

---

From: Martin Thomson <[email-masked]>
Date: Mon, 25 Nov 2019 16:31:47 -0800

@martinthomson pushed 1 commit.

f831a871f2c06df618c13b056d97cf0f6fefefc1  iyengareditorialization

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3262/files/12d3abd22ad130b21f29854fd487546dcd57eb6b..f831a871f2c06df618c13b056d97cf0f6fefefc1

---

From: Martin Thomson <[email-masked]>
Date: Mon, 25 Nov 2019 16:32:47 -0800

Merged #3262 into master.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3262#event-2830107182