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