> Am 18.10.2022 um 21:20 schrieb Roy T. Fielding <field...@gbiv.com>:
>
>> On Oct 6, 2022, at 2:17 AM, Stefan Eissing via dev <dev@httpd.apache.org>
>> wrote:
>>
>>> Am 05.10.2022 um 19:34 schrieb Stefan Eissing via dev
>>> <dev@httpd.apache.org>:
>>>
>>>
>>>
>>>> Am 05.10.2022 um 18:48 schrieb Eric Covener <cove...@gmail.com>:
>>>>
>>>> On Wed, Oct 5, 2022 at 12:44 PM Roy T. Fielding <field...@gbiv.com> 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&view=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.
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.
Cheers,
Stefan
> Cheers,
>
> ....Roy