Subject: [quicwg/base-drafts] Rationalize stream 0 flow control (#1083)

From: Martin Thomson <[email-masked]>
Date: Mon, 29 Jan 2018 23:23:18 +0000 (UTC)

This leaves the exemption to the stream-level flow control for Initial and
Retry packets only (previously it was Initial only).  But, to address the
possibility that the flow control limit is too small for the entire handshake
it permits the sending of MAX_STREAM_DATA (and STREAM_BLOCKED) during the
handshake.

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

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

-- Commit Summary --

  * Rationalize stream 0 flow control

-- File Changes --

    M draft-ietf-quic-tls.md (18)
    M draft-ietf-quic-transport.md (29)

-- Patch Links --

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

---

From: Mike Bishop <[email-masked]>
Date: Mon, 29 Jan 2018 23:40:43 +0000 (UTC)

MikeBishop commented on this pull request.

I like my solution better.  😉

> +connection-level data limits established by MAX_DATA.  That is, data sent on
+stream 0 is not counted against the limit expressed by MAX_DATA.  Data on stream
+0 is still subject to stream-level data limits and MAX_STREAM_DATA.
+
+Data sent in the Initial and Retry packets do not count toward stream 0 flow
+control limits.  These packet types do not permit the sending of data in
+multiple packets, so there is no opportunity for a peer to send MAX_STREAM_DATA
+frames.  Note also that the next packet sent after either of these packets might
+start at a stream offset of 0.  Thus, the only limit on the size of the
+cryptographic handshake messages these packets contain is determined by the MTU.
+
+Endpoints MUST respect the stream flow control limit set by their peer.  A
+client might exceed this limit when sending an Initial packet and so might need
+to await a MAX_STREAM_DATA frame from the server before it can sent additional
+STREAM frames.  A client MAY send ACK frames and STREAM_BLOCKED frames for
+stream 0.

Given that ACK frames don't reference a stream, the idea of an ACK frame for stream 0 seems undefined.  If that phrase wasn't supposed to apply to the ACK frames as well, then I'm having trouble parsing it.

> +the data limits set by its peer.
+
+The cryptographic handshake stream, Stream 0, is exempt from the
+connection-level data limits established by MAX_DATA.  That is, data sent on
+stream 0 is not counted against the limit expressed by MAX_DATA.  Data on stream
+0 is still subject to stream-level data limits and MAX_STREAM_DATA.
+
+Data sent in the Initial and Retry packets do not count toward stream 0 flow
+control limits.  These packet types do not permit the sending of data in
+multiple packets, so there is no opportunity for a peer to send MAX_STREAM_DATA
+frames.  Note also that the next packet sent after either of these packets might
+start at a stream offset of 0.  Thus, the only limit on the size of the
+cryptographic handshake messages these packets contain is determined by the MTU.
+
+Endpoints MUST respect the stream flow control limit set by their peer.  A
+client might exceed this limit when sending an Initial packet and so might need

Actually, no, they can't -- ClientHello has to fit in the single Initial packet, which you say on 3280 doesn't count against this limit, so who cares?

A client might exceed this limit in a Handshake packet, if the handshake takes more than 1 RTT (or the Client Finished is huge for some reason, I suppose), though.  Is that what you're after?

> -from flow control because it needs to be sent in a single packet regardless of
-the server's flow control state. This rule applies even for 0-RTT handshakes
-where the remembered value of MAX_STREAM_DATA would not permit sending a full
-initial cryptographic handshake message.
+the data limits set by its peer.
+
+The cryptographic handshake stream, Stream 0, is exempt from the
+connection-level data limits established by MAX_DATA.  That is, data sent on
+stream 0 is not counted against the limit expressed by MAX_DATA.  Data on stream
+0 is still subject to stream-level data limits and MAX_STREAM_DATA.
+
+Data sent in the Initial and Retry packets do not count toward stream 0 flow
+control limits.  These packet types do not permit the sending of data in
+multiple packets, so there is no opportunity for a peer to send MAX_STREAM_DATA
+frames.  Note also that the next packet sent after either of these packets might
+start at a stream offset of 0.  Thus, the only limit on the size of the

Hm.  Retry is fine, because you're resetting the transport in between, including all the stream states and contents.

But Initial?  You'll have different STREAM frames from different packets identifying the range [0,length) on stream 0 containing different data, and the logic to drop duplicate data needs to *not* drop it on stream 0?  Ick.

If this is the direction you want to go, I'd almost rather see a custom Handshake frame type and let 0 go back to being normal and application-owned.  This direction feels really weird.

-Though data is exchanged on stream 0, the initial flow control window on that
-stream is sufficiently large to allow the TLS handshake to complete.  This
-limits the maximum size of the TLS handshake and would prevent a server or
-client from using an abnormally large certificate chain.
-
-Stream 0 is exempt from the connection-level flow control window.
-
-Consequently, there is no need to signal being blocked on flow control.
-
-Similarly, there is no need to increase the number of allowed streams until the
-handshake completes.
+`MAX_STREAM_DATA` and `STREAM_BLOCKED` frames MUST NOT be sent unprotected
+unless they identify stream 0.  Stream 0 is exempt from the connection-level
+flow control window and stream limits.

But isn't the whole problem here that tampering with these packets can't be detected?  An attacker could cause one side to believe that it had a huge flow control window on stream 0, which could then cause the connection to blow up at some future point if they attempted to send to that limit.  Subsequent, lower MAX_STREAM_DATA frames would be ignored, because they don't increase the largest-seen 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/1083#pullrequestreview-92396174

---

From: Martin Thomson <[email-masked]>
Date: Mon, 29 Jan 2018 15:43:11 -0800

martinthomson commented on this pull request.

Yeah, I'm prepared to acknowledge that as a limitation.  Perhaps we could say that the flow control limit is reset to the lower of the initial limit and the amount used once the handshake completes.

F**c I hate stream 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/1083#discussion_r164600571

---

From: Martin Thomson <[email-masked]>
Date: Mon, 29 Jan 2018 23:54:40 +0000 (UTC)

@MikeBishop, if your solution had any chance of working, I would prefer it also.  But your arbitrary limit cannot work.

The client should only ever send the same data in its Initial packet, so I'm not following your point.  I'm not saying that the Handshake that follows an Initial starts at offset 0 - just that another Initial might follow.

I can see the point for a Retry, in which case both peers will forget (or ignore as invalid) the first set of Initial packets.  There's a nasty gotcha there (#1085), but that probably isn't what you are talking about here.

+

I should delete this sentence, I don't know where it came from.

Yes, I was looking for flow control for client certificates.

These are two separate things:

* ACK frames
* STREAM_BLOCKED frames for stream 0.

Don't know how to make that clearer, since - as you say - your interpretation is nonsensical.

-- 
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/1083#pullrequestreview-92400264

---

From: Mike Bishop <[email-masked]>
Date: Tue, 30 Jan 2018 00:10:31 +0000 (UTC)

Ah, I see what you're getting at.  You're trying to warn that, if there's a Retry, the First Initial and Second Initial will both contain ClientHellos at offset zero, and Retry and the server's Handshake will both contain data starting at offset 0.  I got a totally different interpretation of what you wrote, clearly.

I'd either drop the note, or simply point to the section on Retry with a reminder that the transport state, including stream contents, gets reset following a Retry.

+

I think if you change Initial to Handshake, this is fine.

The statement that "A client MAY send ACK frames" is undoubtedly true, but feels like a bit of a non sequitur without some additional connection to the rest of the paragraph.  Perhaps, "A client in this state SHOULD send a STREAM_BLOCKED frame for stream 0 in a Handshake packet.  This packet MAY also include ACK frames."

-- 
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/1083#discussion_r164604112

---

From: Mike Bishop <[email-masked]>
Date: Tue, 30 Jan 2018 00:15:27 +0000 (UTC)

+

Oh, and this paragraph isn't just clients, which is the point of #725.  Either side might cap out on Handshake packets and get into this state.  So maybe merge this with what I proposed for the following sentence:  "An endpoint which is flow-control blocked on stream 0 SHOULD send...."

-- 
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/1083#discussion_r164606084

---

From: Martin Thomson <[email-masked]>
Date: Tue, 30 Jan 2018 00:18:55 +0000 (UTC)

@martinthomson pushed 1 commit.

0bf027a  Updates in response to review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1083/files/adada506b8570c735f43ece64a369fb8a3c8ba3b..0bf027a92c0bda6597bd68e7eff47e7dbfb5d939

---

From: Martin Thomson <[email-masked]>
Date: Mon, 29 Jan 2018 16:19:41 -0800

Yeah, I reworded.

+

Yes, I noticed that when reworking this.

Yeah, I removed ACK.

-- 
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/1083#discussion_r164606686

---

From: Mike Bishop <[email-masked]>
Date: Tue, 30 Jan 2018 00:33:42 +0000 (UTC)

Better.  I don't think this addresses #1074 yet.  (Clarifying that data on 0 doesn't count toward the connection sum.)  I can extract that portion of mine as a separate PR, or you can lift the relevant text into {{flow-control}}.

> @@ -3374,6 +3367,36 @@ received on all streams, which are used to check for flow control violations. A
 receiver might use a sum of bytes consumed on all contributing streams to
 determine the maximum data limit to be advertised.
 
+
+### Stream 0 Flow Control
+

I'm still a little bothered by "do not count," because that gives us the on-stream version of #1074.  Maybe say "is allowed to temporarily exceed stream 0's flow control limits"?  Otherwise, it looks like the flow control bound is separated from the stream offset, which isn't what you're going for.

+The cryptographic handshake stream, Stream 0, is exempt from the
+connection-level data limits established by MAX_DATA.  That is, data sent on
+stream 0 is not counted against the limit expressed by MAX_DATA.  Data on stream
+0 is still subject to stream-level data limits and MAX_STREAM_DATA.
+
+Data sent in the Initial and Retry packets do not count toward stream 0 flow
+control limits.  These packet types do not permit the sending of data in
+multiple packets, so there is no opportunity for a peer to send MAX_STREAM_DATA
+frames.  Note that sending a Retry causes stream offsets and flow control to be
+reset for subsequent packets.  Thus, the only limit on the size of the
+cryptographic handshake messages these packets contain is determined by the MTU.
+
+Endpoints MUST respect the stream flow control limit set by their peer when
+sending STREAM frames for stream 0 in packets other than Initial or Retry.  A
+client might exceed stream flow control limits when sending an Initial packet

Better.  A server can still fall into the same state, though.  Maybe just say "An endpoint might have reached or exceeded stream flow control limits when sending an Initial or Handshake packet and so...."

-- 
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/1083#pullrequestreview-92406483

---

From: Mike Bishop <[email-masked]>
Date: Mon, 29 Jan 2018 16:35:25 -0800

Yeah, me too.  That flow control reset idea sounds unpleasant, but probably the best we can do if we have to tolerate window updates in unprotected packets.

-- 
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/1083#discussion_r164609379

---

From: Martin Thomson <[email-masked]>
Date: Tue, 30 Jan 2018 01:00:14 +0000 (UTC)

@martinthomson pushed 2 commits.

2cf3d53  Contributing streams
13ab8ad  More editorial feedback

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1083/files/0bf027a92c0bda6597bd68e7eff47e7dbfb5d939..13ab8ad2eb4e9b38a8bcea6cfb3c373f3fefbd31