Re: More on allowed field characters

On Thu, Aug 26, 2021 at 03:51:32PM +1000, Martin Thomson wrote:
> On Mon, Aug 23, 2021, at 15:03, Martin Thomson wrote:
> > It seems like the allowed characters in fields is a gift that keeps on giving.
> 
> Thanks everyone for all the words you gave.
> 
> Based on feedback from Willy and Greg in particular, I've taken another go at this:
> 
>   https://github.com/httpwg/http2-spec/pull/936/files

Thanks Martin, as discussed, I'm overall OK with it (though I'd like to
add some guidance regarding the risks caused by pseudo-hedaer fields
reassembly).

> It says that:
> 
> * fields SHOULD be validated properly (according to HTTP §5.1 and §5.5)
> 
> * failure to validate fields might enable attacks, especially if the message
> ends up in HTTP/1.1 somehow (that is, providing motivation that was lacking
> from previous iterations on this)
> 
> * if fields aren't fully validated, attacks might happen, so minimal
> validation MUST be performed (with the checks previously agreed)
> 
> This does not address Roy's original point directly.  Yes, code that makes
> assumptions without taking responsibility for checking them might be exposed
> to the full consequences of poor decisions.  However, I believe that a lot of
> implementations will abide by the SHOULD here.  This is about levying
> requirements on implementations that might have expected to avoid having to
> validate fields; because we've learned that copying and pasting without
> checking happens.

I'm fine with tunnels or H2-H2 gateways blindly forwarding HEADERS frames
without verifying anything there. They're just taking the risk that their
stream or connection is killed on the next hop by violating some SHOULD or
MUST. The risk really is huge when H2 is implemented as a second protocol
next to H1 because known-reliable code is reused (which is better than to
reinvent new stuff and insert new bugs) but this code wasn't necessarily
designed to be exposed to certain delimiting characters that were previously
filtered out by the parser in H1. This is why I find this additional warnings
useful. And pseudo-headers can strike even when H2 is alone but reassembles
a start line that is processed by regular code.

> (I do worry that this is an overreaction.  The original text in the spec was
> arguably fine.  It was just being ignored.)

Yep, at least with the extra words, there's no more excuse for overlooking
this aspect. It's no more just a definition of what is called a header field
but there are some instructions and warnings around that. But I too was
concerned by the previous wording leading to Roy's reaction that could also
be read as "you're allowed to use all this in H2, make your choice".

Thanks,
Willy

Received on Thursday, 26 August 2021 06:51:12 UTC