Re: svn commit: r1904269 - in /httpd/httpd/trunk: changes-entries/ docs/manual/mod/ modules/http2/ test/modules/http2/

2022-10-05 Thread 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?
> 
> +1 for opt-out.

People downgraded nghttp2 v1.49 where this was on by default because of various 
interop problems.

I am all for strictness, but this improvement in the rfc seems to have thorns 
for users.

- Stefan

Re: svn commit: r1904269 - in /httpd/httpd/trunk: changes-entries/ docs/manual/mod/ modules/http2/ test/modules/http2/

2022-10-05 Thread 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?

+1 for opt-out.


Re: svn commit: r1904269 - in /httpd/httpd/trunk: changes-entries/ docs/manual/mod/ modules/http2/ test/modules/http2/

2022-10-05 Thread Roy T. Fielding
> 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?

Roy



Escaping double quotation marks in the error log

2022-10-05 Thread Rainer Jung

Hi there,

I looked at our escaping functions for logs due to the need of doing 
JSON logging. In principle one can output JSON by using appropriate log 
format definitions in the httpd config. Most special characters in JSON 
are already properly escaped in our output.


But there is one important difference between the escaping of access log 
items and of error log items. The escaping function for the access log 
also escapes double quotation marks as \", the one for the error log 
does not (ap_escape_errorlog_item). It contains the comment "no need for 
this in error log". This is true all the way since the time the escaping 
was introduced at all.


I wonder, whether there is a real necessity for not escaping double 
quotation marks in the error log?


Note, that I am not talking about markup that appears in the definition 
of the ErrorLogFormat itself. This is "just" about double quotation 
marks showing up in the error message itself (or logged headers, env 
vars or notes).


Unfortunately the use of ap_escape_errorlog_item() is buried deep down 
in the code levels and accessing virtual host config seems not possible, 
not even in the callers of ap_escape_errorlog_item().


If there's no other nice idea, I wonder whether:

- it would be OK, to escape double quotation marks as \" in the error 
log for trunk


- add a global config item to do this as well for 2.4.x (default off)

What do you think? Or did you find a better solution for error log json 
logging?


Best regards,

Rainer