Subject: [quicwg/base-drafts] clarify CID length requirements for VN packets (#4187)

From: Marten Seemann <[email-masked]>
Date: Wed, 07 Oct 2020 18:50:21 -0700

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

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

-- Commit Summary --

  * add text about CID lengths to the invariants
  * clarify that future QUIC versions might have different CID length requirements

-- File Changes --

    M draft-ietf-quic-invariants.md (4)
    M draft-ietf-quic-transport.md (8)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/4187.patch
https://github.com/quicwg/base-drafts/pull/4187.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/4187

---

From: Nick Banks <[email-masked]>
Date: Wed, 07 Oct 2020 19:00:29 -0700

@nibanks 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/4187#pullrequestreview-504380149

---

From: Martin Thomson <[email-masked]>
Date: Wed, 07 Oct 2020 20:03:27 -0700

@martinthomson commented on this pull request.

> +A server MUST NOT apply any restrictions to the length of the connection IDs
+when deciding whether to send a Version Negotiation Packet, even if it only
+supports QUIC versions that restrict the acceptable lengths of connection IDs.
+

I think that we can skip this bit.  As the size restrictions are version 1 specific, this is an oddly specific item to call out.

```suggestion
```

> @@ -4554,9 +4554,11 @@ connection IDs gives clients some assurance that the server received the packet
 and that the Version Negotiation packet was not generated by an off-path
 attacker.
 
-As future versions of QUIC may support Connection IDs larger than the version 1
-limit, Version Negotiation packets could carry Connection IDs that are longer
-than 20 bytes.
+Future versions of QUIC may have different requirements for the lengths of
+connection IDs larger than version 1. In particular, connection IDs might not

```suggestion
connection IDs. Other versions might allow connection IDs to be longer.  Other
versions might permit shorter or longer connection IDs to be included in packets
sent by a client to open a connection. A server MUST send a Version Negotiation
packet in response to a packet containing a connection ID smaller than 8 bytes
or larger than 20 bytes.
```

-- 
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/4187#pullrequestreview-504395089

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 07 Oct 2020 20:06:40 -0700

@kazuho commented on this pull request.

@martinthomson Isn't the second sentence (i.e., "Other versions might allow connection IDs to be longer.") excessive? The next sentence covers that, I think.

-- 
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/4187#discussion_r501421903

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 07 Oct 2020 20:13:54 -0700

-As future versions of QUIC may support Connection IDs larger than the version 1
-limit, Version Negotiation packets could carry Connection IDs that are longer
-than 20 bytes.
+Future versions of QUIC may have different requirements for the lengths of
+connection IDs larger than version 1. In particular, connection IDs might not
+be required to have a minimum lengths, and might be longer than 20 bytes.  A
+server therefore MUST respond with a Version Negotiation packets for all
+lengths of connection IDs allowed by the layout defined here.

I'm not sure if "MUST send" is the correct phrase here. This would be a contradiction to what we already have in [section 5.2.2](https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#name-server-packet-handling), states, quote: _if a server receives a packet that indicates an unsupported version but is large enough to initiate a new connection for any supported version, the server SHOULD send a Version Negotiation packet as described in Section 6.1_.

I think what we need to say here is that the CID length rules do not apply to other protocol versions, and no more.

-- 
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/4187#pullrequestreview-504400336

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 07 Oct 2020 20:22:18 -0700

PS. Therefore I think that this paragraph can be as simple as something like: _Note that version-specific rules for QUIC packets (e.g., limits on connection ID lengths) SHOULD NOT influence the decision of whether or not to send Version Negotiation packets, because they might change between the versions._

-- 
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/4187#discussion_r501425713

---

From: Martin Thomson <[email-masked]>
Date: Wed, 07 Oct 2020 20:27:14 -0700

That works for me also.  (I wasn't sure about the "MUST" for that reason, but I couldn't find the right words.)  A "MUST NOT" would work for your suggestion:

> Version-specific rules for QUIC packets MUST NOT influence a server decision about whether to send a Version Negotiation 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/4187#discussion_r501426920

---

From: Martin Thomson <[email-masked]>
Date: Wed, 07 Oct 2020 21:56:02 -0700

```suggestion
Future versions of QUIC may have different requirements for the lengths of
connection IDs.   Version-specific rules for QUIC packets MUST NOT influence a
server decision about whether to send a Version Negotiation 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/4187#pullrequestreview-504428074

---

From: Lars Eggert <[email-masked]>
Date: Thu, 08 Oct 2020 00:48:10 -0700

Could we also maybe add a pointer to the requirement that a DCID in an Initial must be eight bytes to Section 17.2? Because at the moment, it's buried in Section 7.2, where I didn't find it.

-- 
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/4187#issuecomment-705395200

---

From: Nick Banks <[email-masked]>
Date: Thu, 08 Oct 2020 08:10:09 -0700

@nibanks commented on this pull request.

Personally, when I was looking for the rules of when or when not to send a VN packet, I went to the Invariants doc. And I didn't really find much. IMO, the invariants doc should have all the generic rules: which fields to validate, which lengths to ignore (CIDs) and which to enforce (UDP payloads). As we get more QUIC versions, implementers shouldn't have to go searching through every document out there (ideally).

-- 
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/4187#discussion_r501798767

---

From: Nick Banks <[email-masked]>
Date: Thu, 08 Oct 2020 08:12:56 -0700

I don't think we should say a server MUST send a VN. Makes it sound like it's a protocol violation if I don't respond to a flood of packets, each with their own VN packet. For instance, if MsQuic receives a burst of packets from an endpoint, it generally sends a single VN packet in response.

-- 
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/4187#discussion_r501800822

---

From: Mike Bishop <[email-masked]>
Date: Thu, 08 Oct 2020 08:41:23 -0700

@MikeBishop commented on this pull request.

I think this is a good improvement, but some wordsmithing could still be done.

```suggestion
connection IDs. In particular, connection IDs might have a smaller minimum
length or a greater maximum length.  A
```

```suggestion
lengths of connection IDs allowed by the layout defined in Section 5.1
of {{QUIC-INVARIANTS}}.
```

-- 
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/4187#pullrequestreview-504910797

---

From: Marten Seemann <[email-masked]>
Date: Thu, 08 Oct 2020 21:09:10 -0700

@marten-seemann commented on this pull request.

I agree with @nibanks here, and I like @martinthomson's suggestion in https://github.com/quicwg/base-drafts/pull/4187/files#r501426920. I'll use that here instead of in -transport.

-- 
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/4187#discussion_r502178721

---

From: Marten Seemann <[email-masked]>
Date: Thu, 08 Oct 2020 21:09:48 -0700

@marten-seemann pushed 1 commit.

05cc5d9af4c53185585ae682b8d0f12673a2ac53  use @martinthomson's suggestion for the invariants

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4187/files/ec1741271763e767181e63e1f9aa1e5034be63bf..05cc5d9af4c53185585ae682b8d0f12673a2ac53

---

From: Marten Seemann <[email-masked]>
Date: Thu, 08 Oct 2020 21:10:02 -0700

Done in 05cc5d9af4c53185585ae682b8d0f12673a2ac53.

-- 
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/4187#discussion_r502178885

---

From: Marten Seemann <[email-masked]>
Date: Thu, 08 Oct 2020 21:15:20 -0700

3fec711e7223249e4e7934deef4b8e0974cba8cd  apply @martinthompson's and @MikeBishop's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4187/files/05cc5d9af4c53185585ae682b8d0f12673a2ac53..3fec711e7223249e4e7934deef4b8e0974cba8cd

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 08 Oct 2020 21:38:40 -0700

@kazuho 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/4187#pullrequestreview-505358362

---

From: Nick Banks <[email-masked]>
Date: Fri, 09 Oct 2020 07:15:47 -0700

-- 
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/4187#pullrequestreview-505713363

---

From: David Schinazi <[email-masked]>
Date: Fri, 09 Oct 2020 16:58:38 -0700

@DavidSchinazi commented on this pull request.

> @@ -316,6 +316,9 @@ connection IDs gives clients some assurance that the server received the packet
 
+Version-specific rules for QUIC packets MUST NOT influence a server decision

Are you saying that all QUIC servers need to send version negotiation packets in response to an 8-byte packet from an unknown version? That sounds like an amplification vector.

-- 
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/4187#pullrequestreview-506041756

---

From: Martin Thomson <[email-masked]>
Date: Sun, 11 Oct 2020 15:20:42 -0700

The text in the transport doc is fine; this is, as David says, a little sketchy without the minimum length constraint.  A prefix of "other than the minimum packet size, ..." might work, I guess.

-- 
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/4187#discussion_r502973446

---

From: Mike Bishop <[email-masked]>
Date: Mon, 12 Oct 2020 08:53:53 -0700

Or scope the MUST NOT specifically to CID interpretation.

-- 
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/4187#discussion_r503387184

---

From: Magnus Westerlund <[email-masked]>
Date: Thu, 15 Oct 2020 05:52:18 -0700

@gloinul commented on this pull request.

I think the CID specific aspect is fine. It is unclear what other aspects that are specified by the protocol and are not invariant here. 

-As future versions of QUIC may support Connection IDs larger than the version 1
-limit, Version Negotiation packets could carry Connection IDs that are longer
-than 20 bytes.
+Future versions of QUIC may have different requirements for the lengths of
+connection IDs. In particular, connection IDs might have a smaller minimum
+length or a greater maximum length.  Version-specific rules for QUIC packets

With the context of the first new sentence the second sentence is clearly apply to CIDs, but it indicates that there might be other aspects. What aspects are not invariant here? 

Yes, I think this proposal is unclear on what the "MUST NOT" is for. 

-- 
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/4187#pullrequestreview-509339993

---

From: Martin Thomson <[email-masked]>
Date: Thu, 15 Oct 2020 16:28:11 -0700

Merged #4187 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/4187#event-3883953366

---

From: Martin Thomson <[email-masked]>
Date: Thu, 15 Oct 2020 16:29:59 -0700

I've taken this change manually, after reverting the change to the invariants doc.  After looking at this more closely, the invariants doc does not include specifics of how to generate a Version Negotiation packet and this takes an awkward step in that direction.

Rather than start to import the rules into invariants piecemeal like this, I think that it is cleaner to leave the rules for generating a Version Negotiation packet as version-specific, which is already clearly the case.

-- 
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/4187#issuecomment-709642292

---

From: Martin Thomson <[email-masked]>
Date: Thu, 15 Oct 2020 16:32:29 -0700

Addendum: I missed the bit about this being a blanket "version-specific rules for packets".  I've changed this to "version-specific rules for connection IDs", to reflect the context better.  And to avoid tripping up on the packet size rules in version 1.

-- 
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/4187#issuecomment-709643002