Subject: [quicwg/base-drafts] Initial secrets do not change after Retry (#2878)

From: ianswett <[email-masked]>
Date: Mon, 08 Jul 2019 20:38:18 -0700

Alternate PR to #2870 

Fixes #2823 
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Update draft-ietf-quic-tls.md

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 08 Jul 2019 20:41:57 -0700

martinthomson commented on this pull request.

I actually like this, but the intent of #2870 was to document the status quo.  If we do this (we should), then it probably also makes sense to describe the consequences for a stateless Retry.  That is not 100% obvious and probably needs a little bit of guidance.

> @@ -804,6 +803,9 @@ modifying the contents of packets from future versions.
 The HKDF-Expand-Label function defined in TLS 1.3 MUST be used for Initial
 packets even where the TLS versions offered do not include TLS 1.3.
 
+The secrets used for protecting Initial packets do not change during the
+connection, even after receiving a Retry.
+
 {{test-vectors-initial}} contains test vectors for the initial packet
 encryption.

Just down here (vv) there is a note that needs to be removed also.

-- 
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/2878#pullrequestreview-259255374

---

From: ianswett <[email-masked]>
Date: Mon, 08 Jul 2019 20:47:50 -0700

@ianswett pushed 1 commit.

1ce0a2e693321014ad5b0d4a0b160f8ad613e1f7  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/2878/files/55e3184c27aa2424924ab0f9548a15d6e42edb0e..1ce0a2e693321014ad5b0d4a0b160f8ad613e1f7

---

From: Marten Seemann <[email-masked]>
Date: Mon, 08 Jul 2019 22:30:01 -0700

marten-seemann requested changes on this pull request.

At the NY interim, we agreed that a design goal of QUIC's Retry mechanism is that a MITM is not able to perform a Retry. 
By removing the original_connection_id transport parameter, this PR reintroduces that option. The attack works as follows:
1. The MITM captures the Initial packet, and sends a Retry packet. 
2. The client generates a second Initial as a response. This packet will us the DCID provided by the Retry (which is different from the DCID use for the first Initial).
3. The MITM strips out the token, and re-protects the packet with an Initial key derived from the DCID of the packet (it might even choose to change the DCID before forwarding the packet to the server).
4. Because the key used to protect the packet was derived from the DCID, the server is able to unprotect the packet. It replies with an Initial packet.
5. Since Initials are able to change the DCID, the client will accept the Initial packet and change the DCID again (if MITM or server changed the DCID, that is).

-- 
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/2878#pullrequestreview-259275439

---

From: Martin Thomson <[email-masked]>
Date: Mon, 08 Jul 2019 22:34:01 -0700

I don't think that's a concern, because the server is still required to echo the ODCID and the client should check it.  That change is entered into the handshake transcript, so it can't be spoofed.

-- 
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/2878#issuecomment-509495031

---

From: Marten Seemann <[email-masked]>
Date: Mon, 08 Jul 2019 22:34:52 -0700

@martinthomson This PR removes the transport parameter. Nothing is entered into the handshake transcript.

-- 
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/2878#issuecomment-509495225

---

From: Martin Thomson <[email-masked]>
Date: Mon, 08 Jul 2019 22:39:07 -0700

martinthomson requested changes on this pull request.

Oops. I completely missed that somehow. I agree with Marten. Keep the transport parameter and I am ok with this, but without, it is badly exposed.

-- 
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/2878#pullrequestreview-259277409

---

From: Martin Thomson <[email-masked]>
Date: Mon, 08 Jul 2019 22:40:52 -0700

Oh now I see why. Ian removed the transport parameter after I reviewed it. In an update labelled 
"Update draft-ietf-quic-transport.md".

-- 
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/2878#issuecomment-509496490

---

From: ianswett <[email-masked]>
Date: Tue, 09 Jul 2019 09:52:51 -0700

Thanks for walking through that attack Marten, I was under the impression it was no longer necessary, but I'll revert that change and add the TP back.

-- 
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/2878#issuecomment-509721572

---

From: ianswett <[email-masked]>
Date: Tue, 09 Jul 2019 09:57:30 -0700

b40c482826276b4a9e610c71d8428256c515ebfb  Revert changes to transport

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2878/files/1ce0a2e693321014ad5b0d4a0b160f8ad613e1f7..b40c482826276b4a9e610c71d8428256c515ebfb

---

From: ianswett <[email-masked]>
Date: Tue, 09 Jul 2019 10:00:05 -0700

0ff466747fa6d13fcb1cf5fc8758f534f5b856c3  Martin's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2878/files/b40c482826276b4a9e610c71d8428256c515ebfb..0ff466747fa6d13fcb1cf5fc8758f534f5b856c3

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 09 Jul 2019 16:35:26 -0700

kazuho approved this pull request.

The text looks fine, and I think that the design change is also fine.

IIRC, current design allows a server to process a Initial packet with a token that it does not understand. I am not sure if that is good or bad, but at least it's slippery.

The proposed change enforces the use of a retry token for handling the payload, addressing this slipperiness (note: unverifiable new_token tokens can be ignored regardless of this 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/2878#pullrequestreview-259809298

---

From: Martin Thomson <[email-masked]>
Date: Wed, 10 Jul 2019 00:13:31 -0700

> @@ -804,17 +803,12 @@ modifying the contents of packets from future versions.

Is it worth adding some advice on how to implement this?  It's not entirely obvious.

```suggestion
connection, even after receiving a Retry.  A server that sends a Retry
therefore needs to either remember Initial protection keys or
save them in the Retry token.
```

-- 
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/2878#pullrequestreview-259906282

---

From: ianswett <[email-masked]>
Date: Sat, 20 Jul 2019 14:29:51 -0700

bc61fd05a1eda1f92240d39f9006a22e096d7676  Update draft-ietf-quic-tls.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2878/files/0ff466747fa6d13fcb1cf5fc8758f534f5b856c3..bc61fd05a1eda1f92240d39f9006a22e096d7676

---

From: ianswett <[email-masked]>
Date: Sat, 20 Jul 2019 14:30:32 -0700

6aaad3e1de4ec7cd0aeb2d3598d5921ddc9f6be7  Update draft-ietf-quic-tls.md

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

---

From: ianswett <[email-masked]>
Date: Sat, 20 Jul 2019 14:31:18 -0700

Thanks for the suggestion Martin, accepted.

-- 
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/2878#issuecomment-513500664

---

From: David Schinazi <[email-masked]>
Date: Sun, 21 Jul 2019 13:22:55 -0700

DavidSchinazi commented on this pull request.

> @@ -804,17 +803,14 @@ modifying the contents of packets from future versions.
 
+The secrets used for protecting Initial packets do not change during the
+connection, even after receiving a Retry.  A server that sends a Retry
+therefore needs to either remember Initial protection keys or save them

Shouldn't this just say that the server already needs to have access to the original destination CID because of the `original_connection_id` transport parameter? I don't think the server needs to remember/encode initial keys on top of 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/2878#pullrequestreview-264552953

---

From: ianswett <[email-masked]>
Date: Sun, 21 Jul 2019 14:52:23 -0700

67067f61332c82fff8fdc26d35c8dc8cc76b5a75  David's comment

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2878/files/6aaad3e1de4ec7cd0aeb2d3598d5921ddc9f6be7..67067f61332c82fff8fdc26d35c8dc8cc76b5a75

---

From: David Schinazi <[email-masked]>
Date: Sun, 21 Jul 2019 15:17:22 -0700

> @@ -805,8 +805,8 @@ packets even where the TLS versions offered do not include TLS 1.3.
 
 The secrets used for protecting Initial packets do not change during the
 connection, even after receiving a Retry.  A server that sends a Retry
-therefore needs to either remember Initial protection keys or save them
-in the Retry token.
+therefore needs to either remember the original connection ID and Initial
+protection keys or save the original connection ID the Retry token.

typo: connection ID *in* the Retry token.

-- 
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/2878#pullrequestreview-264556871

---

From: ianswett <[email-masked]>
Date: Mon, 22 Jul 2019 04:56:38 -0700

8efdac85fb5e0181b4b1aa3bc825c461f659c049  in the Retry token

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2878/files/67067f61332c82fff8fdc26d35c8dc8cc76b5a75..8efdac85fb5e0181b4b1aa3bc825c461f659c049

---

From: David Schinazi <[email-masked]>
Date: Mon, 22 Jul 2019 07:19:24 -0700

DavidSchinazi 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/2878#pullrequestreview-264847106