On 10/19/22 11:28 AM, Stefan Eissing via dev wrote:
>
>
>> 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.
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
