Subject: [quicwg/base-drafts] handle reordered NEW_CONNECTION_ID frames (#3202)

From: Marten Seemann <[email-masked]>
Date: Wed, 06 Nov 2019 18:22:01 -0800

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

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

-- Commit Summary --

  * handle reordered NEW_CONNECTION_ID frames

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Wed, 06 Nov 2019 18:23:49 -0800

martinthomson approved this pull request.

> @@ -5398,6 +5398,10 @@ and preferred_address transport parameter connection IDs.  The peer SHOULD
 retire the corresponding connection IDs and send the corresponding
 RETIRE_CONNECTION_ID frames in a timely manner.
 
+An endpoint that receives a NEW_CONNECTION_ID frame with a sequence number
+smaller than the Retire Prior To field of a NEW_CONNECTION_ID frame received
+before, it MUST immediately send a corresponding RETIRE_CONNECTION_ID frame.

```suggestion
before, MUST immediately send a corresponding RETIRE_CONNECTION_ID frame.
```

-- 
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/3202#pullrequestreview-313077062

---

From: Marten Seemann <[email-masked]>
Date: Wed, 06 Nov 2019 18:24:51 -0800

@marten-seemann pushed 1 commit.

1568d427726afd00edf0bef4c4a6c4d5fe259321  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3202/files/50b9b3997901707201744740d131a08cb1ab31f5..1568d427726afd00edf0bef4c4a6c4d5fe259321

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 06 Nov 2019 18:39:32 -0800

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/3202#pullrequestreview-313080960

---

From: ianswett <[email-masked]>
Date: Thu, 07 Nov 2019 06:10:13 -0800

ianswett approved this pull request.

Small editorial suggestion, but LG

```suggestion
smaller than the Retire Prior To field of a previously received NEW_CONNECTION_ID
```

+An endpoint that receives a NEW_CONNECTION_ID frame with a sequence number
+smaller than the Retire Prior To field of a NEW_CONNECTION_ID frame received
+before, MUST immediately send a corresponding RETIRE_CONNECTION_ID frame.

```suggestion
frame MUST immediately send a corresponding RETIRE_CONNECTION_ID frame.
```

-- 
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/3202#pullrequestreview-313363137

---

From: Mike Bishop <[email-masked]>
Date: Thu, 07 Nov 2019 07:31:24 -0800

It might be cleaner to say that when you receive RPT, you need to retire the indicated sequence numbers even if you haven't received them yet.  Then you can just discard the NCID frames upon arrival.

-- 
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/3202#issuecomment-551129336

---

From: Eric Kinnear <[email-masked]>
Date: Thu, 07 Nov 2019 10:41:18 -0800

erickinnear approved this pull request.

Small tweak to @ianswett's wording suggestion, but otherwise looks good!

I would potentially also add
```suggestion
MUST immediately send a corresponding RETIRE_CONNECTION_ID frame that 
retires the newly received connection ID.
```
Since there's two NCID frames being referenced here.

-- 
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/3202#pullrequestreview-313552617

---

From: Eric Kinnear <[email-masked]>
Date: Thu, 07 Nov 2019 10:41:58 -0800

@MikeBishop That brings with it a request to replace them, which could get a little messy if I sent RPT of 10000, it seems like it would cut down on useless traffic if we didn't generate the RCID frames until someone actually tries to use them?

-- 
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/3202#issuecomment-551209763

---

From: Mike Bishop <[email-masked]>
Date: Thu, 07 Nov 2019 11:35:22 -0800

The situation is a bit perverse to begin with.  RPT isn't permitted to be >= the sequence number of the new CID, which means they've already issued you 10k CIDs for that situation to be real.

If the peer revokes a block of 10k CIDs via RPT, they're expecting to get 10k RCID frames, minus any that might already have been sent.  Yes, that means they might then issue a similarly large block of CIDs, which you will then forget silently and the insanity will play itself out again the next time they revoke a large batch.  This glut is best managed by using the existing TP and not permitting the peer to send you an unlimited number of CIDs.

This also argues, perhaps, for making the default something more sensible than "infinite" in #3197.

-- 
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/3202#issuecomment-551230648

---

From: Marten Seemann <[email-masked]>
Date: Thu, 07 Nov 2019 19:01:05 -0800

66992a654573597a569668be47b35fbf9384d08c  Ian's and Eric's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3202/files/1568d427726afd00edf0bef4c4a6c4d5fe259321..66992a654573597a569668be47b35fbf9384d08c

---

From: Marten Seemann <[email-masked]>
Date: Thu, 07 Nov 2019 19:06:20 -0800

> It might be cleaner to say that when you receive RPT, you need to retire the indicated sequence numbers even if you haven't received them yet. Then you can just discard the NCID frames upon arrival.

@MikeBishop There's a DoS attack here. If the attacker doesn't comply with the rule that new CIDs have to be issued in order (which is one of the non-enforceable MUSTs in the spec) and issues you CID 1000000 while with the same value for Retire Prior To, the endpoint would have to send a million RETIRE_CONNECTION_ID frames. I think the only way to reliable avoid this DoS is to only send RETIRE_CONNECTION_ID for CIDs that you actually received.

-- 
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/3202#issuecomment-551364763

---

From: Marten Seemann <[email-masked]>
Date: Thu, 07 Nov 2019 19:23:51 -0800

marten-seemann commented on this pull request.

Thanks! I applied your suggestions.

-- 
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/3202#discussion_r343975467

---

From: Martin Thomson <[email-masked]>
Date: Thu, 07 Nov 2019 19:25:25 -0800

martinthomson commented on this pull request.

> @@ -5398,6 +5398,11 @@ and preferred_address transport parameter connection IDs.  The peer SHOULD
 
+An endpoint that receives a NEW_CONNECTION_ID frame with a sequence number
+smaller than the Retire Prior To field of a previously received
+NEW_CONNECTION_ID frame, MUST immediately send a corresponding

```suggestion
NEW_CONNECTION_ID frame MUST immediately send a corresponding
```

-- 
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/3202#pullrequestreview-313765334

---

From: Marten Seemann <[email-masked]>
Date: Thu, 07 Nov 2019 19:26:12 -0800

30e554ae61fd41635c374647a07c2e2c5d85650a  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3202/files/66992a654573597a569668be47b35fbf9384d08c..30e554ae61fd41635c374647a07c2e2c5d85650a

---

From: Mike Bishop <[email-masked]>
Date: Fri, 08 Nov 2019 06:43:04 -0800

Larger change, but one solution to that might be permitting a RCID frame to retire ranges instead of a single CID.

-- 
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/3202#issuecomment-551853987

---

From: ianswett <[email-masked]>
Date: Fri, 08 Nov 2019 06:46:40 -0800

@MikeBishop what about RETIRE_CONNECTION_ID meaning that all IDs equal or less than that one have been retired?  That would force them to be used in order, which seems like a feature in my opinion.

-- 
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/3202#issuecomment-551855372

---

From: Mike Bishop <[email-masked]>
Date: Fri, 08 Nov 2019 07:04:01 -0800

We had that at one point, didn't we?  It is a feature until you start doing probes on other paths.  If you use a CID on a different path, then want to retire it, that also retires a CID you might still be using on the main path.  And it's no problem if you have another CID to roll forward to, but it forces you to burn more CIDs than you otherwise would.  I believe there was also an objection to that model that the gratuitous CID rolls leaked signals about use of other paths.

-- 
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/3202#issuecomment-551862258

---

From: Mike Bishop <[email-masked]>
Date: Fri, 08 Nov 2019 07:07:00 -0800

So I think if we were to do that, it would have to be a flag that indicates whether it's a point retirement or an "everything up to" retirement.  That would require renumbering some frames, which would be a pain.  🤔

-- 
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/3202#issuecomment-551863434

---

From: Marten Seemann <[email-masked]>
Date: Fri, 08 Nov 2019 07:48:22 -0800

> Larger change, but one solution to that might be permitting a RCID frame to retire ranges instead of a single CID.

@MikeBishop I'm not sure if there's a problem here that we need to solve. The DoS that I described only works if an endpoint sends RETIRE_CONNECTION_ID frames for CIDs that it hasn't received / doesn't remember any more. As long as you only retire CIDs that you're actually keeping track of (as the spec currently says), the DoS doesn't work.

-- 
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/3202#issuecomment-551879416

---

From: Martin Thomson <[email-masked]>
Date: Thu, 28 Nov 2019 17:07:21 -0800

@martinthomson pushed 1 commit.

745f4674876f25f212f9b4ddfcc1013242f88e24  Merge branch 'master' into handle-reordered-ncid-frames

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3202/files/30e554ae61fd41635c374647a07c2e2c5d85650a..745f4674876f25f212f9b4ddfcc1013242f88e24

---

From: Martin Thomson <[email-masked]>
Date: Thu, 28 Nov 2019 17:54:46 -0800

Merged #3202 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/3202#event-2840138158