Subject: [quicwg/base-drafts] Don't Change Source CID After Retry (#1491)

From: Nick Banks <[email-masked]>
Date: Wed, 27 Jun 2018 10:12:33 -0700

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

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

-- Commit Summary --

  * Don't Change Source CID After Retry

-- File Changes --

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

-- Patch Links --

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

---

From: MikkelFJ <[email-masked]>
Date: Wed, 27 Jun 2018 10:38:50 -0700

But doesn't this work against fixing loops in retry? Though that hasn't been worked out yet.

-- 
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/1491#issuecomment-400769114

---

From: Nick Banks <[email-masked]>
Date: Wed, 27 Jun 2018 17:40:12 +0000 (UTC)

I thought he retry loop issue was related changing **destination** connection ID, not source.

-- 
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/1491#issuecomment-400769514

---

From: Mike Bishop <[email-masked]>
Date: Wed, 27 Jun 2018 10:49:13 -0700

Agreed.  I thought there was text to the effect that changing the SCID during the handshake was effectively starting a fresh connection attempt, but I don't see it in the draft.  Maybe still in one of the PRs, or maybe that text got lost at some point.

-- 
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/1491#issuecomment-400772158

---

From: MikkelFJ <[email-masked]>
Date: Wed, 27 Jun 2018 10:49:21 -0700

Not all in fresh memory but goes like:
Clients sends original random DCID to server.
Server sends retry back to client with a servers SCID
Client uses retry SCID as its new DCID towards the server.
Client uses whatever SCID it happens to prefer, 0 lenght or otherwise, call this Client-SCID.

If the Client-SCID does not change an new random retries happen, the client might be confused. If the server changes Client-SCID it can distinguish responses. But there is a long dicussion that never got fully resolved. Changing Client-SCID is just one option. Reflecting a separate NONCE per retry is probably either better or also needed.

https://github.com/quicwg/base-drafts/issues/1451

-- 
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/1491#issuecomment-400772206

---

From: Martin Thomson <[email-masked]>
Date: Wed, 27 Jun 2018 14:16:35 -0700

I think that we need MUST NOT here.  Tokens provided in Retry can be validated strictly, and allowing a client to change its source connection ID would make that harder.

-- 
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/1491#issuecomment-400831760

---

From: ianswett <[email-masked]>
Date: Wed, 27 Jun 2018 14:19:40 -0700

ianswett commented on this pull request.

Agreed, MUST NOT

> @@ -633,7 +633,11 @@ If the client received a Retry packet from the server and sends an Initial
 packet in response, then it sets the Destination Connection ID to the value from
 the Source Connection ID in the Retry packet. Changing Destination Connection ID
 also results in a change to the keys used to protect the Initial packet. It also
-sets the Token field to the token provided in the Retry.
+sets the Token field to the token provided in the Retry. Additionally, the
+client SHOULD NOT change the Source Connection ID because the server may include
+the connection ID as part of its token validation logic. If the client changes
+the Source Connection ID in this case, it would likely trigger a new Retry to be

nit: would likely -> could

> @@ -633,7 +633,11 @@ If the client received a Retry packet from the server and sends an Initial
 packet in response, then it sets the Destination Connection ID to the value from
 the Source Connection ID in the Retry packet. Changing Destination Connection ID
 also results in a change to the keys used to protect the Initial packet. It also
-sets the Token field to the token provided in the Retry.
+sets the Token field to the token provided in the Retry. Additionally, the
+client SHOULD NOT change the Source Connection ID because the server may include

MUST NOT

-- 
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/1491#pullrequestreview-132613045

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 27 Jun 2018 14:57:20 -0700

@MikeBishop 
> I thought there was text to the effect that changing the SCID during the handshake was effectively starting a fresh connection attempt, but I don't see it in the draft. 

FWIW, my understanding is that the existing text is (emphasis mine):
> During the handshake, an endpoint might receive multiple packets with the long header, and thus be given multiple opportunities to update the Destination Connection ID it sends. A client MUST only change the value it sends in the Destination Connection ID in response to the first packet of each type it receives from the server (Retry or Initial); **a server MUST set its value based on the Initial packet. Any additional changes are not permitted; if subsequent packets of those types include a different Source Connection ID, they MUST be discarded.** This avoids problems that might arise from stateless processing of multiple Initial packets producing different connection IDs.
> [4.7 Connection ID Encoding](https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#connection-id-encoding)

Anyways I agree that this is written from server's viewpoint, and I agree that we should have a "MUST NOT" for the client.

-- 
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/1491#issuecomment-400842078

---

From: Nick Banks <[email-masked]>
Date: Wed, 27 Jun 2018 15:33:46 -0700

@nibanks pushed 1 commit.

6d48169  should -> must

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1491/files/93b705e92da517221c50d4e4ee4a7d128c9e5faa..6d48169b404bd0ab239c82f9f735ec8faf2d9ebf

---

From: Martin Thomson <[email-masked]>
Date: Wed, 27 Jun 2018 16:34:14 -0700

@martinthomson pushed 1 commit.

6a68e07  Removing extra text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1491/files/6d48169b404bd0ab239c82f9f735ec8faf2d9ebf..6a68e074e84f04e70a186294a3e468c3c98cef5b

---

From: Martin Thomson <[email-masked]>
Date: Wed, 27 Jun 2018 16:35:27 -0700

Merged #1491.

-- 
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/1491#event-1705140302

---

From: Martin Thomson <[email-masked]>
Date: Wed, 27 Jun 2018 16:35:30 -0700

Thanks Nick.  This will make -13.  I consider it editorial (based on the text @kazuho cites, which should have been generic.)  There are a bunch more changes we need to consider with Retry, but this at least clears one of those up.

-- 
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/1491#issuecomment-400863237