On 10/19/22 11:28 AM, Stefan Eissing via dev wrote:
>
>
>> Am 18.10.2022 um 21:20 schrieb Roy T. Fielding :
>>
>>> On Oct 6, 2022, at 2:17 AM, Stefan Eissing via dev
>>> wrote:
>>>
Am 05.10.2022 um 19:34 schrieb Stefan Eissing via dev
:
> Am 05.10.2022 um 18:48 schrieb Eric Covener :
>
> On Wed, Oct 5, 2022 at 12:44 PM Roy T. Fielding wrote:
>>
>>> On Sep 26, 2022, at 5:29 AM, ic...@apache.org wrote:
>>>
>>> Author: icing
>>> Date: Mon Sep 26 12:29:47 2022
>>> New Revision: 1904269
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1904269=rev
>>> Log:
>>> *) mod_http2: new directive "H2HeaderStrictness" to control the
>>> compliance
>>> level of header checks as defined in the HTTP/2 RFCs. Default is 7540.
>>> 9113 activates the checks for forbidden leading/trailing whitespace in
>>> field values (available from nghttp2 v1.50.0 on).
>>
>> I am not seeing why that should be optional. It adds overhead, but it
>> reduces
>> variability (for HPACK) and might prevent some downstream
>> vulnerabilities, IIRC.
>> Maybe an internal switch for testing with default on?
>>>
>>> To add some more detail:
>>>
>>> - rfc9113 ch 8.2.1 states: "A field value MUST NOT start or end with an
>>> ASCII whitespace character"
>>> - nghttp2 v1.49.0 implemented that check, non-optional. Things broke.
>>> - nghttp2 v1.50.0 added to its API so that the application can control the
>>> behaviour on request of Daniel Stenberg
>>> - I added "H2HeaderStrictness" (in trunk only for now) to steer that.
>>>
>>> This is not a normalization issue, it's a hard "MUST NOT" e.g. rejection of
>>> the request/response carrying such a field. While I agree that there are
>>> many advantages in having fields more strict, the way to get there is not
>>> clear.
>>>
>>> I am totally in favour of avoiding "H2HeaderStrictness", but just enforcing
>>> rfc9113 here will not do. What would our response be for people whose
>>> legacy clients/fronted applications will no longer work?
>>
>> Well, normally, I don't have a problem with just breaking them.
>> They are broken. Someone can fix them.
>>
>> It isn't a normalization issue. Whitespace that magically appeared when h2
>> changed the framing of header field values immediately became a security
>> vulnerability for all downstream recipients of h2/h3 messages that use
>> generic HTTP (semantic) field values expecting that stuff to have
>> been excluded during parsing.
>>
>> IOW, it's a security hole and our code either fixes it or gets a CVE.
>> We MUST NOT forward the malformed data in fields. That is not an option.
>>
>> OTOH, how we deal with a request containing malformed data in fields
>> (after making it well-formed) is a SHOULD send 400, not a MUST.
>> If we want to be super nice to the folks shipping bad code (or running pen
>> testers) and have an option to strip the naughty bits out while forwarding
>> the message, that's fine with me as an optional directive. But that won't
>> help them interop with the rest of the Internet.
>
> Speaking about our implementation, I just added tests to trunk. Configuring
> 'Header add name "$value"' and making http/1.1 requests, curl sees the
> returned headers as:
>
> configured, returned
> ['123 ', '123 '], # trailing space stays
> ['123\t', '123\t'],# trailing tab stays
> [' 123', '123'], # leading space is stripped
> [' 123', '123'], # leading spaces are stripped
> ['\t123', '123'], # leading tab is stripped
>
> Test case 'test_h1_007_02'.
>
> Linking nghttp2 v1.49.0 and newer, without further config, will RST_STREAM
> cases 1 and 2 when using HTTP/2. While succeeding with HTTP/1.1. I would find
> that highly confusing.
Thanks. Initially I thought it was about request headers, but the tests are
about response headers. Does the "no leading/trailing
whitespace" rule apply to both?
>
> I understand that in the definition of Core HTTP, leading/trailing whitespace
> MUST NOT carry any semantics and SHOULD be avoided. If Apache httpd, in its
> version-independent field handling, should decide to universally strip such
> ws, that would be totally fine with me.
>
> The question in implementing mod_http2 is when cases 1+2 should FAIL and when
> we should serve them. If we have any other config I can derive that from, I'd
> happily remove "H2HeaderStrictness" again.
As far as I understand Roy, we should deny by default but have a directive that
allows us to strip leading/trailing whitespaces
from the header values and accept them then (provided they are ok otherwise).
But I would be also fine if we just silently strip the leading/trailing ws
always.
Regards
RĂ¼diger