Subject: [quicwg/base-drafts] Do I need to reset congestion info when the port changes? (#3842)

From: ekr <[email-masked]>
Date: Tue, 07 Jul 2020 18:34:26 -0700

> On confirming a peer's ownership of its new address, an endpoint MUST
immediately reset the congestion controller and round-trip time estimator for
the new path to initial values (see Sections A.3 and B.3 in {{QUIC-RECOVERY}})
unless it has knowledge that a previous send rate or round-trip time estimate is
valid for the new path.  For instance, an endpoint might infer that a change in
only the client's port number is indicative of a NAT rebinding, meaning that the
new path is likely to have similar bandwidth and round-trip time. 

This is kind of an odd formulation. What knowledge does the endpoint have in this case that we don't already have? Why not just make an explicit exception for port-only changes?

-- 
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/issues/3842

---

From: Martin Thomson <[email-masked]>
Date: Tue, 07 Jul 2020 23:27:42 -0700

...because that would create an explicit endorsement of the practice.  This text was somewhat carefully negotiated to avoid creating that sort of endorsement.

That said, I expect people to ignore port.  But I also expect some servers to be able to recognize the presence of a NAT that uses a pool of addresses and to treat all of those addresses as equivalent also.  And in IPv6 there are many cases where parts of the address are not relevant (/64 per host means that you might as well not bother with the last half in a lot of cases now).  So there is a risk in narrowly permitting port variations that certain types of address validation shortcuts are implicitly excluded.

-- 
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/issues/3842#issuecomment-655315277

---

From: ekr <[email-masked]>
Date: Wed, 08 Jul 2020 05:41:53 -0700

I see what you mean, but I think that this kind of hinting at the user without explicitly stating it does a disservice to the implementor, who then needs to figure it out for themselves. IMO we should explicitly state the exceptions that the WG believes are permissible and then add this escape hatch 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/issues/3842#issuecomment-655494462

---

From: ianswett <[email-masked]>
Date: Wed, 08 Jul 2020 14:49:19 -0700

I'm not sure I have a better suggestion than the existing text for @ekr concern, but I noticed a different small possible issue.

The process of validating the path is very likely to give you a RTT measurement for the path, so it might be worth tweaking the MUST to say "MUST ... and discard any round trip estimates associated with the old path" or just "MUST not use round trip time measurements from the old path on the new one."?  One could keep the estimates for the old path, as we've discussed, so I think saying you must reset or discard the information is a bit strong.

-- 
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/issues/3842#issuecomment-655775834

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 08 Jul 2020 18:10:49 -0700

This isn't meant to be a disservice to the implementer -- to an unaware implementer, the requirement is clear: reset the RTT and the congestion controller. This is meant to provide an escape hatch for those experimenting or for those who have other sources of information. For example, it might be possible that address changes within a /24 are also NAT rebinding (and there's evidence of this), so we want to allow endpoints to use this knowledge, but we don't want to specify that behavior.

I think a "MUST ... unless" captures 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/issues/3842#issuecomment-655836679

---

From: ekr <[email-masked]>
Date: Wed, 08 Jul 2020 19:31:30 -0700

I agree that we should leave room for the implementor do do more. However, what I am saying is that we already know that port changes (and perhaps lower-64 changes for v6) are likely to be rebinding and so we should tell people that rather than asking them to figure it out for themselves.

-- 
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/issues/3842#issuecomment-655859483

---

From: Christian Huitema <[email-masked]>
Date: Sun, 12 Jul 2020 08:48:54 -0700

I would be weary about a blanket statement that if a port (or address) change is merely rebinding, then transmission should go on with the previously estimated rate and window. The port or address change is a network event indicating that something changed. It used to be that NAT rebinding were strongly correlated with transmission hiatus -- the NAT would recycle bindings if it did not observe traffic for some time, typically about 30 seconds. It may also well be that the rebinding indicates a routing change on the path to the NAT. In both cases, the network condition did change and merely reusing the old estimates is a mistake.

-- 
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/issues/3842#issuecomment-657240091

---

From: ekr <[email-masked]>
Date: Sun, 12 Jul 2020 09:34:01 -0700

I'm not sure I agree with this analysis, but if that's the group consensus then we should prohibit this behavior, not have a MUST NOT with an unclear exception. My point is that what goes in this document should be what the WG thinks that generic internet servers ought to do.

-- 
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/issues/3842#issuecomment-657245916

---

From: Praveen Balasubramanian <[email-masked]>
Date: Sun, 12 Jul 2020 09:50:12 -0700

I agree resetting congestion state is better in this case. Since NAT rebinding only occurs on idle, resetting state seems in spirit of past recommendations around resetting congestion window on idle. I think we should remove the exception. 

-- 
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/issues/3842#issuecomment-657248003

---

From: Christian Huitema <[email-masked]>
Date: Sun, 12 Jul 2020 10:12:52 -0700

@pravb it would be nice if congestion response allowed better choices than "continue as nothing happened" and "go back to start as if nothing was known about the path". I think the endpoints have some knowledge about the past of the path, and also an indication that this knowledge could be wrong. How wrong might depend on timing, e.g., time since past measurements. I could see applying some Bayesian logic there. But this is clearly research.

-- 
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/issues/3842#issuecomment-657250754

---

From: Jana Iyengar <[email-masked]>
Date: Sun, 12 Jul 2020 17:22:39 -0700

I'd like to point out that this is unilateral endpoint behavior, and removing the exception does not mean that deployments won't implement different things here. This is at a minimum something that implementations will play around with, and the exception allows room for it because we believe it's safe to allow some discretion 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/issues/3842#issuecomment-657299158

---

From: Christian Huitema <[email-masked]>
Date: Sun, 12 Jul 2020 17:45:29 -0700

@janaiyengar we might debate how safe it is to keep sending as if nothing changed. There is some risk of creating a spike of congestion on the new path if the assessment was wrong. On the other hand, doing that when "just the port changed" will work most of the time, especially if the congestion controller already shrinks the window after the link goes idle, which many do. So in the hand I agree that we are probably better off just keeping the existing 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/issues/3842#issuecomment-657303214

---

From: ianswett <[email-masked]>
Date: Mon, 13 Jul 2020 15:13:59 +0000 (UTC)

I'm still happy with the existing text, FWIW.  I think it captures the important points.

-- 
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/issues/3842#issuecomment-657619474

---

From: ianswett <[email-masked]>
Date: Tue, 14 Jul 2020 11:13:13 -0700

@nharper pointed me to this text, which could add confustion on this point.

> Not all changes of peer address are intentional, or active, migrations. The peer could experience NAT rebinding: a change of address due to a middlebox, usually a NAT, allocating a new outgoing port or even a new outgoing IP address for a flow. An endpoint MUST perform path validation ({{migrate-validate}}) if it detects any change to a peer's address, unless it has previously validated that address.
https://github.com/quicwg/base-drafts/blob/master/draft-ietf-quic-transport.md#connection-migration-migration

My current reading is that you don't have to reset the congestion controller if you're pretty sure it's a NAT rebind, but you still need to perform path validation.  Is that correct?

-- 
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/issues/3842#issuecomment-658332505

---

From: ianswett <[email-masked]>
Date: Wed, 15 Jul 2020 12:37:06 -0700

Follow-up Q: Do you need to wait for a PATH_RESPONSE before sending on the new path(which delays application data 1RTT), or can you concurrently validate the path while also starting to use 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/issues/3842#issuecomment-658966195

---

From: Christian Huitema <[email-masked]>
Date: Wed, 15 Jul 2020 13:01:32 -0700

There is a security risk if a client could convince a server to send data to an arbitrary address. The challenge mitigates that risk.

-- 
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/issues/3842#issuecomment-658978304

---

From: Nick Harper <[email-masked]>
Date: Wed, 15 Jul 2020 13:15:14 -0700

Could an amplification attack limit be imposed to allow some usage of the new path before validating 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/issues/3842#issuecomment-658985302

---

From: David Schinazi <[email-masked]>
Date: Wed, 15 Jul 2020 13:34:04 -0700

We're in the process of implementing this, and as currently specified the draft adds a round trip delay compared to what we did in GoogleQUIC. Using a middle ground with an amplification limit as suggested by @nharper might be required to ensure reasonable performance.

-- 
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/issues/3842#issuecomment-658994557

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 16 Jul 2020 19:10:07 -0700

The amplification limit can probably be used, but it's likely going to not be good enough, since you probably just received a request after a long pause and not a full sized packet. 

@DavidSchinazi: Does GQUIC have this amplification protection at the moment?

-- 
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/issues/3842#issuecomment-659789324

---

From: David Schinazi <[email-masked]>
Date: Thu, 16 Jul 2020 19:52:04 -0700

@janaiyengar currently Connection Migration on gQUIC does not use an amplification limit nor path validation. We haven't seen that kind of attack at scale. I think we should add those protections when we implement the IETF take on Connection Migration, but we'll need to make sure the performance hit isn't too high. If we go with the amplification factor then we can have the client send a bunch of padding to make it 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/issues/3842#issuecomment-659804919

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 16 Jul 2020 23:36:09 -0700

It'd be great if clients could send a full-sized datagram after quiescence.

But regardless, I'm not too worried about servers going above 3x in this case, because only an on-path attacker can use this behavior to mount a reflection attack.

-- 
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/issues/3842#issuecomment-659894080

---

From: ianswett <[email-masked]>
Date: Fri, 17 Jul 2020 09:07:23 -0700

In section 9.3 "Responding to Connection MIgration", we allow sending the initial window per estimated round-trip time.  There's no mention of amplification limit I can see.

https://tools.ietf.org/html/draft-ietf-quic-transport-29#section-9.3
An endpoint MAY send data to an unvalidated peer address, but it MUST
   protect against potential attacks as described in Section 9.3.1 and
   Section 9.3.2. 
In 9.3.1: Until a peer's address is deemed valid, an endpoint MUST
   limit the rate at which it sends data to this address.  The endpoint
   MUST NOT send more than a minimum congestion window's worth of data
   per estimated round-trip time (kMinimumWindow, as defined in
   [QUIC-RECOVERY]).
   ....
   If an endpoint skips validation of a peer address as described in
   Section 9.3, it does not need to limit its sending rate.

-- 
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/issues/3842#issuecomment-660192315

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 17 Jul 2020 19:22:59 -0700

Ah, yes, thank you for finding that text. I think we did that because of
what Kazuho said -- that only an on-path attacker can mount this reflection
attack.

@nharper @dschinazi: Given the text in 9.3, is there a problem anymore?

On Fri, Jul 17, 2020 at 9:07 AM ianswett <[email-masked]> wrote:

> In section 9.3 "Responding to Connection MIgration", we allow sending the
> initial window per estimated round-trip time. There's no mention of
> amplification limit I can see.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/quicwg/base-drafts/issues/3842#issuecomment-660192315>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACUOBVGYQEHFDE2Z3LJ7M6DR4BZLFANCNFSM4OUAV2OA>
> .
>

-- 
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/issues/3842#issuecomment-660409262

---

From: David Schinazi <[email-masked]>
Date: Sat, 18 Jul 2020 11:36:27 -0700

@janaiyengar we'll have to run experiments to confirm that. Since kMinimumWindow is defined as 10 packets, that might be enough for most requests. If it doesn't turn out to be enough, we may need to change the limit to `max(kMinimumWindow, amplification_factor * num_packets_received_on_unvalidated_path)` - but we'll collect data before we propose a change like 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/issues/3842#issuecomment-660523251

---

From: Lars Eggert <[email-masked]>
Date: Mon, 20 Jul 2020 05:28:11 -0700

Proposed resolution is to close with no action, consensus to be confirmed

-- 
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/issues/3842#issuecomment-660996017

---

From: ianswett <[email-masked]>
Date: Mon, 20 Jul 2020 07:41:17 -0700

I think I'm ok with close with no action from a design perspective, but I find having this text in multiple locations and having similar, but different statements quite confusing.

I'd rather you label this editorial and we try to make the text a bit clearer and/or better organized.

-- 
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/issues/3842#issuecomment-661082229

---

From: Lars Eggert <[email-masked]>
Date: Mon, 20 Jul 2020 07:45:26 -0700

@LPardue and my read of the discussion was that a solution that was going to address the issue would have to be a design change. Hence it's a design issue, and the resolution is "no change". 

We can definitely spin out some related editorial changes into another issue though.

-- 
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/issues/3842#issuecomment-661084723

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 20 Jul 2020 19:41:37 -0700

@ianswett -- what text organization do you think might help?

-- 
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/issues/3842#issuecomment-661584893

---

From: Lars Eggert <[email-masked]>
Date: Mon, 20 Jul 2020 23:48:03 -0700

OK, I'll label this as ready, and you can spin a new editorial issue for any such changes.

-- 
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/issues/3842#issuecomment-661669155

---

From: ianswett <[email-masked]>
Date: Tue, 21 Jul 2020 14:22:15 -0700

@janaiyengar a state transition diagram would be very nice I think, similar to streams.  And maybe some reorg, but I need to re-read the relevant sections at once to understand how that could 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/issues/3842#issuecomment-662113167

---

From: Lars Eggert <[email-masked]>
Date: Wed, 22 Jul 2020 00:42:47 -0700

More discussion [here](https://mailarchive.ietf.org/arch/msg/quic/pwsYrpJwVNX3TrvvA66G6JoPHdY/)

-- 
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/issues/3842#issuecomment-662297786

---

From: Lars Eggert <[email-masked]>
Date: Wed, 22 Jul 2020 22:43:35 -0700

FYI, I've written up Ted's suggestion from the mailing list as #3945 (with some word-smithing)

-- 
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/issues/3842#issuecomment-662828572

---

From: Lucas Pardue <[email-masked]>
Date: Wed, 29 Jul 2020 05:57:21 -0700

Due to the discussion during the consensus call, we'll run this through the Late-stage process again.

-- 
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/issues/3842#issuecomment-665647448

---

From: Martin Thomson <[email-masked]>
Date: Wed, 29 Jul 2020 18:58:49 -0700

We should mark it proposal-ready though, right?

-- 
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/issues/3842#issuecomment-666034798