Subject: Re: [quicwg/base-drafts] Specify behavior for post-handshake CRYPTO messages (#2524)

From: Nick Harper <[email-masked]>
Date: Thu, 04 Apr 2019 19:40:37 +0000 (UTC)

@martinthomson Is there anything else I need to do to get this PR 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/2524#issuecomment-480035718

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 04 Apr 2019 17:21:49 -0700

janaiyengar commented on this pull request.

A few comments

> @@ -1438,6 +1438,31 @@ New transport parameters can be registered according to the rules in
 {{iana-transport-parameters}}.

+## Cryptographic Message Buffering
+
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, an endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+
+Implementations MUST support buffering at least 4096 bytes of data received in
+CRYPTO frames out of order. Endpoints MAY choose to allow more data to be
+buffered during the handshake. A larger limit during the handshake could allow
+for larger keys or credentials to be exchanged. An endpoint's buffer size does
+not need to remain constant during the life of the connection.
+
+Being unable to buffer CRYPTO frames during the handshake leads to a connection

```suggestion
Being unable to buffer CRYPTO frames during the handshake can lead to a connection
```

> +potentially force its peer to buffer an unbounded amount of data.
+
+Implementations MUST support buffering at least 4096 bytes of data received in
+CRYPTO frames out of order. Endpoints MAY choose to allow more data to be
+buffered during the handshake. A larger limit during the handshake could allow
+for larger keys or credentials to be exchanged. An endpoint's buffer size does
+not need to remain constant during the life of the connection.
+
+Being unable to buffer CRYPTO frames during the handshake leads to a connection
+failure. If an endpoint's buffer is exceeded during the handshake, it can expand
+its buffer temporarily to complete the handshake. If an endpoint cannot expand
+its buffer, it MUST close the connection with a CRYPTO_BUFFER_EXCEEDED error
+code.
+
+Once the handshake completes, if an endpoint is unable to buffer all data in a
+CRYPTO frame, it MAY discard all subsequent CRYPTO frames, or it MAY close the

This isn't clear to me. If the endpoint is unable to buffer data in a CRYPTO frame, why does it need to drop _subsequent_ CRYPTO frames?

> +CRYPTO frames out of order. Endpoints MAY choose to allow more data to be
+buffered during the handshake. A larger limit during the handshake could allow
+for larger keys or credentials to be exchanged. An endpoint's buffer size does
+not need to remain constant during the life of the connection.
+
+Being unable to buffer CRYPTO frames during the handshake leads to a connection
+failure. If an endpoint's buffer is exceeded during the handshake, it can expand
+its buffer temporarily to complete the handshake. If an endpoint cannot expand
+its buffer, it MUST close the connection with a CRYPTO_BUFFER_EXCEEDED error
+code.
+
+Once the handshake completes, if an endpoint is unable to buffer all data in a
+CRYPTO frame, it MAY discard all subsequent CRYPTO frames, or it MAY close the
+connection with an CRYPTO_BUFFER_EXCEEDED error code. If an endpoint chooses to
+discard all subsequent CRYPTO frames, the packets containing these CRYPTO frames
+MUST be acknowledged.

The endpoint MUST ack frames that are dropped? I don't think I follow 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/2524#pullrequestreview-223058169

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 04 Apr 2019 17:45:18 -0700

kazuho commented on this pull request.

+

Because TLS handshake messages require a stream that guarantees in-order delivery. Once you discard some bytes, you cannot use subsequent data on the stream. Therefore, the only way to keep the connection open after discarding some bytes is to discard all the following bytes, while sending ACKs.

-- 
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/2524#discussion_r272415300

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 04 Apr 2019 17:46:12 -0700

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/2524#pullrequestreview-223062779

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 04 Apr 2019 22:10:08 -0700

+

Ah, ok, thanks @kazuho.

-- 
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/2524#discussion_r272445844

---

From: ianswett <[email-masked]>
Date: Tue, 16 Apr 2019 09:11:53 -0700

ianswett commented on this pull request.

They were received and processed at the transport layer, so this makes sense.

How about "Packets containing discarded CRYPTO frames MUST be acknowledged, because they were received and processed by the transport, even though they were discarded prior to the crypto stack processing 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/2524#discussion_r275879404

---

From: Nick Harper <[email-masked]>
Date: Tue, 16 Apr 2019 14:09:17 -0700

nharper commented on this pull request.

I tweaked the wording to be similar to 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/2524#discussion_r275993681

---

From: ianswett <[email-masked]>
Date: Tue, 16 Apr 2019 21:12:17 +0000 (UTC)

ianswett 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/2524#pullrequestreview-227447469

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 16 Apr 2019 15:28:11 -0700

@janaiyengar pushed 1 commit.

951f7115f45e70e27b6dea951c45046e800344fa  nits

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2524/files/4a3ff179f1e70d581bcd14ed519b8c52e071dd4c..951f7115f45e70e27b6dea951c45046e800344fa

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 16 Apr 2019 22:28:15 +0000 (UTC)

janaiyengar 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/2524#pullrequestreview-227475771

---

From: Mike Bishop <[email-masked]>
Date: Thu, 18 Apr 2019 16:02:41 -0700

MikeBishop commented on this pull request.

Minor nits.

> +potentially force its peer to buffer an unbounded amount of data.
+
+Implementations MUST support buffering at least 4096 bytes of data received out
+of order in CRYPTO frames. Endpoints MAY choose to allow more data to be
+buffered during the handshake. A larger limit during the handshake could allow
+for larger keys or credentials to be exchanged. An endpoint's buffer size does
+not need to remain constant during the life of a connection.
+
+Being unable to buffer CRYPTO frames during the handshake can lead to a
+connection failure. If its buffer is exceeded during the handshake, an endpoint
+can temporarily expand its buffer to complete the handshake. If an endpoint
+cannot expand its buffer, it MUST close the connection with a
+CRYPTO_BUFFER_EXCEEDED error code.
+
+Once the handshake completes, if an endpoint is unable to buffer all data in a
+CRYPTO frame, it MAY discard all subsequent CRYPTO frames, or it MAY close the

"subsequent" here could mean further in the CRYPTO stream, or could mean received later in time.  If the latter, then the currently-buffered crypto data will never be delivered and can also be discarded; we should say so.  In either case, we should be specific about which "subsequent" we mean.

+
+Implementations need to maintain a buffer of CRYPTO data received out of order.
+Because there is no flow control of CRYPTO frames, an endpoint could
+potentially force its peer to buffer an unbounded amount of data.
+
+Implementations MUST support buffering at least 4096 bytes of data received out
+of order in CRYPTO frames. Endpoints MAY choose to allow more data to be
+buffered during the handshake. A larger limit during the handshake could allow
+for larger keys or credentials to be exchanged. An endpoint's buffer size does
+not need to remain constant during the life of a connection.
+
+Being unable to buffer CRYPTO frames during the handshake can lead to a
+connection failure. If its buffer is exceeded during the handshake, an endpoint
+can temporarily expand its buffer to complete the handshake. If an endpoint
+cannot expand its buffer, it MUST close the connection with a

"cannot" implies ability, when the endpoint might simply not want to.  Perhaps better to say "does not" expand?

-- 
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/2524#pullrequestreview-228541205

---

From: Nick Harper <[email-masked]>
Date: Fri, 19 Apr 2019 14:57:51 -0700

+

I've changed this to "does not".

+

I didn't consider it potentially meaning further in the CRYPTO stream (I was thinking later in time). I've clarified that it means later in time.

Previously I had added language to state that if CRYPTO frames are being discarded, then the endpoint can tear down all crypto state that it has. I removed that since @martinthomson pointed out that's an obvious thing to do. I'm thinking that discarding any buffered but not processed crypto data falls under tearing down crypto state. If this no longer seems obvious, I'll add back that the endpoint can tear down crypto state and discard any remaining buffered crypto data.

-- 
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/2524#discussion_r277099061

---

From: Martin Thomson <[email-masked]>
Date: Tue, 23 Apr 2019 15:15:25 -0700

Merged #2524 into master.

-- 
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/2524#event-2295515926