Subject: [quicwg/base-drafts] Limit RCID state (#3547)

From: martinduke <[email-masked]>
Date: Thu, 26 Mar 2020 13:37:26 -0700

This is attempt to write down @martinthomson &#39;s proposal for #3509. This was surprisingly hard to write, and undoubtedly could be improved.

There are cleaner ways to solve this problem, but this one is lowest-impact to the existing spec.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Limit RCID state

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Thu, 26 Mar 2020 14:54:38 -0700

@ianswett commented on this pull request.

I don't think this describes quite what I had in mind, but maybe we can simplify this by not requiring sending RETIRE_CONNECTION_ID in this case?

> @@ -1069,6 +1069,18 @@ to cease using the connection IDs when requested can result in connection
 failures, as the issuing endpoint might be unable to continue using the
 connection IDs with the active connection.
 
+An endpoint MAY elect to only send or retransmit RETIRE_CONNECTION_ID frames

```suggestion
An endpoint MAY elect to only send RETIRE_CONNECTION_ID frames
```

> @@ -1069,6 +1069,18 @@ to cease using the connection IDs when requested can result in connection
 failures, as the issuing endpoint might be unable to continue using the
 connection IDs with the active connection.
 
+An endpoint MAY elect to only send or retransmit RETIRE_CONNECTION_ID frames
+with sequence numbers greater than or equal to the highest Retire Prior To field
+received minus its advertised active_connection_id_limit. This bounds the
+necessary state at the endpoint. For example, if an endpoint receives a Retire
+Prior To field of 7 and has an advertised active_connection_id_limit of 4, it
+may cease sending any RETIRE_CONNECTION_ID frame with sequence number less than

Is this the functionality we want?  If we're going to go this direction, we might as well just say you don't need to send RETIRE_CONNECTION_ID for anything below Retire Prior To, and you can assume it'll be retired 'soon'.  Maybe that's a good solution anyway?

-- 
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/3547#pullrequestreview-382444185

---

From: martinduke <[email-masked]>
Date: Thu, 26 Mar 2020 15:03:20 -0700

@martinduke commented on this pull request.

The value of RCID is to let the receiver know that it will never receive a packet with that CID again, barring packet reordering. (one could use an ACK of NCID as an implicit signal, though we don't do that in QUIC). I think we want to preserve that. If the language I wrote puts clients in the situation where they're not getting all the RCIDs, it's because they are buggy, malicious, or in a pretty extreme corner case.

Moreover your suggestion is a wider departure from the spec, likely to create more problems for existing implementations.

-- 
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/3547#discussion_r398919893

---

From: martinduke <[email-masked]>
Date: Thu, 26 Mar 2020 15:04:57 -0700

I really wanted to make it clear that an endpoint might have sent RCID but choose to abandon retransmission if lost, based on this criterion. I may not have worded it well, but I don't think your suggestion fixes 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/3547#discussion_r398920589

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 26 Mar 2020 16:45:58 -0700

@kazuho requested changes on this pull request.

I'm afraid this advice might be inadequate.

It is possible to mount attacks without using Retire Prior To (see https://github.com/quicwg/base-drafts/issues/3509#issuecomment-597970501), and therefore the defense should rather not depend on the value of Retire Prior To.

I think it would be better to state that:
* An endpoint SHOULD limit the number of unacknowledged RETIRE_CONNECTION_ID frames (or the count of sequence numbers contained in those frames) that the endpoint tracks.
* That limitation SHOULD be no less than 2 * max_connection_id_limit.

I think that was what @martinthomson had in mind when he wrote https://github.com/quicwg/base-drafts/issues/3509#issuecomment-597425775.

`2 * max_connection_id_limit` is a better ballpark figure than `max_connection_id_limit`, because it gives some confidence to the issuer of CIDs that it can issue a new CID immediately when it receives RCID, without the fear of that causing some state to be lost in the peer.

-- 
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/3547#pullrequestreview-382494284

---

From: martinduke <[email-masked]>
Date: Thu, 26 Mar 2020 17:12:55 -0700

I guess we're going to have this discussion in two places, but I'm not sure the non-RPT case is a valid threat model.

I am a little concerned with your formulation that the endpoint could make bad choices about which RCIDs it decides to stick with. For example, we might never increase RPT and for some reason have a low sequence number that sticks around and is still being used. The server really better actually send the RCID.

I'm happy to haggle about the exact threshold, but my formulation frees the endpoint of all the state only when the client is doing something legal but silly.

-- 
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/3547#discussion_r398965329

---

From: Martin Thomson <[email-masked]>
Date: Thu, 26 Mar 2020 22:11:18 -0700

@martinthomson commented on this pull request.

I didn't have a specific limit in mind.  I might consider active_connection_id_limit as the number (which bounds the total number of things you need to remember to 2 times that as you might also need to remember connection IDs for active use as well).  So I don't think that you need to specify a limit, just say that endpoints need to track the highest sequence number that needs to be retired and a contiguous block of sequences numbers for which it will attempt to send RETIRE_CONNECTION_ID frames.  That block might start at the bottom of the overall block of sequence numbers that require retirement.

This seems too concrete to me.

-- 
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/3547#discussion_r399038671

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 26 Mar 2020 23:37:08 -0700

@kazuho commented on this pull request.

@martinthomson 
> I didn't have a specific limit in mind. I might consider active_connection_id_limit as the number (which bounds the total number of things you need to remember to 2 times that as you might also need to remember connection IDs for active use as well).

Thank you for the clarification. If that is the case, let my explain why I think `max_connection_id_limit * 2` is the correct value.

The draft currently allows an endpoint to request retirement of all the active CIDs that the peer retains at once. We explicitly define how the peer should behave in such case; see https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#section-5.1.2-5. That means that the use of Retire Prior To might cause as much as *active_connection_id_limit* CIDs to be scheduled to be sent using RCID frames at once.

We also assume that the use of Retire Prior To is going to be infrequent.

Therefore, we need *active_connection_id_limit* in conjunction to the use of Retire Prior To.

Concurrently, the peer might be running business as usual, consuming CIDs as necessary. That means that we need some room for these too. We might argue that the active_connection_id_limit selected by the peer reflects the maximum pace that that peer is going to consume CIDs.

Based on this observation, my argument is that the sum of the two requirements makes sense; i.e. `max_connection_id_limit * 2`.

-- 
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/3547#discussion_r399060761

---

From: ianswett <[email-masked]>
Date: Fri, 27 Mar 2020 04:54:15 -0700

You can only retire as many as you store, so the only way you could have more than max_connection_id_limit RETIRE_CONNECTION_ID frames outstanding is if you need to retransmit all of them due to loss and subsequently you've both received a new connection ID AND need to retire it.  Retiring it because you decided to use it typically takes at least a round trip, so it's most likely the subsequent retiring is due to the peer's Retire Prior To increasing again.

Given the above situation starts looking increasingly like an attack(or a bug) and sending RETIRE_CONNECTION_ID quickly for the Retire Prior To case isn't critical, I'd suggest max_connecion_id_limit is a better recommendation.

-- 
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/3547#discussion_r399211747

---

From: ianswett <[email-masked]>
Date: Fri, 27 Mar 2020 05:20:18 -0700

How about we recommend limiting the number of RETIRE_CONNECTION_ID frames outstanding, but implementations SHOULD be willing to have at least max_connection_id_limit outstanding to allow a peer to retire all previously issued CIDs?

That isn't overly proscriptive, but gives a reasonable lower bound on how many implementations should allow.

-- 
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/3547#discussion_r399224566

---

From: ianswett <[email-masked]>
Date: Fri, 27 Mar 2020 05:35:16 -0700

I left some specific suggestions, PTAL.

We typically don't talk about retransmissions, so this read oddly to me.

+3.
+
+Peers are discouraged from issuing updates of the Retire Prior To field before

```suggestion
Peers SHOULD NOT issue updates of the Retire Prior To field before
```

> +To. Doing so may result in the peer being uncertain if it is to receive
+additional packets with an old Connection ID.

```suggestion
To. Doing so may result in the peer being unwilling to immediately retire all the connection IDs, if retiring them would cause it to exceed its limit on outstanding RETIRE_CONNECTION_ID frames, as described above.
```

+3.

```suggestion
An endpoint MAY elect to limit the number of outstanding RETIRE_CONNECTION_ID frames
to bound the necessary state.  In order to allow a peer to retire all previously issued
connection IDs, this limit SHOULD be at least the active_connection_id_limit.
```

-- 
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/3547#pullrequestreview-382811114

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 27 Mar 2020 05:50:39 -0700

Consider the case where  client has sent an RCID frame. The server has received that frame and has sent an NCID frame carrying a replacement. But that server has not sent an ACK yet, or the ACK it has sent has been lost.

At this moment, the server might decide to update the CID encryption key, and try to retire all the CIDs that has been provided to the client. When the NCID frame carrying the request to retire reaches the client, the client is now required to track retirement of `max_connection_id_limit + 1` CIDs.

I agree that this is a corner case. But our previous consensus has been to allow an endpoint to request retirement of all the CIDs that have been provided at once. Assuming that we want to retain that consensus, requiring endpoints to track *more* than `max_connection_id_limit` is IMO the logical choice.

The effect of *not* requiring that would be that an endpoint that decides to retire the CIDs that it has provided to the peer has to do retirement in multiple phases; e.g., first request retirement of half of them, wait for RCID frames, then retire the other half. That's much more complicated than just saying recommending endpoints to track something like `max_connection_id_limit * 2`.

-- 
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/3547#discussion_r399240482

---

From: ianswett <[email-masked]>
Date: Fri, 27 Mar 2020 06:04:55 -0700

I don't want to force retirement to be in multiple phases in edge cases like this, but I think it's fine to limit RCID to max_connection_id_limit outstanding frames.  Later, when some RCID frames get acknowledged, check if there are more CIDs that need retiring.  The case when the limit is exceeded needs to be handled in some way, so I'd rather have that code hit very infrequently than basically never.

An alternative I'd be ok with is to use max_connection_id_limit*2 and if the limit is exceeded, close the connection.  I don't see a problem with that, as we have limits like that already in our code, and it's very simple.  But it's fairly different from what we've been discussing.

-- 
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/3547#discussion_r399248483

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 27 Mar 2020 06:20:17 -0700

> I don't want to force retirement to be in multiple phases in edge cases like this, but I think it's fine to limit RCID to max_connection_id_limit outstanding frames. Later, when some RCID frames ge…

I think I agree that tracking 2*max_connection_id_limit of RCID frames in just one way taking care of these edge cases. I'm fine with leaving it up to the implementations how they would deal with this problem.

-- 
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/3547#discussion_r399257766

---

From: martinduke <[email-masked]>
Date: Fri, 27 Mar 2020 09:47:36 -0700

Would it be better say just "send", and then have a separate sentence like "an endpoint MAY abandon retransmission of frames retiring sequence numbers that meet these conditions."?

-- 
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/3547#discussion_r399401876

---

From: martinduke <[email-masked]>
Date: Fri, 27 Mar 2020 11:32:44 -0700

@martinduke pushed 1 commit.

efd55324649513ffc2cd7299ab3a19e094f81f17  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/3547/files/9374722d4f723e5cd1889ade44e10ab603bb710b..efd55324649513ffc2cd7299ab3a19e094f81f17

---

From: martinduke <[email-masked]>
Date: Fri, 27 Mar 2020 11:33:25 -0700

9e1d5db348262fab522e0c794698031157aa67d7  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/3547/files/efd55324649513ffc2cd7299ab3a19e094f81f17..9e1d5db348262fab522e0c794698031157aa67d7

---

From: martinduke <[email-masked]>
Date: Fri, 27 Mar 2020 11:41:41 -0700

b98bb9ea461c2099c2c0672e0356fc06b002c826  Ian's comments plus some additional text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/9e1d5db348262fab522e0c794698031157aa67d7..b98bb9ea461c2099c2c0672e0356fc06b002c826

---

From: martinduke <[email-masked]>
Date: Fri, 27 Mar 2020 11:45:39 -0700

your new text solves this problem, thanks

-- 
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/3547#discussion_r399471118

---

From: martinduke <[email-masked]>
Date: Fri, 27 Mar 2020 11:46:19 -0700

a62abddacec3745841f89e9b40a76c64811edac6  Whoops, missed one

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/b98bb9ea461c2099c2c0672e0356fc06b002c826..a62abddacec3745841f89e9b40a76c64811edac6

---

From: martinduke <[email-masked]>
Date: Fri, 27 Mar 2020 11:49:40 -0700

@ianswett thanks for the new text, that helped.

I think we've resolved outstanding issues, unless my new text has created new problems.

2 possible ways to enlarge this solution:
- should we describe this attack in Security Considerations? As these MAYs are an effort to counter something in particular, perhaps we should discuss.
- it would clean up some accounting issues if each NCID frame MUST set Retire Prior To to more than the maximum continuous sequence number that it had recieved an RCID for. Worthwhile?

-- 
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/3547#issuecomment-605209817

---

From: ianswett <[email-masked]>
Date: Fri, 27 Mar 2020 12:02:49 -0700

@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/3547#pullrequestreview-383127752

---

From: Mike Bishop <[email-masked]>
Date: Fri, 27 Mar 2020 13:08:05 -0700

> it would clean up some accounting issues if each NCID frame MUST set Retire Prior To to more than the maximum continuous sequence number that it had recieved an RCID for. Worthwhile?

That's interesting to send at first glance, but then poses the question of what the recipient of NCID should do when it sees an RPT value change.  There's a difference in behavior between "I've seen these" and "you need to send these."

-- 
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/3547#issuecomment-605293035

---

From: ianswett <[email-masked]>
Date: Fri, 27 Mar 2020 14:26:38 -0700

I think this PR is sufficient to both document the issue and provide some minimum guidance, so I'd suggest we editorialize a bit and then move forward with a solution along these lines.

-- 
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/3547#issuecomment-605322051

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 27 Mar 2020 16:34:33 -0700

> @@ -1069,6 +1069,17 @@ to cease using the connection IDs when requested can result in connection
 
+An endpoint MAY elect to limit the number of outstanding RETIRE_CONNECTION_ID

I think this should be a SHOULD.

In case of a client-driven attack using path probes, an innocent server without this type of bound would end up holding infinite amount of state.

> @@ -1069,6 +1069,17 @@ to cease using the connection IDs when requested can result in connection
 
+An endpoint MAY elect to limit the number of outstanding RETIRE_CONNECTION_ID
+frames to bound the necessary state. In order to allow a peer to retire all
+previously issued connection IDs, this limit SHOULD be at least the
+active_connection_id_limit. Within this limit, endpoints SHOULD prioritize
+sending frames which the peer is least likely to have received.
+
+Endpoints SHOULD NOT issue updates of the Retire Prior To field before receiving
+RETIRE_CONNECTION_ID frames for the previous update of Retire Prior To. Doing so
+may result in the peer being unwilling to immediately retire all the connection
+IDs, if retiring them would cause it to exceed its limit on outstanding
+RETIRE_CONNECTION_ID frames, as described above.

I think that the last sentence is inaccurate. As pointed out in my previous comments (https://github.com/quicwg/base-drafts/pull/3547#discussion_r399240482), an endpoint might receive a request to retire all the CIDs while it is already in the process of retiring some. In such case, *just* remembering `max_connection_id_limit` CIDs being retired is insufficient.

I think it would be better if we removed the recommendations from these two paragraphs, so that each of them becomes a problem statement, then in a new paragraph describe some of the measures that an endpoint SHOULD toke; e.g.:
* remember max_connection_id_limit CIDs, and also track those below RPT (as suggested by @ianswett in https://github.com/quicwg/base-drafts/pull/3547#discussion_r399248483)
* remember 2 * max_connection_id_limit

-- 
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/3547#pullrequestreview-383250846

---

From: Nick Banks <[email-masked]>
Date: Fri, 27 Mar 2020 17:41:12 -0700

It seems all the discussion of the problem is happening here and not on the issue, so I will propose my alternative solution here:

I know folks have been resistant to changing state based off receiving an acknowledgement of a packet/frame they sent, but what if we changed how the RetirePriorTo works, and just required the packet containing the frame to be acknowledged, instead of requiring the RCID to be sent? Would this solve the problems?

This would mean the receiver of the NCID shouldn't have to track any additional state beyond what it takes to acknowledge the packet (which they have to do anyways); it just immediately throws away the state. And the RCID packet would only be used when the endpoint that was given the CID chooses to retire 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/3547#issuecomment-605368930

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 27 Mar 2020 18:54:09 -0700

@nibanks I think this PR has been an attempt to clarify the "leaky" approach, that tries to clarify the amount of state that an endpoint should retain. There are indeed discussion of what that amount is, but I think it is essential to have such detailed discussion on the PR in order to determine clarify what a particular approach would look like.

To paraphrase, I think we are making process on this PR to describe a specific approach, if we are to consider something different, I think that discussion should happen on the issue (and emerge as a separate PR).

-- 
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/3547#issuecomment-605377813

---

From: ianswett <[email-masked]>
Date: Sat, 28 Mar 2020 06:30:40 -0700

SHOULD is fine with me.

-- 
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/3547#discussion_r399662819

---

From: Martin Thomson <[email-masked]>
Date: Sun, 29 Mar 2020 18:07:34 -0700

I'm still unconvinced that we need to be as precise as @kazuho, but the other suggestions he makes are spot on.

"this limit" here seems a bit non-specific

-- 
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/3547#pullrequestreview-383481026

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 10:31:32 -0700

I don't think this last sentence is necessary, so I'd be inclined towards removing 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/3547#pullrequestreview-384077488

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 10:37:47 -0700

```suggestion
An endpoint SHOULD elect to limit the number of outstanding RETIRE_CONNECTION_ID
```

-- 
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/3547#pullrequestreview-384082268

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 10:39:09 -0700

```suggestion
active_connection_id_limit.
```

-- 
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/3547#pullrequestreview-384083253

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 10:40:13 -0700

```suggestion
previously issued connection IDs, the limit on the number of outstanding RETIRE_CONNECTION_IDs SHOULD be at least the
```

-- 
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/3547#pullrequestreview-384084068

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 10:43:02 -0700

Suggestions added, PTAL

-- 
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/3547#pullrequestreview-384086225

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 11:05:09 -0700

66c76207669906a0937130fb5dda426e928e3462  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/3547/files/a62abddacec3745841f89e9b40a76c64811edac6..66c76207669906a0937130fb5dda426e928e3462

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 11:05:28 -0700

c4d24944d60ecfb5fbc4873ff3d7c9438ab1f536  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/3547/files/66c76207669906a0937130fb5dda426e928e3462..c4d24944d60ecfb5fbc4873ff3d7c9438ab1f536

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 11:05:42 -0700

f0b64a1301e329349ef1ba5fe4507d2288170782  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/3547/files/c4d24944d60ecfb5fbc4873ff3d7c9438ab1f536..f0b64a1301e329349ef1ba5fe4507d2288170782

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 11:06:07 -0700

I'm not sure which one you're referring to.

-- 
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/3547#discussion_r400390407

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 11:08:38 -0700

9ec4b0c1d4086cc17f849563ca55bf2932ca13f9  Resolved all reviews I know how to address

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/f0b64a1301e329349ef1ba5fe4507d2288170782..9ec4b0c1d4086cc17f849563ca55bf2932ca13f9

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 18:10:32 +0000 (UTC)

Thanks, I think this LGTM.

-- 
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/3547#pullrequestreview-384107175

---

From: Martin Thomson <[email-masked]>
Date: Mon, 30 Mar 2020 15:41:02 -0700

@martinthomson approved this pull request.

Thanks for working on this.  I have some suggestions to reduce text.  That reduction won't make things worse, I think.

> @@ -1069,6 +1069,19 @@ to cease using the connection IDs when requested can result in connection
 
+An endpoint SHOULD elect to limit the number of outstanding RETIRE_CONNECTION_ID

```suggestion
An endpoint SHOULD limit the number of outstanding RETIRE_CONNECTION_ID
```

> +RETIRE_CONNECTION_IDs SHOULD be at least the active_connection_id_limit.  If the
+number of outstanding RETIRE_CONNECTION_IDs exceeds twice the
+active_connection_id_limit, the connection MAY be closed with a connection error
+of type CONNECTION_ID_LIMIT_ERROR.

```suggestion
RETIRE_CONNECTION_IDs SHOULD be at least the active_connection_id_limit.
```
That sentence is unnecessary.  If a peer starts forcing extra retirement, then this is no different than any of the multitude of other pathological behaviours they might exhibit.  It doesn't need special text.

```suggestion
Endpoints SHOULD NOT issue updates of the Retire Prior To field before receiving
RETIRE_CONNECTION_ID frames for the previous update of Retire Prior To.
```
Rather than explain this and get the numbers right, just leave the recommendation.

-- 
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/3547#pullrequestreview-384281576

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 16:19:24 -0700

e10134ef4d335a0640286d1437afa16418655df3  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/3547/files/9ec4b0c1d4086cc17f849563ca55bf2932ca13f9..e10134ef4d335a0640286d1437afa16418655df3

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 16:20:41 -0700

4e3c1263433231a9e98365ec5fb75b541ef9de5e  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/3547/files/e10134ef4d335a0640286d1437afa16418655df3..4e3c1263433231a9e98365ec5fb75b541ef9de5e

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 16:22:16 -0700

Well @ianswett just added the ERROR bit, so I'd like his input on this.

-- 
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/3547#discussion_r400552413

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 16:23:54 -0700

46f1894b7ec811fd56f54d49a98d6cd32a3aec0e  text alignment

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/4e3c1263433231a9e98365ec5fb75b541ef9de5e..46f1894b7ec811fd56f54d49a98d6cd32a3aec0e

---

From: ianswett <[email-masked]>
Date: Mon, 30 Mar 2020 16:29:52 -0700

Assuming we move forward with this PR, this is what I intend our implementation to do.  I realize that 'A server's gotta do what a server's gotta do' but I'd prefer an explicit expectation for ease of debugging.

-- 
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/3547#discussion_r400554936

---

From: Martin Thomson <[email-masked]>
Date: Mon, 30 Mar 2020 17:16:07 -0700

In that case, I would prefer something a little more terse and less prescriptive.

> An endpoint MAY choose to treat having too many connection IDs in need of retirement as a connection error of type CONNECTION_ID_LIMIT_ERROR.

-- 
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/3547#discussion_r400569461

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 18:09:59 -0700

33548fd46adf3885542bbbe87ee2498bd2b92d75  MT's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3547/files/46f1894b7ec811fd56f54d49a98d6cd32a3aec0e..33548fd46adf3885542bbbe87ee2498bd2b92d75

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 18:11:05 -0700

Ok, trying to put this one in the ground.

I think the only remaining dispute is between @kazuho and @martinthomson on whether active_connection_id_limit is a sufficient number of RCIDs to remember.

-- 
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/3547#issuecomment-606336934

---

From: Marten Seemann <[email-masked]>
Date: Mon, 30 Mar 2020 18:37:26 -0700

Reading through this proposal, I fear that this would be very difficult to implement. If the ACK for a RETIRE_CONNECTION_ID frame is lost, and endpoint cannot safely send a NEW_CONNECTION_ID frame with a RetirePriorTo, since this would cause the peer to have more than `max_connection_id_limit` RETIRE_CONNECTION_ID frames in flight. In order to safely use RetirePriorTo, this endpoint then would have to track receipt of the ACK frame. I'd very much like to avoid the complexity associated with that.

-- 
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/3547#issuecomment-606345475

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 19:04:36 -0700

Thank you for working on the PR. I think this PR almost done.

The only complaint I have is the limit, which as I have previously stated. If people are going to use the minimum recommended value, it becomes risky to retire all CIDs at once. I'd prefer recommending something safer. Please see my suggestion below.

> @@ -1069,6 +1069,15 @@ to cease using the connection IDs when requested can result in connection
 
+An endpoint SHOULD limit the number of outstanding RETIRE_CONNECTION_ID frames
+to bound the necessary state. In order to allow a peer to retire all previously
+issued connection IDs, the limit on the number of outstanding
+RETIRE_CONNECTION_IDs SHOULD be at least the active_connection_id_limit. An

How about something like: _In order to allow a peer to retire all previously issued connection IDs while an endpoint waits for some RETIRE_CONNECTION_ID frames to be acknowledged, the limit on the number of outstanding RETIRE_CONNECTION_IDs SHOULD be at least the active_connection_id_limit plus the number of retirements that the endpoint might initiate at one time._

-- 
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/3547#pullrequestreview-384353335

---

From: Martin Thomson <[email-masked]>
Date: Mon, 30 Mar 2020 20:21:07 -0700

I don't think that is sufficient or necessary.

1. sufficiency: an adversary can send a long sequence of NEW_CONNECTION_ID frames with seq: N and rpt: N-1.  This is both entirely legitimate and completely outside the spirit of the protocol.

2. necessity: if you track min(unacknowledged RCID) and max(RPT) then you can track as few RCID frames as you like.  It might be nice if people used active_connection_id_limit*2, but that number only optimizes for a complete turnover of connection IDs every round trip.  It is an otherwise arbitrary number.  I don't think that we need to optimize for that case.

The only problem arises with this approach is when that gap widens more than you would like.  At which point you can throw down a CONNECTION_CLOSE and walk away.

-- 
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/3547#discussion_r400619617

---

From: martinduke <[email-masked]>
Date: Mon, 30 Mar 2020 21:04:39 -0700

It occurred to me that we might be talking about two entirely different ideas.

1) The "Maximum number of outstanding RCIDs" is a momentary restriction. I might send only 10 at a time, or whatever, but eventually I will send all of the RCIDs that the protocol required before any of these PRs.

2) I need only send N RCIDs at a time, and can assume all other (earlier) sequence numbers have been de facto retired.

If my limit is 10 and I get 100 NCIDs that increment RPT by 1 in rapid succession, option (1) would have us send all 100 RCIDs over 10 RTTs, assuming no loss. option (2) would have us send 10 and be done with it. Which are we describing 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/3547#discussion_r400630478

---

From: Martin Thomson <[email-masked]>
Date: Mon, 30 Mar 2020 21:13:50 -0700

I definitely mean option 1.  Which leads to eventual resolution.

Note that if we also take Ian's #3550, then none of these RCID frames are necessary.  You only use RCID for discretionary use (where you use a connection ID and find you no longer need it).  That's why I think that's a nice addition.

-- 
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/3547#discussion_r400632590

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 21:46:16 -0700

I agree that we do not need to care about sufficiency.

> if you track min(unacknowledged RCID) and max(RPT) then you can track as few RCID frames as you like. It might be nice if people used active_connection_id_limit*2, but that number only optimizes for a complete turnover of connection IDs every round trip.

This observation is what I disagree with. The intent of the change I proposed right above is to allow an endpoint to retire all the CID at once, without being concerned about the peer losing track of some sequence numbers carried by CID frames already inflight, or the peer considering the endpoint to be doing something malicious.

Assuming that an endpoint that retires CIDs track each of the RCID frame it sends, the safe number is _active_connection_id_limit_ (the number of CIDs that might be retired at once by RPT) plus _the number of retirements that that endpoint might initiate at once_.

To give a specific example, consider the following case.
* A client, with max_connection_id_limit = 2, is using CID<sub>0</sub> for the main path, and is probing a new path using CID<sub>1</sub>.
* Server sends NCID(CID<sub>2</sub>, RPT=2), NCID(CID<sub>3</sub>, RPT=2).
* Client receives these NCID frames, and assigns CID<sub>2</sub> to the main path, CID<sub>3</sub> to the path being probed. The client sends RCID(CID<sub>0</sub>), RCID(CID<sub>1</sub>).
* Before receiving ACK for the two RCID frames, the client decides to abandon the path being probed. It needs to send RCID(CID<sub>2</sub>).

At this point, the client is required to track the flight of RCID frames carrying *three* CIDs. And it has to make sure that RCID(CID<sub>2</sub>) reaches the peer, otherwise it would not receive a CID that substitutes CID<sub>2</sub>.

The recommendation in this PR now (i.e. "the limit on the number of outstanding RETIRE_CONNECTION_IDs SHOULD be at least the active_connection_id_limit") fails to cover this 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/3547#discussion_r400640521

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 21:54:39 -0700

@martinthomson 
> Note that if we also take Ian's #3550, then none of these RCID frames are necessary. You only use RCID for discretionary use (where you use a connection ID and find you no longer need it). That's w…

I think I agree with this. Status quo is just confusing, as it requires endpoints to send RCID frames for the retired CIDs they have received, but does not require the frames to be sent for the CIDs that they receive afterwards.

This means that from the viewpoint of the endpoint that requests retirement, it cannot assume that it would see RCID frames for the CIDs that it has requested retirement, but in most cases it would see them. This could turn into a trap in some implementations.

I agree that we should adopt #3550, so that we would never assume to see RCID for the CIDs that we request retirement.

-- 
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/3547#discussion_r400642662

---

From: Martin Thomson <[email-masked]>
Date: Mon, 30 Mar 2020 22:00:16 -0700

I see what you mean by the CID<sub>2</sub> example, but my counter would be that you wouldn't retire that connection ID because you were still busy working through the previous set of retirements.  (This assumes that you don't or can't start to retire CID<sub>2</sub> before being told to retire the others.)

The problem with discretionary retirement is that it won't be contiguous in the same way that Retire Prior To has to be, so you potentially have a patchwork of connection IDs that are variously valid, in use, unused, or retired.  You can use run-length encoding to reduce state somewhat, but there is no easy way to compress this state commitment.

But at least you can avoid the costs of retiring stuff that you are forced to retire.  That's all I was worried about.  Costs of discretionary activities are something people just have to factor in.

-- 
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/3547#discussion_r400644119

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 30 Mar 2020 22:36:04 -0700

> I see what you mean by the CID<sub>2</sub> example, but my counter would be that you wouldn't retire that connection ID because you were still busy working through the previous set of retirements.…

Yeah I think my view is that it is hard to implement such behavior without having additional state, because the peer can request an endpoint to retire CIDs. You can delay the start to retire CID<sub>2</sub>. But to do that, you need to have a delayed slot (i.e. state).

> Costs of discretionary activities are something people just have to factor in.

I agree. My complaint is that the recommendation of "at least active_connection_id_limit" is incorrect.

As we seem to be converging on adopting #3550, I've posted an example that does not involve the use of RPT in https://github.com/quicwg/base-drafts/issues/3509#issuecomment-606408197. I posted it on the issue as I think it is a part of the problem statement rather than the solution.

-- 
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/3547#discussion_r400653972

---

From: martinduke <[email-masked]>
Date: Tue, 31 Mar 2020 09:04:38 -0700

Alright, I implemented this PR this morning and it's quite lightweight. Moreover, it's fully interoperable with draft-27.

#3550 is a much more drastic change in behavior that will break interoperability. draft-27 NCID senders will wait for RCIDs that will never come. If we're going to make a breaking change I'd just as soon we go to #3553 and have a comprehensive solution that is less likely to end up with corner cases.

-- 
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/3547#discussion_r401033075

---

From: ianswett <[email-masked]>
Date: Tue, 31 Mar 2020 09:32:14 -0700

Q: At what threshold did you close the connection?  2 * active_connection_id_limit?  Or did you not close the connection and instead do something else?

-- 
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/3547#discussion_r401052048

---

From: martinduke <[email-masked]>
Date: Tue, 31 Mar 2020 09:39:56 -0700

So we're limiting in-flight to 2*(the limit), which for us is currently 4. I set a very large threshold to tear down the connection -- currently, if highest_rpt is 100 more than least_unacked. That makes it exceedingly unlikely weird ack loss cases are going to bring me down, but I can tear down the connection if it gets ridiculous. There's really no difference in state between it being 50, 100, or 1000 -- it just means the client will get application data a little slower.

-- 
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/3547#discussion_r401056926

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 31 Mar 2020 16:10:42 -0700

I finally caught up with this issue, and I think that implementer advice is my preferred way forward. To that end, I like this PR as the resolution.

-- 
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/3547#issuecomment-606932484

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 31 Mar 2020 16:10:58 -0700

@janaiyengar 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/3547#pullrequestreview-385166323

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 31 Mar 2020 16:46:01 -0700

@martinduke Thank you for sharing your experience.

Could you please clarify why you have two limits? Are you limiting the number of inflight RCID frames to 4, even though you retain state of up to 100 CIDs to be retired?

I'm curious, because that's not something I would expect as an outcome of this PR. My read of the text proposed in the PR has been that we assume that a RCID frame becomes "outstanding" when an endpoint decides to (or being instructed to) retire a CID.

IIUC, what we need to do is limit the amount of state that the endpoint retains. Number of RCID frames sent at one time does not really matter, as it is bound by the CWND size.

-- 
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/3547#discussion_r401277994

---

From: martinduke <[email-masked]>
Date: Tue, 31 Mar 2020 20:51:13 -0700

@kazuho. I initially read "outstanding" as meaning "intended to retire", just like you, but further up in the thread @martinthomson said that's not what he meant. And anyway, I soon saw that that interpretation is equally easy to implement and more compliant with the existing spec.

First, assume the endpoint never retires a CID voluntarily. I just have to store the active CIDs. The retired CIDs require only two state variables:
- highest retire prior to (highest_rpt)
- lowest RCID seqnum unacked (lowest_rua)

Each packet records the highest seqnum retired because it always retires everything below that, as you'll see below.

`process_ncid():
    if (retire_prior_to <= highest_rpt)
        quit;
    if (retire_prior_to > lowest_rua + 100)
       abort;
    if (highest_rpt == lowest_rua)
        highest_rpt = retire_prior_to
        send_rcid_pkt()
    else
        highest_rpt = retire_prior_to

send_rcid_pkt():
    limit = MIN(highest_rpt, lowest_rua + 2 * active_cid_limit)
    for i = lowest_rua : limit
        send_rcid(i)
    pkt.max_seq_num = limit

process_ack(pkt):
    lowest_rua = MAX(lowest_rua, pkt.max_seq_num + 1) 
    if (lowest_rua < highest_rpt) 
        send_rcid_pkt()`

If I have a backlog of CIDs to retire, I will clear 2 * active_cid_limit of them each RTT assuming no losses. This number is essentially arbitrary but should definitely fit in a single packet. 100 is an arbitrarily large number just in case the client is being malicious. Again, you don't want it to be so small that unlucky packet losses cause a connection error, but beyond that any number is fine.

If we are retiring CIDs voluntarily, we'll have to keep some extra state. We can put them in a special data structure that send_rcid_pkt also checks, but will definitely be finite as it is controlled locally. If I retire them in sequence, nothing stops me from increasing highest_rpt even though I never received it in an NCID frame, and therefore using the mechanism above.

***

The nice thing about this design is that we are fully compliant with the draft-27 language in every way, except that the RCID frames are not always sent *immediately*. #3553 is a cleaner design but this one has essentially no impact on the peer at all if it's implementing -27.

-- 
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/3547#discussion_r401340815

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 31 Mar 2020 21:55:46 -0700

@martinduke Thank you for the clarification. I now understand the intent. I think that we should adjust the text in this PR.

To mitigate the attacks being discussed, what we need to do is cap the amount of state used for retaining the CIDs that "are intended to be retired." While an endpoint may restrict the number of RCID frames being sent at one time, that is unrelated to the attack, and therefore I would advocate for removing that requirement (as it is an unnecessary design change).

Speaking of your implementation, what matters is 100. 4 (2 * active_connection_id_limit) is an arbitrary choice unrelated to the attack; my argument is that therefore we do not need to specify that in spec.

-- 
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/3547#discussion_r401355627

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 31 Mar 2020 22:24:55 -0700

PS. To avoid the confusion being discussed in the comments right above, I think my preference goes to having something like:

_An adversarial peer can request an endpoint to retain state for an unlimited amount of connection IDs being retired by withholding acknowledgements for the RETIRE_CONNECTION_ID frames, at the same time sending NEW_CONNECTION_ID frames with updated values in the Retire Prior To fields, or probing and discarding paths repeatedly._

_As a mitigation, endpoints SHOULD limit the amount of state that they retain for retiring the connection IDs. In order to allow a well-behaving peer to request retirement of all the previously issued connection IDs while an endpoint waits for some RETIRE_CONNECTION_ID frames to be acknowledged,  an endpoint SHOULD be capable of tracking at least the active_connection_id_limit plus the number of retirements that the endpoint might initiate at one time. When the amount of state required for tracking the connection IDs being retired exceeds that limit, an endpoint MAY close the connection with a CONNECTION_ID_LIMIT_ERROR._

-- 
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/3547#discussion_r401362644