Subject: [quicwg/base-drafts] Scope of disable_active_migration (#3670)

From: Mike Bishop <[email-masked]>
Date: Tue, 19 May 2020 15:43:48 -0700

Following on @janaiyengar&#39;s insight that disable_active_migration is a property of an address, not a peer, we simply need to say which address the TP applies to.

This PR fixes #3608 by scoping the definition of disable_active_migration to the handshake address only.  This means that existing clients remain fully compliant, albeit slightly more conservative than necessary.

While I find @kazuho&#39;s argument convincing that a server capable of using SPA is highly unlikely to be unable to handle migration, it would be trivial for a future extension to define a second transport parameter carrying two bits that disable active migration for those addresses as well.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Scope of disable_active_migration

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Tue, 19 May 2020 16:24:41 -0700

@ianswett approved this pull request.

> @@ -1229,9 +1229,11 @@ when a client's address changes.
   transport parameter to request that clients move connections to that dedicated
   address. Note that clients could choose not to use the preferred address.
 
-A server in a deployment that does not implement a solution to
-maintain connection continuity during connection migration
-SHOULD disallow migration using the disable_active_migration transport
+A server in a deployment that does not implement a solution to maintain
+connection continuity during connection migration SHOULD disallow migration

```suggestion
connection continuity when the client address changes SHOULD indicate migration is not supported
```

> @@ -1229,9 +1229,11 @@ when a client's address changes.
   transport parameter to request that clients move connections to that dedicated
   address. Note that clients could choose not to use the preferred address.
 
-A server in a deployment that does not implement a solution to
-maintain connection continuity during connection migration
-SHOULD disallow migration using the disable_active_migration transport
+A server in a deployment that does not implement a solution to maintain
+connection continuity during connection migration SHOULD disallow migration
+using the disable_active_migration transport parameter.  The
+disable_active_migration transport parameter does not prohibit subsequent

```suggestion
disable_active_migration transport parameter does not prohibit
```

-- 
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/3670#pullrequestreview-414887843

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 19 May 2020 16:39:21 -0700

@kazuho commented on this pull request.

LGTM modulo the point below.

> @@ -4872,11 +4874,11 @@ max_ack_delay (0x0b):
 disable_active_migration (0x0c):
 
 : The disable active migration transport parameter is included if the endpoint
-  does not support active connection migration ({{migration}}). Peers of an
-  endpoint that sets this transport parameter MUST NOT send any packets,
-  including probing packets ({{probing}}), from a local address or port other
-  than that used to perform the handshake.  This parameter is a zero-length
-  value.
+  does not support active connection migration ({{migration}}) on the address
+  being used during the handshake.  Peers of an endpoint that sets this
+  transport parameter MUST NOT send any packets, including probing packets
+  ({{probing}}), from a local address or port other than that used to perform
+  the handshake. This parameter is a zero-length value.

To me it seems that we need to also change this sentence (starting from "Peers ..."). We need to tighten the restriction to only when the client sends the packets to the original address of the server.

Maybe something like: _Peers of an endpoint that sets this transport parameter MUST NOT send any packets, including probing packets ({{probing}}) __to the address that was used to perform the handshake__, from a local address or port other than that used to perform the handshake._

-- 
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/3670#pullrequestreview-414895639

---

From: Martin Thomson <[email-masked]>
Date: Tue, 19 May 2020 16:40:19 -0700

@martinthomson commented on this pull request.

Good point (I made this same point verbally, but failed to notice the absence of a fix).

-- 
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/3670#discussion_r427660242

---

From: Mike Bishop <[email-masked]>
Date: Wed, 20 May 2020 13:18:51 -0700

@MikeBishop commented on this pull request.

I toyed with that and the existing sentence already has so many sub-phrases that it became too unwieldy.  I'll see if I can split it up any better.

-- 
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/3670#discussion_r428283381

---

From: Mike Bishop <[email-masked]>
Date: Wed, 20 May 2020 13:20:07 -0700

@MikeBishop pushed 1 commit.

0f18864a4e20cd70d291deb12e91417fca646a1b  Apply suggestions from code review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3670/files/6ab4369e359889e6431b5ef9287f946d346a9a91..0f18864a4e20cd70d291deb12e91417fca646a1b

---

From: Mike Bishop <[email-masked]>
Date: Wed, 20 May 2020 13:27:34 -0700

2f83dde0952fa7778d94d98160fa2f11904e41bb  Move probing aside

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3670/files/0f18864a4e20cd70d291deb12e91417fca646a1b..2f83dde0952fa7778d94d98160fa2f11904e41bb

---

From: Martin Thomson <[email-masked]>
Date: Wed, 20 May 2020 17:15:54 -0700

Seems fine.

> @@ -1229,10 +1229,11 @@ when a client's address changes.
 
-A server in a deployment that does not implement a solution to
-maintain connection continuity during connection migration
-SHOULD disallow migration using the disable_active_migration transport
-parameter.
+A server in a deployment that does not implement a solution to maintain
+connection continuity when the client address changes SHOULD indicate migration
+is not supported using the disable_active_migration transport parameter.  The
+disable_active_migration transport parameter does not prohibit connection
+migration after a client has acted on the preferred_address transport parameter.

```suggestion
migration after a client has acted on a preferred_address transport parameter.
```

> @@ -2150,15 +2151,15 @@ The design of QUIC relies on endpoints retaining a stable address for the
 duration of the handshake.  An endpoint MUST NOT initiate connection migration
 before the handshake is confirmed, as defined in section 4.1.2 of {{QUIC-TLS}}.
 
-An endpoint also MUST NOT send packets from a different local address, actively
-initiating migration, if the peer sent the disable_active_migration transport
-parameter during the handshake. An endpoint which has sent this transport
-parameter, but detects that a peer has nonetheless migrated to a different
-network MUST either drop the incoming packets on that path without generating a
-stateless reset or proceed with path validation and allow the peer to migrate.
-Generating a stateless reset or closing the connection would allow third parties
-in the network to cause connections to close by spoofing or otherwise
-manipulating observed traffic.
+If the peer sent the disable_active_migration transport parameter, an endpoint
+also MUST NOT send packets (including probing packets; see {{probing}}) from a
+different local address to the address the peer used during the handshake. An
+endpoint which has sent this transport parameter, but detects that a peer has

```suggestion
endpoint that has sent this transport parameter, but detects that a peer has
```

> @@ -2150,15 +2151,15 @@ The design of QUIC relies on endpoints retaining a stable address for the
 duration of the handshake.  An endpoint MUST NOT initiate connection migration
 before the handshake is confirmed, as defined in section 4.1.2 of {{QUIC-TLS}}.
 
-An endpoint also MUST NOT send packets from a different local address, actively
-initiating migration, if the peer sent the disable_active_migration transport
-parameter during the handshake. An endpoint which has sent this transport
-parameter, but detects that a peer has nonetheless migrated to a different
-network MUST either drop the incoming packets on that path without generating a
-stateless reset or proceed with path validation and allow the peer to migrate.
-Generating a stateless reset or closing the connection would allow third parties
-in the network to cause connections to close by spoofing or otherwise
-manipulating observed traffic.
+If the peer sent the disable_active_migration transport parameter, an endpoint
+also MUST NOT send packets (including probing packets; see {{probing}}) from a
+different local address to the address the peer used during the handshake. An
+endpoint which has sent this transport parameter, but detects that a peer has
+nonetheless migrated to a different network MUST either drop the incoming

```suggestion
nonetheless migrated to a different remote address MUST either drop the incoming
```

> +  the recipient MUST NOT send any packets to the address the peer used to
+  perform the handshake from a local address or port other than that used to
+  perform the handshake.  This parameter is a zero-length value.

I *might have a suggestion.
```suggestion
  an endpoint MUST NOT use a new local address when sending to the address
  that the peer used during the handshake.  This parameter is a zero-length value.
```

-- 
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/3670#pullrequestreview-415795983

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 20 May 2020 20:41:18 -0700

@kazuho approved this pull request.

Thank you for the changes.

I like @martinthomson's editorial improvements, but even without them, I think we are ready.

-- 
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/3670#pullrequestreview-415858516

---

From: David Schinazi <[email-masked]>
Date: Wed, 20 May 2020 21:43:35 -0700

@DavidSchinazi commented on this pull request.

> @@ -4872,11 +4873,11 @@ max_ack_delay (0x0b):
 
+  being used during the handshake.  When a peer sets this transport parameter,

I would reiterate the sentence from above here:
> The disable_active_migration transport parameter does not prohibit connection migration after a client has acted on the preferred_address transport parameter.

I fear that otherwise folks might forget that detail as it is very much not clear from the name of the parameter

-- 
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/3670#pullrequestreview-415874448

---

From: Mike Bishop <[email-masked]>
Date: Thu, 21 May 2020 09:55:56 -0700

9d27aab64095948c292d7823e33c929fbe605085  Apply suggestions from @martinthomson

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3670/files/2f83dde0952fa7778d94d98160fa2f11904e41bb..9d27aab64095948c292d7823e33c929fbe605085

---

From: Mike Bishop <[email-masked]>
Date: Thu, 21 May 2020 09:59:32 -0700

bd95712de002d2527bfabc9688d02c1b26e71cfd  Repetition can be useful and helpful

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

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 26 May 2020 15:43:28 -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/3670#pullrequestreview-418728306