Subject: [quicwg/base-drafts] introduce a max_connection_ids transport parameter (#1998)

From: Marten Seemann <[email-masked]>
Date: Tue, 13 Nov 2018 08:01:10 +0000 (UTC)

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

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

-- Commit Summary --

  * introduce a max_connection_ids transport parameter

-- File Changes --

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

-- Patch Links --

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

---

From: MikkelFJ <[email-masked]>
Date: Tue, 13 Nov 2018 00:06:14 -0800

mikkelfj commented on this pull request.

Looks good to me

>  An endpoint SHOULD ensure that its peer has a sufficient number of available and
-unused connection IDs.  While each endpoint independently chooses how many
-connection IDs to issue, endpoints SHOULD provide and maintain at least eight
-connection IDs.  The endpoint can do this by always supplying a new connection
-ID when a connection ID is retired by its peer or when the endpoint receives a
-packet with a previously unused connection ID.  Endpoints that initiate
-migration and require non-zero-length connection IDs SHOULD provide their peers
-with new connection IDs before migration, or risk the peer closing the
-connection.
+unused connection IDs. Endpoints store received connection IDs for future use. 
+They advertise the limit how many unretired connection IDs they are willing to 

Word missing?

-- 
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/1998#pullrequestreview-174231329

---

From: Marten Seemann <[email-masked]>
Date: Tue, 13 Nov 2018 00:10:09 -0800

marten-seemann commented on this pull request.

Where?

-- 
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/1998#discussion_r232930761

---

From: Marten Seemann <[email-masked]>
Date: Tue, 13 Nov 2018 00:10:31 -0800

@marten-seemann pushed 1 commit.

1bcf7c3  fix whitespace

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1998/files/8a75c3cb691bb15f71dbbfb178c94ac428dfb714..1bcf7c353d6c4e09f5d082eaa8bbd290acef0d3f

---

From: MikkelFJ <[email-masked]>
Date: Tue, 13 Nov 2018 00:12:20 -0800

I wonder if exposing max ID limit in TP is a minor sexurity concern. Easier to attack endpoints with few active ID’s esp. with future multipath.

-- 
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/1998#issuecomment-438174560

---

From: Marten Seemann <[email-masked]>
Date: Tue, 13 Nov 2018 00:14:26 -0800

> @@ -3806,6 +3803,7 @@ language from Section 3 of {{!TLS13=RFC8446}}.
       max_ack_delay(11),
       disable_migration(12),
       preferred_address(13),
+      max_connection_ids(14),

Do we need to renumber these again?

> @@ -5303,6 +5312,7 @@ from 0xFF00 to 0xFFFF are reserved for Private Use {{!RFC8126}}.
 | 0x9   | VERSION_NEGOTIATION_ERROR | Version negotiation failure   | {{error-codes}} |
 | 0xA   | PROTOCOL_VIOLATION        | Generic protocol violation    | {{error-codes}} |
 | 0xC   | INVALID_MIGRATION         | Violated disabled migration   | {{error-codes}} |
+| 0xD   | CONNECTION_ID_LIMIT_ERROR | Too many connection IDs provided | {{error-codes}} |

The whole table looks like it wants to be reordered 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/1998#pullrequestreview-174233744

---

From: ianswett <[email-masked]>
Date: Tue, 13 Nov 2018 08:31:48 -0800

ianswett commented on this pull request.

> @@ -3981,6 +3979,12 @@ A client MUST NOT include an original connection ID, a stateless reset token, or
 a preferred address.  A server MUST treat receipt of any of these transport
 parameters as a connection error of type TRANSPORT_PARAMETER_ERROR.
 
+max_connection_ids (0x000e):
+
+: The maximum number of connection IDs that the peer is willing to store.
+  This value includes only connection IDs sent in NEW_CONNECTION_ID frames.
+  If this parameter is absent, a default of 8 is assumed.

I'd prefer a default of 0.

-- 
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/1998#pullrequestreview-174462935

---

From: ianswett <[email-masked]>
Date: Tue, 13 Nov 2018 08:32:05 -0800

This works for me, FWIW.

-- 
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/1998#pullrequestreview-174463142

---

From: erickinnear <[email-masked]>
Date: Tue, 13 Nov 2018 09:53:41 -0800

erickinnear commented on this pull request.

I'm not sure I see a default of 0 as being better than 8. 
If we're going to introduce a TP, and if everyone is going to set the TP all the time, then the default doesn't matter. If not, then I think we want to maintain 8 (or some similar value) for the same reasons that we had it before there was a TP. 

-- 
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/1998#discussion_r233155731

---

From: Mike Bishop <[email-masked]>
Date: Tue, 13 Nov 2018 11:12:11 -0800

MikeBishop commented on this pull request.

"advertise the limit how many" could probably do with a limit "on" how many, but it might read better to simply "advertise how many".

-- 
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/1998#discussion_r233183862

---

From: MikkelFJ <[email-masked]>
Date: Tue, 13 Nov 2018 11:47:33 -0800

The point is that otherwise everyone is going to change it from 8. Using 0 avoids the discussion of who's default is better and is in line with other 0 defaults such as max streams.

-- 
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/1998#discussion_r233197032

---

From: ianswett <[email-masked]>
Date: Tue, 13 Nov 2018 11:48:21 -0800

Agreed, we've consistently gone with 0 being a default whenever possible, even in cases when 0 makes the transport basically useless.

-- 
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/1998#discussion_r233197308

---

From: erickinnear <[email-masked]>
Date: Tue, 13 Nov 2018 19:50:55 +0000 (UTC)

Then that works for me! 

-- 
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/1998#discussion_r233198173

---

From: Marten Seemann <[email-masked]>
Date: Tue, 13 Nov 2018 16:39:29 -0800

@marten-seemann pushed 2 commits.

fa65397  Mike's suggestion
8ff9361  assume 0 as default for max_connection_ids

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1998/files/1bcf7c353d6c4e09f5d082eaa8bbd290acef0d3f..8ff93615a18bee5e0b51f96e87c0eddb2a6062e3

---

From: Marten Seemann <[email-masked]>
Date: Tue, 13 Nov 2018 16:39:38 -0800

Done.

-- 
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/1998#discussion_r233277305

---

From: Marten Seemann <[email-masked]>
Date: Wed, 14 Nov 2018 00:39:55 +0000 (UTC)

Thanks for the suggestion, @MikeBishop!

-- 
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/1998#discussion_r233277355

---

From: janaiyengar <[email-masked]>
Date: Wed, 14 Nov 2018 03:50:33 -0800

janaiyengar commented on this pull request.

-unused connection IDs.  While each endpoint independently chooses how many
-connection IDs to issue, endpoints SHOULD provide and maintain at least eight
-connection IDs.  The endpoint can do this by always supplying a new connection
-ID when a connection ID is retired by its peer or when the endpoint receives a
-packet with a previously unused connection ID.  Endpoints that initiate
-migration and require non-zero-length connection IDs SHOULD provide their peers
-with new connection IDs before migration, or risk the peer closing the
-connection.
+unused connection IDs. Endpoints store received connection IDs for future use.
+They advertise how many unretired connection IDs they are willing to store in
+the transport parameters. An endpoint MUST NOT provide more connection IDs than
+this limit. An endpoint MUST treat receipt of more than this number of
+connection IDs as an error of type CONNECTION_ID_LIMIT_ERROR. If an endpoint has

Why is this error necessary? A peer can simply drop the ones that it can't store.

+They advertise how many unretired connection IDs they are willing to store in
+the transport parameters. An endpoint MUST NOT provide more connection IDs than

Suggestion to replace this previous sentence: "An endpoint uses a transport parameter to advertise the maximum number of connection IDs it can store for future use."

-unused connection IDs.  While each endpoint independently chooses how many
-connection IDs to issue, endpoints SHOULD provide and maintain at least eight
-connection IDs.  The endpoint can do this by always supplying a new connection
-ID when a connection ID is retired by its peer or when the endpoint receives a
-packet with a previously unused connection ID.  Endpoints that initiate
-migration and require non-zero-length connection IDs SHOULD provide their peers
-with new connection IDs before migration, or risk the peer closing the
-connection.
+unused connection IDs. Endpoints store received connection IDs for future use.
+They advertise how many unretired connection IDs they are willing to store in
+the transport parameters. An endpoint MUST NOT provide more connection IDs than
+this limit. An endpoint MUST treat receipt of more than this number of
+connection IDs as an error of type CONNECTION_ID_LIMIT_ERROR. If an endpoint has
+provided its peer with the maximum number of connection IDs, the earliest time
+it can provide a new connection ID is when the peer retires a connection ID.

suggestion: "... connection IDs, it can only provide a new connection ID when the peer retires one."

The maximum number of connection IDs that this endpoint can store for future use. These connection IDs are issued by the peer and sent in NEW_CONNECTION_ID frames.

> @@ -4958,6 +4962,10 @@ INVALID_MIGRATION (0xC):
 : A peer has migrated to a different network when the endpoint had disabled
   migration.
 
+CONNECTION_ID_LIMIT_ERROR (0xd):
+
+: A peer received more connection IDs than its advertised limit.

See my comment above

-- 
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/1998#pullrequestreview-174818016

---

From: Marten Seemann <[email-masked]>
Date: Wed, 14 Nov 2018 05:36:36 -0800

I *can* store a lot. But I might not be willing to.

-- 
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/1998#discussion_r233448622

---

From: Marten Seemann <[email-masked]>
Date: Wed, 14 Nov 2018 05:40:44 -0800

I regard this as an equivalent to flow control and stream ID flow control. We have errors there if the peer violates a limit. It feels weird to me to set a limit, but then to silently tolerate a violation of that limit. And it will be a lot easier to develop compliant implementation if we actually enforce the limit.

-- 
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/1998#discussion_r233450045

---

From: MikkelFJ <[email-masked]>
Date: Wed, 14 Nov 2018 06:00:14 -0800

One downside to the count based TP is that an endpoint can't set aside fix size buffer for CID's because it depends on the CID lengths. Especially for constrained devices this could be an issue. A device might handle have 32 bytes set aside for 4 CIDs up to length 8, but it would only be able to handle one CID of length 18.

-- 
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/1998#issuecomment-438671219

---

From: Mike Bishop <[email-masked]>
Date: Mon, 19 Nov 2018 11:30:21 -0800

Why "only" connection IDs sent in NCID frames?  The CIDs provided in the handshake and SPA consume space in this list, get retired, etc. If they don't count toward this limit, that would suggest that I'll be temporarily 1-2 over the limit early in the connection, then drift down to the limit as I retire those CIDs. That doesn't seem like the desired effect.

-- 
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/1998#pullrequestreview-176460933

---

From: Christian Huitema <[email-masked]>
Date: Mon, 26 Nov 2018 21:58:06 -0800

huitema commented on this pull request.

I we have default = 0, do we also need a "migration not supported" TP? It would seem that not setting a non null value would effectively prevent migration.

-- 
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/1998#discussion_r236528581

---

From: Mike Bishop <[email-masked]>
Date: Tue, 27 Nov 2018 10:42:27 -0800

It would, but you can support CID rotation *without* supporting migration, so the reverse isn't true.

-- 
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/1998#discussion_r236795616