Subject: [quicwg/base-drafts] use TRANSPORT_PARAMETER_ERROR when authenticating connection IDs (#3691)

From: Marten Seemann <[email-masked]>
Date: Sat, 23 May 2020 21:18:52 -0700

As discussed in https://github.com/quicwg/base-drafts/pull/3499/files#r422775281, most of the errors that can occur when authenticating connection IDs should be TRANSPORT_PARAMETER_ERRORs, not PROTOCOL_VIOLATIONs.

The guiding principle here (as we apply it for FRAME_ENCODING_ERRORs) is that if an error is detectable without accessing connection state, we use the more specific error. So for example, since `initial_source_connection_id` and `original_destination_connection_id` are required transport parameters, their absence should be responded to with a TRANSPORT_PARAMETER_ERROR. A similar argument applies to the absence of the `retry_source_connection_id` TP, although I can see how one might use PROTOCOL_VIOLATION here (note that this is still valid behavior with this PR, as every more specific error can be replaced with a PROTOCOL_VIOLATION, if an implementation wishes to do so).
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * use TRANSPORT_PARAMETER_ERROR when authenticating connection IDs

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Sun, 24 May 2020 03:16:58 -0700

This changes the error codes that we require, so it is technically a design change.  I think we'll need an issue to track this.

-- 
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/3691#issuecomment-633208960

---

From: Marten Seemann <[email-masked]>
Date: Tue, 26 May 2020 20:23:12 -0700

@martinthomson Created #3703.

-- 
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/3691#issuecomment-634404460

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 20:42:14 -0700

@martinthomson commented on this pull request.

>  carrying attacker-chosen connection IDs during the handshake. An endpoint MUST
-treat any of the following as a connection error of type PROTOCOL_VIOLATION:
+treat any of the following as a connection error of type
+TRANSPORT_PARAMETER_ERROR:

A list is good here, but these aren't all equal.  In particular, the behaviour that is conditioned on whether a Retry was sent is not consistent with your principle.  I would argue that those are also PROTOCOL_VIOLATION cases.

If you agree, then you can move those two into the next list (which currently only has one item).  That leaves two items in this list and I think that can be turned back into prose.

-- 
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/3691#pullrequestreview-418836032

---

From: Marten Seemann <[email-masked]>
Date: Tue, 26 May 2020 21:07:47 -0700

@marten-seemann pushed 1 commit.

065e9aa278211b463d125d3afc376e32375bf084  use PROTOCOL_VIOLATIONs for the retry_source_connection_id checks

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3691/files/994f32dd4fe1c160ad8ac1c88180e5b2b059e5a9..065e9aa278211b463d125d3afc376e32375bf084

---

From: Marten Seemann <[email-masked]>
Date: Tue, 26 May 2020 21:07:49 -0700

@marten-seemann commented on this pull request.

Agreed. And fixed.

-- 
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/3691#discussion_r430845740

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 21:22:20 -0700

I made a prose suggestion in a PR, which you can take or leave.

-- 
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/3691#discussion_r430848695

---

From: Marten Seemann <[email-masked]>
Date: Tue, 26 May 2020 21:59:57 -0700

@marten-seemann pushed 2 commits.

e241f42ed92765c3393c406f7aedc560c4609f04  Turn the absence clauses into prose
bb533c01f7947d39d94074c8e7459c1ded3f077c  Merge pull request #6 from quicwg/3691-prose

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3691/files/065e9aa278211b463d125d3afc376e32375bf084..bb533c01f7947d39d94074c8e7459c1ded3f077c

---

From: Marten Seemann <[email-masked]>
Date: Tue, 26 May 2020 22:00:08 -0700

Thank you. Merged.

-- 
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/3691#discussion_r430857545

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 26 May 2020 22:46:58 -0700

@kazuho commented on this pull request.

-* absence of the original_destination_connection_id transport parameter from
-  the server,
+An endpoint MUST treat the following as a connection error of type
+PROTOCOL_VIOLATION:
 
 * absence of the retry_source_connection_id transport parameter from the server
   after receiving a Retry packet,

I think my preference would be to allow either PROTOCOL_VIOLATION or TRANSPORT_PARAMETER_ERROR to be used when retry_source_connection_id is missing (or when it exists for no reason).

This is borderline, and in the past, we have allowed more than one error code to be used when that is reasonable. In this specific case, the QUIC stack can ask the decoder of Transport Parameters to provide source_connection_id (or to not provide one). In such a design, it is reasonable for the decoder of the Transport Parameters to raise an error when that expectation does not match.

-- 
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/3691#pullrequestreview-418872644

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 22:55:36 -0700

I think that the existing rules we basically permit either.  Is there some way you would prefer to phrase this?

-- 
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/3691#discussion_r430872936

---

From: Marten Seemann <[email-masked]>
Date: Tue, 26 May 2020 23:00:34 -0700

We permit specific error codes to be replaced by PROTOCOL_VIOLATION, not the other way round, iirc.

-- 
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/3691#discussion_r430874546

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 26 May 2020 23:02:10 -0700

The simplest change would be to move these two cases that deal with `retry_source_connection_id` above, so that they would be classified as TRANSPORT_PARAMETER_ERROR. Doing so does not forbid endpoints from sending PROTOCOL_VIOLATION, as we have a catch-all clause stating that sending PROTOCOL_VIOLATION in place of other error code is permitted.

If want to be verbose, we could say that _An endpoint MUST treat the following as a connection error of type PROTOCOL_VIOLATION or TRANSPORT_PARAMETER_ERROR: (list the two bullet points covering `retry_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/3691#discussion_r430875128

---

From: Martin Thomson <[email-masked]>
Date: Tue, 26 May 2020 23:51:36 -0700

WFM

-- 
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/3691#discussion_r430893414

---

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

@MikeBishop commented on this pull request.

 * presence of the retry_source_connection_id transport parameter when no Retry
-  packet was received, or

Why no "or"?

-- 
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/3691#pullrequestreview-419573888

---

From: Martin Thomson <[email-masked]>
Date: Wed, 27 May 2020 18:24:24 -0700

+  packet was received.

```suggestion
  packet was received, or
```

-- 
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/3691#pullrequestreview-419716248

---

From: Martin Thomson <[email-masked]>
Date: Thu, 28 May 2020 06:52:59 -0700

If you take the suggestions here, I will mark this for inclusion in the next consensus call.  (Otherwise, we should continue discussion.)

+PROTOCOL_VIOLATION:

```suggestion
An endpoint MUST treat the following as a connection error of type
TRANSPORT_PARAMETER_ERROR or PROTOCOL_VIOLATION:
```

Just to enact the suggestion from below and avoid ambiguity.

-- 
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/3691#pullrequestreview-420128404