Subject: Re: [quicwg/base-drafts] Clarify text around preferred address (#3589)

From: ekr <[email-masked]>
Date: Fri, 08 May 2020 13:46:54 -0700

@ekr requested changes on this pull request.

> @@ -1084,10 +1084,10 @@ Sending a RETIRE_CONNECTION_ID frame indicates that the connection ID will not
 be used again and requests that the peer replace it with a new connection ID
 using a NEW_CONNECTION_ID frame.
 
-As discussed in {{migration-linkability}}, endpoints limit the use of a
-connection ID to a single network path where possible. Endpoints SHOULD retire
-connection IDs when no longer actively using the network path on which the
-connection ID was used.
+As discussed in {{migration-linkability}}, endpoints MUST limit the use of a
+connection ID to packets sent from a single local address.  Endpoints SHOULD
+retire connection IDs when they are no longer actively using the network path on
+which the connection ID was used.

This text is now contradictory, because I can send from A -> B and A->C but the subsequent text now refers to "the network path"

> @@ -2277,11 +2278,21 @@ linked by any other entity.
 At any time, endpoints MAY change the Destination Connection ID they send to a
 value that has not been used on another path.
 
-An endpoint MUST use a new connection ID if it initiates connection migration as
-described in {{initiating-migration}} or probes a new network path as described
-in {{probing}}.  An endpoint MUST use a new connection ID in response to a
-change in the address of a peer if the packet with the new peer address uses an
-active connection ID that has not been previously used by the peer.
+An endpoint MUST NOT reuse a connection ID when sending from more than one local
+address, for example when initiating connection migration as described in
+{{initiating-migration}} or when probing a new network path as described in
+{{probing}}.
+
+Similarly, an endpoint MUST NOT reuse a connection ID when sending to more than
+one destination address.  However, if an endpoint receives packets from a new
+source address with the same destination connection ID, it MAY continue to use
+the current connection ID with the new address.

This text is really confusing. We have a direct prohibition followed by an unexplained exception. Can we provide some reasoning 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/3589#pullrequestreview-408514847

---

From: Martin Thomson <[email-masked]>
Date: Sun, 10 May 2020 17:04:55 -0700

@martinthomson commented on this pull request.

Perhaps the answer here is to say "endpoints limit the use of a connection ID to a single network path".  The point about NAT rebinding/inadvertent migration here is not relevant and somewhat distracting.  The reintroduction of the "MUST" is problematic as this implies a canonical requirement, but you can't really make that without adding the necessary context about NAT rebinding.

How about:

> However, if a peer experiences NAT rebinding, an endpoint could receive packets from its peer using a new source address with the same destination connection ID.  If a peer is first to use a connection ID on a new network path and that connection ID was originally used on another network path, an endpoint MAY also use a connection ID that it used on the original network path.

That is much more wordy, but it might be clearer.  The point is to say that you don't reuse a connection ID unless your peer reuses first.

There is a wrinkle here in the case that an endpoint has multiple valid addresses (think preferred address).  In that case, we don't want to allow reuse as that implies a bunch of other things.  The fix there might be to add:

> This exception that allows the use of a connection ID on multiple network paths only applies when the remote address changes and the local address remains constant.

-- 
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/3589#discussion_r422718955

---

From: ekr <[email-masked]>
Date: Mon, 11 May 2020 04:59:08 -0700

@ekr commented on this pull request.

Perhaps the problem is the use of "network path". As I understand it, the requirement here is that from your perspective the 5-tuple be the same, right? The issue is that with NAT your perspective and others may different. So  perhaps "MUST limit the use of a connection ID to a single pair of remote/local address"

Do we actually have to make this exception? I haven't been tracking all of the conversations, but why not just have endpoints use a new CID in this case.

In general an endpoint MUST limit the use of a connection ID to a single pair of remote and local addresses. This prevents linkage between network paths. However, if an endpoint may experiences unexpected NAT rebinding, that endpoint's peer may receive a packet from a new remote address but reusing an existing connection ID, thus apparently violating this rule linking those paths. [TODO: Something about how you have to accept this?]
As an exception to the rule above, if an endpoint sees a given connection ID reused from a new remote address, it MAY also reuse its corresponding connection ID when sending to that address, as long as it sends from the same local address.

-- 
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/3589#discussion_r422984471

---

From: Eric Kinnear <[email-masked]>
Date: Mon, 11 May 2020 10:35:53 -0700

@erickinnear commented on this pull request.

> Perhaps the problem is the use of "network path".

I would agree, I think we've been trying to steer away from "network path" in favor of things from the perspective of each endpoint.

> The point about NAT rebinding/inadvertent migration here is not relevant and somewhat distracting.

The NAT rebinding is something that's been covered elsewhere already, I believe, but every review that we've had of this text has resulted in someone raising their hand to say "don't forget about NAT rebinding here", so have kept text in to settle that.

> The issue is that with NAT your perspective and others may different.

When we distilled this down, we went from the perspective of the requirements that are placed on each endpoint to help cope with this.

> This text is now contradictory, because I can send from A -> B and A->C

That shouldn't be the case: 
```
An endpoint MUST NOT reuse a connection ID when sending from more than one local
address, ...
```
```
Similarly, an endpoint MUST NOT reuse a connection ID when sending to more than
one destination address.
```

I think we can change the text above to not mention a "network path" and instead talk about the destination address, since that's what we really mean in 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/3589#discussion_r423205675

---

From: Eric Kinnear <[email-masked]>
Date: Mon, 11 May 2020 12:48:08 -0700

I like the framing of the last sentence you have there, although it's interesting to think through the question about if we really need an exception here. 

In theory, the problem with changing for this is that a NAT rebinding can cause us to "burn" a CID, making it so that instead of having 2 linked CIDs, we now have 3 linked CIDs. As long as we retire the old one (which is already in the text) once we change to the new one, we should be able to keep a steady supply of those around, although it would mean that someone in the middle of the network could fiddle with things to keep us generating and exchanging more and more CIDs. 

So, the idea is that if someone on the network changes without your knowledge, we've already linked those two 5-tuples by the fact that you've got the same CID in one direction, there's no harm in continuing to keep using that CID in the other direction. 

It does seem better to keep doing that, rather than opening ourselves up to letting someone on the network arbitrarily churn through CIDs.

This would mean the text becomes much like what you have in the TODO above, essentially we're saying that the _sender_ of the packet MUST follow these restrictions, but if you see that your peer did not, then \<the text you have above\>.

-- 
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/3589#discussion_r423278139

---

From: Martin Thomson <[email-masked]>
Date: Mon, 11 May 2020 15:49:33 -0700

I am comfortable with either outcome here.  @erickinnear, do you want to try to integrate these comments in something approximating the shape you describe?

-- 
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/3589#discussion_r423363613

---

From: Eric Kinnear <[email-masked]>
Date: Mon, 11 May 2020 16:19:39 -0700

Yup! Working on an update for these this afternoon.

-- 
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/3589#discussion_r423373575

---

From: Eric Kinnear <[email-masked]>
Date: Mon, 11 May 2020 18:26:47 -0700

@erickinnear pushed 1 commit.

f7a9c3b3bb78485bc65476b894306899f70d8205  Refer to addresses instead of network paths

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

---

From: Eric Kinnear <[email-masked]>
Date: Mon, 11 May 2020 18:28:01 -0700

Updated to fold this in, also note that much of this explanation (and further requirements on behavior in the event of a NAT rebinding, such as potentially needing to validate the new path) is present in the immediately following paragraph: 
```
These requirements regarding connection ID reuse apply only to the sending of
packets, as unintentional changes in path without a change in connection ID are
possible.  For example, after a period of network inactivity, NAT rebinding
might cause packets to be sent on a new path when the client resumes sending.
An endpoint responds to such an event as described in {{migration-response}}.
```
Updated in f7a9c3b

-- 
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/3589#discussion_r423410755

---

From: Eric Kinnear <[email-masked]>
Date: Mon, 11 May 2020 18:29:18 -0700

Updated the text in f7a9c3b to refer to being done with either your local address (migrated to a new address) or your destination address (only relevant for preferred_address and someday maybe server migration). Whenever you're done with an address on either end, you should probably retire the CID that you used with that address.

-- 
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/3589#discussion_r423411147

---

From: Martin Thomson <[email-masked]>
Date: Mon, 11 May 2020 18:38:29 -0700

Thanks for doing that Eric.  It looks good 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/3589#discussion_r423413570

---

From: ianswett <[email-masked]>
Date: Tue, 12 May 2020 15:19:30 -0700

@ianswett commented on this pull request.

I think the new MUST could be adjusted to be less confusing.

> @@ -1084,9 +1084,10 @@ Sending a RETIRE_CONNECTION_ID frame indicates that the connection ID will not

There's an exception to this MUST down below that's fairly clear about it's purpose.  Given that, this new MUST seems contradictory unless some caveats are added?

-- 
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/3589#pullrequestreview-410445521

---

From: Martin Thomson <[email-masked]>
Date: Tue, 12 May 2020 16:22:35 -0700

```suggestion
As discussed in {{migration-linkability}}, endpoints limit the use of a
```
There is an exception in the referenced text that isn't repeated here.  We don't need to repeat that, so it is best to keep this as advisory text.

-- 
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/3589#discussion_r424088541

---

From: Eric Kinnear <[email-masked]>
Date: Wed, 13 May 2020 09:17:34 -0700

a7101944f1dec2dd4d4c890e56f3029e1648545d  Remove duplicate MUST

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

---

From: ianswett <[email-masked]>
Date: Wed, 13 May 2020 12:30:55 -0700

@ianswett approved this pull request.

Thanks for the change.

-- 
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/3589#pullrequestreview-411235113

---

From: Martin Thomson <[email-masked]>
Date: Wed, 13 May 2020 18:14:49 -0700

@ekr, you asked that this be blocked by requesting changes.  That means that you volunteered to re-review too :)  Can you take another look to verify that the recent fixes address your concerns?

-- 
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/3589#issuecomment-628328222

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 13 May 2020 23:04:30 -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/3589#pullrequestreview-411487159

---

From: Eric Kinnear <[email-masked]>
Date: Thu, 14 May 2020 14:33:18 -0700

@erickinnear pushed 2 commits.

ca0decd66b88576741494c0fc077d2b78424e279  Merge branch 'master' into ek/preferred_address
77b06fcadb8d708a9fb932930ff7742a640b7e0a  Update whitespace for consistency

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3589/files/a7101944f1dec2dd4d4c890e56f3029e1648545d..77b06fcadb8d708a9fb932930ff7742a640b7e0a

---

From: ekr <[email-masked]>
Date: Fri, 15 May 2020 13:25:32 -0700

> +Similarly, an endpoint MUST NOT reuse a connection ID when sending to more than
+one destination address.  Due to network changes outside the control of its
+peer, an endpoint might receive packets from a new source address with the same
+destination connection ID, in which case it MAY continue to use the current
+connection ID with the new remote address while still sending from the same
+local address.

```suggestion
Due to network changes outside the control of its
peer, an endpoint might receive packets from a new source address with the same
destination connection ID, in which case it MAY continue to use the current
connection ID with the new remote address while still sending from the same
local address. In all other cases, an endpoint MUST NOT reuse a connection ID when sending to more than
one destination address. 
```

-- 
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/3589#pullrequestreview-412944745

---

From: ekr <[email-masked]>
Date: Fri, 15 May 2020 13:25:51 -0700

I suggested some clearer language, but this looks fine.

-- 
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/3589#pullrequestreview-412944954

---

From: Eric Kinnear <[email-masked]>
Date: Fri, 15 May 2020 13:29:30 -0700

I don't dislike the flow once flipped, but the first sentences of this and the preceding paragraph are trying to be very clearly the main requirement, with the subsequent items explaining motivation and any exceptions.

This is in response to the original issue we're trying to address, which is that all of these requirements were in place but got lost in the rest of the text, so the idea was to highlight the main requirements in a single place.

Thoughts?

-- 
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/3589#discussion_r426031726

---

From: Martin Thomson <[email-masked]>
Date: Sun, 17 May 2020 16:56:32 -0700

I understand the spirit of the change, which is to make the primary requirement properly conditional, but it does so by burying that requirement.  I'm comfortable with the original.  Happy to iterate again later.

-- 
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/3589#discussion_r426319693

---

From: Martin Thomson <[email-masked]>
Date: Sun, 17 May 2020 16:57:12 -0700

Merged #3589 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/3589#event-3345068654