Subject: [quicwg/base-drafts] Authenticate connection IDs (#3499)

From: Martin Thomson <[email-masked]>
Date: Tue, 03 Mar 2020 22:52:40 -0800

This defines a new transport parameter and new rules for the server when it generates the Source Connection ID for Initial packets.

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

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

-- Commit Summary --

  * Authenticate connection IDs

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Wed, 04 Mar 2020 08:44:57 -0800

ianswett approved this pull request.

Thanks for writing this up, I think it's likely the right fix.

If we're going to do this, I'd like to include it in the next consensus call if possible, since it's an incompatible wire format 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/3499#pullrequestreview-368943386

---

From: David Schinazi <[email-masked]>
Date: Wed, 04 Mar 2020 09:52:30 -0800

DavidSchinazi commented on this pull request.

>  from the server, it MUST discard any packet it receives with a different Source
 Connection ID.
 
+Each endpoint includes the connection ID it includes in Initial packets in the

I think this text should be more specific about which connection ID it's talking about (the source connection ID). Also, a sentence spelling out how validation is performed can't hurt.

-- 
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/3499#pullrequestreview-368993407

---

From: Martin Thomson <[email-masked]>
Date: Wed, 04 Mar 2020 14:47:39 -0800

@martinthomson pushed 1 commit.

6640a46f525244ceec7a4dad795cf203a6b28f35  Moar text on authentication of connection IDs

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3499/files/6876f72da14a74400650659e7e8c193c3be1e393..6640a46f525244ceec7a4dad795cf203a6b28f35

---

From: Martin Thomson <[email-masked]>
Date: Wed, 04 Mar 2020 14:47:57 -0800

martinthomson commented on this pull request.

 Connection ID.

Thanks David.  Fixed; and you get two sentences.

-- 
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/3499#discussion_r387980884

---

From: Christopher Wood <[email-masked]>
Date: Wed, 04 Mar 2020 15:00:30 -0800

chris-wood commented on this pull request.

 Connection ID.
 
+Each endpoint includes the Source Connection ID it includes in Initial packets

```suggestion
Each endpoint includes its Initial packet Source Connection ID 
```

-- 
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/3499#pullrequestreview-369187323

---

From: Martin Thomson <[email-masked]>
Date: Wed, 04 Mar 2020 15:13:56 -0800

f994a56b201197f601d914b0a68e5056a52cb68b  Grammar is HARD

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3499/files/6640a46f525244ceec7a4dad795cf203a6b28f35..f994a56b201197f601d914b0a68e5056a52cb68b

---

From: Martin Thomson <[email-masked]>
Date: Wed, 04 Mar 2020 15:14:44 -0800

Connection ID.

Thanks.  I've tried to avoid that sort of train of nouns in the past.  I have an alternative for you.  More words again :(, but maybe clearer.

-- 
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/3499#discussion_r387990958

---

From: Christopher Wood <[email-masked]>
Date: Wed, 04 Mar 2020 15:15:45 -0800

Connection ID.

No worries! Just a suggestion in reaction to redundant "includes."

-- 
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/3499#discussion_r387991296

---

From: David Schinazi <[email-masked]>
Date: Wed, 04 Mar 2020 17:07:23 -0800

I agree that this is the right fix. However, I think I would prefer to allow the server to reuse the client's initial DCID when sending the first server initial when there are no retries involved. From a theoretical standpoint, requiring the server to change the CID makes the one offered by the client useless which seems silly - but that is admittedly a very weak argument. From a practical standpoint, that would allow us to deploy this change faster.

-- 
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/3499#issuecomment-594974055

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 04 Mar 2020 17:12:34 -0800

kazuho 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/3499#pullrequestreview-369235892

---

From: ianswett <[email-masked]>
Date: Wed, 04 Mar 2020 17:22:34 -0800

David is correct that that change would speed our deployment by quite a bit.

I'm more concerned about raising the bar too high for implementers that have very simple load balancing infrastructure, so I think this deserves an email to the list if you're willing, @martinthomson 

-- 
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/3499#issuecomment-594978177

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 04 Mar 2020 17:44:12 -0800

IMO I think it is a good idea to discourage servers from using client-generated data for load balancing etc., even though we might agree that we do not need to forbid that.

Maybe SHOULD 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/3499#issuecomment-594983525

---

From: David Schinazi <[email-masked]>
Date: Wed, 04 Mar 2020 22:06:58 -0800

I'm not sure we should be discouraging servers from using client-generated connection IDs. We've been doing that for years and I'm not sure there is a credible attack 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/3499#issuecomment-595045877

---

From: Martin Thomson <[email-masked]>
Date: Wed, 04 Mar 2020 22:23:43 -0800

Discuss this on the issue please.

-- 
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/3499#issuecomment-595050422

---

From: Christian Huitema <[email-masked]>
Date: Thu, 05 Mar 2020 17:05:44 -0800

huitema commented on this pull request.

Could we add to this PR a description of the attack, maybe in the security section, and the suggestion that servers may want to tie the retry token to both the ODCID and the server's initial SCID?

 Connection ID.
 
+Each endpoint includes the value of the Source Connection ID from Initial
+packets it sends in the handshake_connection_id transport parameter; see
+{{transport-parameter-definitions}}.  Each endpoint validates that the value
+received from the peer is identical to the value of the transport parameter.
+Absense of the handshake_connection_id transport parameter or a mismatch in
+values MUST be treated as a connection error of type PROTOCOL_VIOLATION.  When
+sending a Retry packet or the first Initial packet, a server MUST select values
+for the Source Connection ID field that differ from the values the client
+includes in the Destination Connection ID field.  These measures ensure that the
+choice of connection ID cannot be influenced by an attacker.
+

I have some difficulty parsing that in the retry scenario. is "the Source Connection ID from Initial
packet" the value found in the retry packet sent by the server, or is the source connection ID in the first initial packet of the server? Currently, there is no requirement that these two be the same 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/3499#pullrequestreview-370021781

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Mar 2020 18:43:32 -0700

d92b9781fecedbe32471d0051805e29b62989a77  Authenticate connection IDs

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Mar 2020 18:44:58 -0700

Hi everyone, I've redone this based on discussion.  Please take another look.

@huitema, I don't think that this benefits from a description of an attack.  This is not a change made in response to an attack, but in response to a request to make the protocol easier to analyze for the purposes of proving its security.

-- 
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/3499#issuecomment-597401628

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Mar 2020 18:45:58 -0700

2fc5157ed013ae8f84f17e44409c42be87660feb  Authenticate connection IDs

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3499/files/d92b9781fecedbe32471d0051805e29b62989a77..2fc5157ed013ae8f84f17e44409c42be87660feb

---

From: David Schinazi <[email-masked]>
Date: Tue, 10 Mar 2020 19:18:20 -0700

DavidSchinazi approved this pull request.

Thanks for writing this! I support this change and only have minor nits.

> +verifying them ensures that the choice of connection ID cannot be influenced by
+an attacker. An endpoint MUST treat any of the following as a connection error
+of type PROTOCOL_VIOLATION:
+
+* absence of the handshake_connection_id transport parameter from either
+  endpoint,
+
+* absence of the original_connection_id transport parameter from the server,
+
+* absence of the retry_connection_id transport parameter from the server after
+  receiving a Retry packet,
+
+* presence of the retry_connection_id transport parameter when no Retry packet
+  was received, or
+
+* a mismatch between values of these transport parameters and the Destination

I think you mean `a mismatch between values of these transport parameters and the respective Connection ID fields an endpoint used.` since it doesn't just protect destination

> @@ -1522,9 +1560,10 @@ specify whether they MUST, MAY, or MUST NOT be stored for 0-RTT. A client need
 not store a transport parameter it cannot process.
 
 A client MUST NOT use remembered values for the following parameters:
-original_connection_id, preferred_address, stateless_reset_token, and
-ack_delay_exponent. The client MUST use the server's new values in the
-handshake instead, and absent new values from the server, the default value.
+ack_delay_exponent, handshake_connection_id, original_connection_id,

Shouldn't `original_connection_id` stay in this list? And what about `handshake_connection_id`?

> @@ -4357,13 +4395,15 @@ A client MUST NOT reset the packet number for any packet number space after
 processing a Retry packet; {{packet-0rtt}} contains more information on this.
 
 A server acknowledges the use of a Retry packet for a connection using the
-original_connection_id transport parameter (see
-{{transport-parameter-definitions}}).  If the server sends a Retry packet, it
+retry_connection_id transport parameter; see
+{{transport-parameter-definitions}}. If the server sends a Retry packet, it

We may want to rephrase this to more clearly state that `original_connection_id` is always filled in by the server

> @@ -4572,9 +4612,8 @@ original_connection_id (0x00):
 
 : The value of the Destination Connection ID field from the first Initial packet
   sent by the client.  This transport parameter is only sent by a server.  This
-  is the same value sent in the "Original Destination Connection ID" field of a
-  Retry packet (see {{packet-retry}}).  A server MUST include the
-  original_connection_id transport parameter if it sent a Retry packet.
+  is the same value sent in the "Original Destination Connection ID" field that

Thanks for catching that, my bad for missing it.

> @@ -4742,6 +4781,22 @@ active_connection_id_limit (0x0e):
   When a zero-length connection ID is being used, the active_connection_id_limit
   parameter MUST NOT be sent.
 
+handshake_connection_id (0x0f):
+
+: The value that the endpoint included in the Source Connection ID field of the
+  first Initial packet it sends during the handshake. Endpoints MUST validate
+  that this transport parameter is present and that it matches the value that
+  was received in Initial packets. Authenticating this value ensures that an
+  attacker is unable to influence the selection of connection IDs during the

to avoid jinxing things, how about `Authenticating this value mitigates an attacker's ability to influence the selection of connection IDs during 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/3499#pullrequestreview-372435145

---

From: martinduke <[email-masked]>
Date: Tue, 10 Mar 2020 19:44:03 -0700

martinduke requested changes on this pull request.

Thanks to @martinthomson and @dschinazi for making it clear this is about security proofs, not specific attacks. I'm more inclined to support it now.

If there is really no path to proving that the spec already authenticates server initial SCIDs, then this may be the price we have to pay to be provably secure. The Retry parts may be necessary in any case.

But it's a pain. Clients keeping up to 3 different peer CIDs at one time in the handshake; longer retry tokens; and a bunch of mandatory TPs, changing on a per-connection basis, with no observable purpose. It feels like bloat but I don't have the tools and resources to prove it.

What if some of these CIDs are zero length? Do we omit them?

-- 
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/3499#pullrequestreview-372441388

---

From: David Schinazi <[email-masked]>
Date: Tue, 10 Mar 2020 20:16:29 -0700

My read of the zero-length CID case is that the TP MUST be included, and be zero-length. Perhaps we should add a sentence to clarify 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/3499#discussion_r390724969

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Mar 2020 20:38:03 -0700

0560290f9041482293cea9cdd053d326345b735a  Review feedback from David and Martin

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3499/files/2fc5157ed013ae8f84f17e44409c42be87660feb..0560290f9041482293cea9cdd053d326345b735a

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Mar 2020 20:44:15 -0700

beba71f8e874d803b9fb712312d62f769988d74d  Less text, more cross-reference

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3499/files/0560290f9041482293cea9cdd053d326345b735a..beba71f8e874d803b9fb712312d62f769988d74d

---

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Mar 2020 20:44:43 -0700

Done! Thanks both.

As for the "mitigates" formulation, I think that we are safe in making claims, but I realize that we don't need to.  I've removed a lot of text from the transport parameter definitions and replaced it with a pointer to the new section.

-- 
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/3499#discussion_r390730977

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 17 Mar 2020 17:22:14 -0700

LGTM. I like the cleanliness and explicitness of this design.

-- 
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/3499#pullrequestreview-376489495

---

From: ianswett <[email-masked]>
Date: Sun, 22 Mar 2020 05:47:03 -0700

Some small nits, but this still LG.

I'd feel more confident in the text's completeness and correctness if a few implementations implemented it and achieved interop.  I don't anticipate any issues, but when we change the handshake, unanticipated new edge cases can be introduced.

> @@ -1475,6 +1475,48 @@ lifetime of a connection, especially in response to connection migration
 ({{migration}}); see {{issue-cid}} for details.

+## Authenticating Connection IDs {#cid-auth}
+
+The choice each endpoint makes about connection IDs during the handshake is
+authenticated by including all values in transport parameters; see
+{{transport-parameters}}. This ensures that all connection IDs are authenticated
+by the cryptographic handshake.
+
+Each endpoint includes the value of the Source Connection ID field from Initial
+packets it sends in the handshake_connection_id transport parameter; see

One could read that this Connection ID is only used on Initial packets.  If so, I think initial_connection_id would be a better TP name.  But I think this CID can be used used on Initial and 0-RTT from the client and all Initial, Handshake and 1-RTT from the server?

+  endpoint,
+
+* absence of the original_connection_id transport parameter from the server,
+
+* absence of the retry_connection_id transport parameter from the server after
+  receiving a Retry packet,
+
+* presence of the retry_connection_id transport parameter when no Retry packet
+  was received, or
+
+* a mismatch between values received from a peer in these transport parameters
+  and the value sent in the corresponding Destination Connection ID fields of
+  Initial packets.
+
+If a zero-length connection IDs is selected, the corresponding transport

```suggestion
If a zero-length connection ID is selected, the corresponding transport
```

-- 
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/3499#pullrequestreview-379001134

---

From: Christian Huitema <[email-masked]>
Date: Sun, 22 Mar 2020 10:43:18 -0700

@ianswett I have implement it in picoquic as such:

1) Always send the handshake CID. Not an interop issue since implementations are supposed to ignore unknown CID

2) Send the retry CID when also sending the ODCID.

3) Verify that the client is not sending Retry CID

4) If the protocol version is newer than "draft 27", check that the handshake CID is present, and that the retry CID is present if required. Then check that the values are what is expected.

5) When verifying the retry token, check that the retry CID sent by the client is what is expected.

That version of picoquic is used in the interop runner, and I have not noticed any issue related to CID checks. But then the tests are bypassed with draft-27 versions. I suppose I could split the TP test into verifying that the CID are present if the version is newer than draft-27, and then check that the CID have the expected value if they are present. That would allow for interop testing with draft 27.

-- 
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/3499#issuecomment-602245593

---

From: David Schinazi <[email-masked]>
Date: Sun, 22 Mar 2020 13:42:57 -0700

I think it's called `handshake_connection_id` because it is the connection ID used during the handshake, I think it's a reasonable name

-- 
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/3499#discussion_r396138532

---

From: David Schinazi <[email-masked]>
Date: Sun, 22 Mar 2020 13:48:42 -0700

I like @huitema's deployment plan. I think it would be valuable to get deployment experience of these new transport parameters in draft-27 so that it doesn't break interop. Perhaps we should add a temporary subsection to the spec on how to this, with a note that it'll be removed before publication. It could say something like this:
1. If using draft-28 or later, these transport parameters MUST be sent, and received ones MUST be validated.
2. If using draft-27 or earlier, these transport parameters SHOULD be sent, and upon receipt:
    - if `handshake_connection_id` is present, then validate using draft-28 rules
    - else validate using draft-27 rules

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/3499#issuecomment-602270861

---

From: Christian Huitema <[email-masked]>
Date: Sun, 22 Mar 2020 13:55:40 -0700

@DavidSchinazi in practice, you can only publish new spec as draft-28. So what you want is not a new draft, but maybe a note on the implementers' slack. By the way, I did change my implementation to check the values if they are sent.

-- 
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/3499#issuecomment-602271629

---

From: David Schinazi <[email-masked]>
Date: Sun, 22 Mar 2020 14:11:22 -0700

@huitema That sounds good to me, or perhaps on the implementer's wiki

-- 
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/3499#issuecomment-602273562

---

From: Martin Thomson <[email-masked]>
Date: Sun, 22 Mar 2020 14:54:30 -0700

efb4c78cb4f64b9a926382777700f964ec6c0d61  singular

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

---

From: Christian Huitema <[email-masked]>
Date: Sun, 22 Mar 2020 16:37:22 -0700

@DavidSchinazi, @ianswett I just added a wiki page on [testing this PR](https://github.com/quicwg/base-drafts/wiki/Testing-the-authentication-of-connection-ID-before-draft-28).

-- 
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/3499#issuecomment-602298216

---

From: Mike Bishop <[email-masked]>
Date: Mon, 23 Mar 2020 11:46:35 -0700

MikeBishop approved this pull request.

> +Each endpoint ensures that the values of transport parameters matches the
+values. The values provided by a peer for these transport parameters MUST match

This sentence feels like it ends prematurely.  Perhaps...
```suggestion
Each endpoint ensures that the values of the peer's transport parameters
match the values the endpoint sent in its packet headers. The values
provided by a peer for these transport parameters MUST match the
```

-- 
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/3499#pullrequestreview-379720471

---

From: Martin Thomson <[email-masked]>
Date: Tue, 24 Mar 2020 03:06:51 -0700

41928cd80c885542633c0bd9898640850e51c69c  complete sentences.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3499/files/efb4c78cb4f64b9a926382777700f964ec6c0d61..41928cd80c885542633c0bd9898640850e51c69c

---

From: Martin Thomson <[email-masked]>
Date: Tue, 24 Mar 2020 03:09:55 -0700

f44f24e94b3eae910909cdcc76b738792a6a0105  Merge branch 'master' into authenticate-hs-cid

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3499/files/41928cd80c885542633c0bd9898640850e51c69c..f44f24e94b3eae910909cdcc76b738792a6a0105