W3C home > Mailing lists > Public > ietf-http-wg@w3.org > April to June 2014

Re: Stuck in a train -- reading HTTP/2 draft.

From: Poul-Henning Kamp <phk@phk.freebsd.dk>
Date: Tue, 17 Jun 2014 20:28:08 +0000
To: Martin Thomson <martin.thomson@gmail.com>
cc: Mark Nottingham <mnot@mnot.net>, HTTP Working Group <ietf-http-wg@w3.org>
Message-ID: <1775.1403036888@critter.freebsd.dk>
In message <CABkgnnUeS823DWFY+CdBaKF2GUzc4_QJoRf1RXVcqxgKhN-f7A@mail.gmail.com>
, Martin Thomson writes:

>>> I would split the difference:  A R1 single-bit field, which must
>>> be sent as zero and ignored on reception, and a R2 single-bit field,
>>> which must be sent a zero, and where receiving a one is an error.
>
>For instance, the bits leading up to the length field in the frame
>header could be moved from R1 to R2 under this taxonomy and I'm happy
>to do that if others think that it's a good idea.
>https://github.com/http2/http2-spec/issues/531

Those were the bits I was referring to.

>>> It's also pretty non-obvious why we need CONTINUATION in the
>>> first place:  Why not simply send HEADERS until one of them has
>>> the END_HEADERS bit set ?
>
>http://http2.github.io/faq/#why-the-rules-around-continuation-on-headers-frames

I'm not asking why we need multiple frames, that's painfully obvious
when the cookie mistake is retained.

I'm asking why the frames have different names ?

Rather than

	HEADERS	
	CONTINUATION * N
	CONTINUATION + END_HEADERS

I would do

	HEADERS * N
	HEADERS + END_HEADERS

Since I don't see how CONTINUATION differs from HEADERS in any way
other way than name and type number.

Eliminating one frame type would simplify the RFC text and 
implementations, (Ref: Antoine de Saint Exupéry's wisdom that
"Perfection is finally attained not when there is no longer anything
to add, but when there is no longer anything to take away.")

>>> What happens of SETTINGS_MAX_CONCURRENT_STREAMS gets set to zero
>>> and stays there ?  How long time does the client wait for before
>>> opening a new connection ?
>
>Fact is, zero isn't really all that special.

The operative bit was "and stays there ?"

Shouldn't we at least offer some recommended minimum timeouts ?

>>> Are clients allowed to open multiple connections to bypass the
>>> SETTINGS_MAX_CONCURRENT_STREAMS limit ?
>>>
>>> Are proxies ?
>
>That is a good point.  https://github.com/http2/http2-spec/issues/529
>
>I think that both come down to providing advice around concurrency
>limits.  We need to work out what the messaging here is.  I don't
>think that we want to encourage the use of bypass mechanisms.

I don't think its just a matter of "messaging" I think it is a
very nasty conflict between QoS and DoS mitigation.

If we state up front that clients SHALL NOT have more than one
TCP connection opened to the same destination IP#, then we lay
the ground work for the first layer of DoS mitigation strategy
without seriously inconveniencing any normal clients.

But unless servers somehow detect client side proxies and offer
them a much more generous SETTINGS_MAX_CONCURRENT_STREAMS, service
to clients behind those proxies will suck, in direct proportion
to the number of clients behind the proxy.

The necessary logic to tell a proxy from a client from a hostile
DoS-zombie is neither going to be pretty nor particularly efficient,
since access to the algorithm/source code by definition means you
can circumvent it with artificial traffic.


>>> Page 36: SETTINGS synchronization
>>> ---------------------------------

>That's not true.  You send SETTINGS, you carry on.  You only lose RTTs
>if you want to apply tight limits and then open them again.

99% of those ACKs will be ignored.

I propose that the remaining 1% could get the exact same functionality
by sending SETTINGS + PING and wait for the pong.

That means less wasted bandwidth, less RFC text and less implementation
source code.

>And the explicit acknowledgement allows for different implementation
>strategies.

Using PING to get the acknowledgement and only when the strategy
actually calls for an ack, while saving RFC text and source code
gives you the same flexibility.

>>> Page 39: GOAWAY
>>> ---------------
>>>
>>> I would give the sender the ability to add a small-ish integer
>>> to the Last-Stream-ID, as a way to avoid wasted work.
>>>
>>> Doing that, I think an opening gambit of
>>>
>>>       SETTINGS(MAX_CONNCURRENT_STREAMS = n)
>>>       GOAWAY(Last-Stream-ID = n)
>>>
>>> could be a very efficient DoS mitigation strategy, which would give
>>> legitimate users a chance.  In particular if we allow GOWAY to
>>> update the Last-Stream-ID once the client has proven non-hostile.
>>>
>>> In fact, it's not obvious to me why GOAWAY isn't simply a field
>>> in SETTINGS to begin with ?
>
>Except that GOAWAY explicitly says that stream n+2 is safe to retry
>elsewhere.  How would your proposal here avoid issues with that?

(I'm not sure where you get n+2 from there ?)

Then please add a SETTINGS_LAST_STREAM parameter instead:

	SETTINGS_LAST_STREAM (6):  This setting acts as a quota
	   for the amount of work that will be served.  The peer
	   SHALL NOT attempt to open streams numbered higher than
	   the current setting.

>>> Page 42:  Flow Control Window
>>> -----------------------------
>>>
>>> Again:  Why is WINDOW_UPDATE a separate frame type ?  a field
>>> in SETTINGS would do just fine.
>
>Because you need to be able to advertise increased space.

Nobody has said anywhere that SETTINGS parameters cannot
be relative or incremental values ?

Which reminds me: Has anybody thought about consolidating 
WINDOW_UPDATE ?

As messages they suffer from a pretty big overhead, and I suspect
we will often see concurrent updates to a stream window and
the connection window at the same time.

If we defined a flag on WINDOW_UPDATE to mean that the
per stream increment should also be applied to the
per connection window, we could save 12 bytes and some
processing for that case.

>>> For obvious DoS reasons, it's a mistake to not apply flowcontrol
>>> to HEADERS.
>
>Yes, you can't use flow control as a tool to mitigate DoS that uses
>HEADERS.  I think that we've reached the conclusion that we don't want
>to be using that hammer to mitigate all similar problems.  I know
>that's not a particularly satisfactory answer, but there are a bunch
>of secondary concerns.  In other words, it was not as obvious as you
>might like to think.

I think it's very obvious:  Having retained the cookie mistake
and excempted them from flow-control, you've pretty much defined
how DoS attacks on HTTP/2 will look.

>>> I also think it is a mistake to exempt the 8 byte frame header from
>>> the FCW:  You can fill a lot of pipe with 1 byte DATA frames when
>>> the overhead isn't counted.
>
>This is another place where the flow control window isn't usable.  It
>doesn't bother me a great deal.  It seems like there are bigger holes
>in this fence.

What fence are you referring to now ?

I'm asking because I don't see anything in the protocol that even
remotely looks like a comprehensive built in DoS resistance.

While I admire the candid listing of weaknesses in 10.5, I really
don't think "read the disclaimer" is a very usable attitude to
DoS mitigation.

>>> Page 51: out of order flags
>>> ---------------------------
>
>Yes, we know.  It's probably not worth fixing though.

I beg to differ.

Let me just quote the para I'm objecting to:

   The last frame in the sequence bears an END_STREAM flag, though a
   HEADERS frame bearing the END_STREAM flag can be followed by
   CONTINUATION frames that carry any remaining portions of the header
   block.

How do I know I have received the last CONTINUATION frame if that frame
is not marked by the END_STREAM flag ?

I think it is very much worth fixing this so that the last frame
of the stream and ONLY the last frame of the stream has the END_STREAM
flag set.


>>> Page 51: More details in example
>>> --------------------------------
>>>
>>> The example in 8.1 should show where the END_HEADERS and END_STREAM bits
>>> are set.
>
>It does.

In -12 it mentions neither:

   1.  one HEADERS frame, followed by zero or more CONTINUATION frames
       (containing the message headers; see [HTTP-p1], Section 3.2), and

   2.  zero or more DATA frames (containing the message payload; see
       [HTTP-p1], Section 3.3), and

   3.  optionally, one HEADERS frame, followed by zero or more
       CONTINUATION frames (containing the trailer-part, if present; see
       [HTTP-p1], Section 4.1.2).

>>> Page 51:  To strict ordering ?
>>> ------------------------------
>>>
>>>   Other frames (from any stream) MUST NOT occur between either HEADERS
>>>   frame and the following CONTINUATION frames (if present), nor between
>>>   CONTINUATION frames.
>>>
>>> Isn't this needlessly strict ?  No harm would come from DATA frames
>>> or SETTING frames being stuffed in there.
>>>
>>> All this trouble could be avoided by only submitting headers for
>>> decompression, as a unit, when the END_HEADERS have been received.
>
>That creates a nice state exhaustion/denial of service opportunity
>that we decided not to permit.

I really don't understand that answer:  Buffering the compressed
header will take up less space than buffering the uncompressed
header ?

>>> Page 51: Gibberish
>>> ------------------
>>>
>>> This needs to be translated to human readable form:
>>>
>>>   Otherwise, frames MAY be interspersed on the stream between these
>>>   frames, but those frames do not carry HTTP semantics.  In particular,
>>>   HEADERS frames (and any CONTINUATION frames that follow) other than
>>>   the first and optional last frames in this sequence do not carry HTTP
>>>   semantics.
>
>Improved text gratefully accepted.

I'd happy attempt if I had any idea what it atttempts to say, but I
literally looked a that paragraph for 10 minutes without finding out.

If nobody else knows, you should strike it.

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk@FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.
Received on Tuesday, 17 June 2014 20:30:19 UTC

This archive was generated by hypermail 2.4.0 : Friday, 17 January 2020 17:14:31 UTC