Subject: [quicwg/base-drafts] Limit fallout of on-stream badness (#3336)

From: Mike Bishop <[email-masked]>
Date: Mon, 13 Jan 2020 13:07:18 -0800

Fixes #3300; closes #3303.

Kazuho&#39;s point is that individual streams might be relayed by the peer from connections to other parties (multiple origins for a server, multiple user agents for a client).  If that peer isn&#39;t checking stream contents, they might unintentionally relay something bad; this shouldn&#39;t be required to blow up the connection for the other parties (though that&#39;s still a permitted and reasonable implementation).

This change says that the payload of the request stream never mandates a connection error; contents of the control stream still do.

If we want to relax this, I think this is the most sensible way.  That &quot;if&quot; is still an open question.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Limit fallout of on-stream badness

-- File Changes --

    M draft-ietf-quic-http.md (26)

-- Patch Links --

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

---

From: Kazuho Oku <[email-masked]>
Date: Mon, 13 Jan 2020 20:18:54 -0800

kazuho approved this pull request.

The changes look good, in sense that it aligns the scope of frame-level errors that happen on the request stream or the push stream to that of H2.

-- 
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/3336#pullrequestreview-342283977

---

From: ianswett <[email-masked]>
Date: Tue, 14 Jan 2020 13:20:25 -0800

Thanks for writing this PR, seeing it written down makes me like it clear that I'm not a fan of this change.

One can certainly implement a proxy that doesn't validate the frames, but for this proxy to receive a single H3 connection on the front and send multiple H3 connections out the back, it's going to have to do more than blindly proxy.  For example, there's only one control stream, and I can't imagine how it's possible to run QPACK anything but hop-by-hop.  It also has to map push Ids, etc.

If there's some practical use case I'm missing, please tell me, but in most cases, blindly proxying these frames wouldn't even work, with the DATA frame being the possible exception?

-- 
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/3336#issuecomment-574379204

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 Jan 2020 13:39:15 -0800

If we agree that frames can never be tunneled, we could trim this back to only malformed headers and leave frame errors as being connection-fatal always.  (Though in fairness, if you set your QPACK table size to zero, you could proxy the HEADERS frames directly.)

-- 
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/3336#issuecomment-574386256

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 Jan 2020 13:43:11 -0800

Correction:  Malformed headers are already stream errors, so the "trim back" would be to close the PRs entirely.

-- 
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/3336#issuecomment-574387870

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 14 Jan 2020 15:34:05 -0800

There still is a problem that we can't restrict what a client ultimately does. If we could describe the cost of killing a connection, instead of _requiring_ limiting the endpoint response, that might go a long way.

-- 
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/3336#issuecomment-574424129

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 14 Jan 2020 15:47:22 -0800

LPardue requested changes on this pull request.

Where is the "Disapprove" button? :)

I previously argued for relaxation of a connection error related to reducing Push ID limits. The argument against relaxation was that the client (in this case) has all required information to prevent such an occurence. 

This proposal seems advantagous but I'd say it makes the protocol too meek. Especially control frames on request streams. For example, a server that can wants to operate this way will accept bogus frames on a client's requests streams, so it will probably accept a request and then abort the stream causing the client to be starved (and eventually timeout?). Attack of the zombie H3 connections. 

-- 
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/3336#pullrequestreview-342913018

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 14 Jan 2020 15:47:33 -0800

@MikeBishop 
> Malformed headers are already stream errors, so the "trim back" would be to close the PRs entirely.

If our principle is to require H3 endpoints to do their own framing, I think we should go one step further than just closing this PR.

We should stop stating that existence of extraneous frames is considered a malformed request / response (see https://quicwg.org/base-drafts/draft-ietf-quic-http.html#section-4.1.3), and that it's a connection error.

I am perfectly fine with having more rigid rules for frames, as it is a hop-by-hop concept. That's very different from header fields which are end-to-end and also might convey information that cannot be understood by an H3 endpoint sitting at the middle.

-- 
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/3336#issuecomment-574427942

---

From: Mike Bishop <[email-masked]>
Date: Tue, 14 Jan 2020 15:58:39 -0800

The "extraneous frames" might be separable, or at least made more specific....  Unknown frames are already allowed:
> Frames of unknown types (Section 9), including reserved frames (Section 7.2.9) MAY be sent on a request or push stream before, after, or interleaved with other frames described in this section.

The main thing that will make you blow up is DATA frame before HEADERS, or multiple sets of HEADERS after the end of the DATA.  I'm amenable to the idea that should be a connection error instead.

-- 
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/3336#issuecomment-574430943

---

From: Martin Thomson <[email-masked]>
Date: Wed, 15 Jan 2020 15:30:13 -0800

I think that this a reasonable compromise, even if I'm not 100% happy with it.

Part of what we are attempting to do here is establish what the common intersection is between implementations.  Some tolerance for errors in streams is awful, but it seems inevitable that there will be pressures to be tolerant that have more force than anything we might define here.  Maybe that's a little too cynical, but there you go.

If you accept that thesis, it doesn't necessarily follow that errors will remain hidden or go unpunished (if punitive measures are your thing).  All we need is some non-negligible portion of the deployed base, as encountered by new implementations, both client and server, to make these stream errors more visible in some way.

-- 
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/3336#issuecomment-574905887

---

From: Mike Bishop <[email-masked]>
Date: Thu, 16 Jan 2020 11:10:30 -0800

> I think that this a reasonable compromise, even if I'm not 100% happy with it.

To be clear, is "this" the discussion @kazuho and I were having, or the contents of this PR?

-- 
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/3336#issuecomment-575299539

---

From: Mike Bishop <[email-masked]>
Date: Thu, 16 Jan 2020 12:14:08 -0800

Closed #3336 via #3344.

-- 
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/3336#event-2955916823

---

From: Martin Thomson <[email-masked]>
Date: Thu, 16 Jan 2020 16:20:36 -0800

The contents of the PR.

-- 
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/3336#issuecomment-575408701