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

From: ianswett <[email-masked]>
Date: Mon, 17 Dec 2018 13:13:14 -0800

@marten-seemann In concept, I think this is good, but can you resolve the outstanding comments?

-- 
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-448000755

---

From: Marten Seemann <[email-masked]>
Date: Tue, 18 Dec 2018 21:23:09 -0800

marten-seemann 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.

This leads to a weird corner case. If I send a max_connection_ids value of 0, then the connection ID used during the handshake would already overflow this limit. If I sent 1, and at the same time send the SPA, this would also overflow.
Limiting the counting to NCID frames avoids all these corner cases.

-- 
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_r242794244

---

From: Marten Seemann <[email-masked]>
Date: Tue, 18 Dec 2018 21:23:33 -0800

>  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 how many unretired connection IDs they are willing to store in
+the transport parameters. An endpoint MUST NOT provide more connection IDs than

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_r242794279

---

From: Marten Seemann <[email-masked]>
Date: Tue, 18 Dec 2018 21:24:05 -0800

-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.

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_r242794327

---

From: Marten Seemann <[email-masked]>
Date: Tue, 18 Dec 2018 21:25:30 -0800

@ianswett I accepted most of @janaiyengar's suggestions. I think @MikeBishop's comment still needs some discussion.

-- 
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-448473690

---

From: ianswett <[email-masked]>
Date: Wed, 19 Dec 2018 06:28:31 -0800

ianswett commented on this pull request.

This LGTM, but we definitely need to resolve Mike's comment.

> @@ -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),

I don't see a need to, personally.

I think it makes sense to only apply this to NEW_CONNECTION_ID CIDs as Marten said.

SPA is a special case, since you have to use the new CID with the new address, I don't think I can treat that CID like other CIDS.

> @@ -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.

I think your comment was lost, can you restate it 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/1998#pullrequestreview-186566865

---

From: Mike Bishop <[email-masked]>
Date: Wed, 19 Dec 2018 13:16:32 -0800

MikeBishop commented on this pull request.

Hm, that's true.  But the reverse creates an odd situation where the server can issue 7 CIDs when the client has said they only want 5, but the client can't (or shouldn't) reduce its value by 2 and constrain itself for the lifetime of the connection because of a start-only overflow.

Perhaps the server SHOULD NOT issue more total CIDs than this, but MUST NOT issue more CIDs via NCID?

-- 
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_r243078086

---

From: Mike Bishop <[email-masked]>
Date: Wed, 19 Dec 2018 13:18:01 -0800

Worst case, from an implementation perspective, if you hit your limit and still have the handshake CIDs, retire them and the right things will happen.

-- 
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_r243078488

---

From: janaiyengar <[email-masked]>
Date: Wed, 19 Dec 2018 13:33:28 -0800

janaiyengar commented on this pull request.

You have the two sides mixed up. This param should convey the max that _this_ endpoint is willing to store. For clarity, I also suggest that you explicitly call out who is sending the new connection IDs: "... includes only connections IDs sent by the peer..."

Ugh. I don't like including the CID with SPA in this count for the reason @ianswett mentions. Given that the only CID that a client will _store_ is the one that is sent on an SPA, and even that will get used immediately (or get discarded), I think it's reasonable to say that the number of _stored_ CIDs is limited by max_connection_ids.

-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 SHOULD 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
+unused connection IDs. Endpoints store received connection IDs for future use.
+An endpoint uses a transport parameter to advertise the maximum number of
+connection IDs it can store for future use. 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

Do we want a new error for this, or can we live with PROTOCOL_VIOLATION?

-- 
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-186755366

---

From: Dmitri Tikhonov <[email-masked]>
Date: Wed, 19 Dec 2018 14:01:38 -0800

dtikhonov commented on this pull request.

SPA=?

-- 
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_r243090990

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 19 Dec 2018 14:04:39 -0800

kazuho commented on this pull request.

I think this discussion is a good reason to not have the enforcement (i.e. close the connection if peer exceeds the maximum).

As @ekr has pointed out in https://github.com/quicwg/base-drafts/issues/1994#issuecomment-441305154, the `max_connection_ids` can be a purely advisory value without defining any error conditions.

-- 
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_r243091767

---

From: Kazuho Oku <[email-masked]>
Date: Wed, 19 Dec 2018 14:05:18 -0800

@dtikhonov Server Preferred Address

-- 
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_r243091936

---

From: Marten Seemann <[email-masked]>
Date: Thu, 20 Dec 2018 18:40:20 -0800

I'm not sure why enforcing this creates pressure to increase the max. Sure, you need to advertise a value that actually makes sense (i.e. you should request as many CIDs as you think you'll need), and not rely on the peer overflowing this limit (which is exactly what we're trying to avoid in the first place).

-- 
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_r243475549

---

From: Marten Seemann <[email-masked]>
Date: Thu, 20 Dec 2018 18:42:30 -0800

That's the old discussion of error codes vs. error messages, right? Considering the fact that we have separate *_LIMIT_ERRORs for flow control and stream IDs, I thought it would be consistent to have an error code for this as well. I could live with PROTOCOL_VIOLATION as well, if people are unhappy with 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/1998#discussion_r243475748

---

From: janaiyengar <[email-masked]>
Date: Fri, 21 Dec 2018 20:52:25 +0000 (UTC)

@kazuho: I think that's fair, and I'm ok with not closing the connection if a peer violates my max_connection_id. We need to be clear that an endpoint that drops a NEW_CID because it has no space MUST NOT send a RETIRE_CID. This is an easy bug, so it's worth noting.

-- 
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_r243689601