Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line?

[Ruediger Pluem]

On 6/18/20 8:55 PM, Roy T. Fielding wrote:
>> On Jun 18, 2020, at 9:03 AM, Stefan Eissing > > wrote:
>>> Am 18.06.2020 um 16:51 schrieb William A Rowe Jr >> >:
>>>
>>>
>>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem  > wrote:
>
> I came across the question if we should not reject HTTP protocols >= 
> 2.0 in the request line when we parse it
> in ap_parse_request_line.
> This does not affect mod_http2 if loaded as HTTP/2.0 connections 
> itself are not parsed via ap_parse_request_line
> and sending a
>
> GET /something HTTP/2.0
>
> as request line is not a valid way to start a HTTP 2.0 connection and 
> I doubt that it will be for future major versions.
>>>
>>> Correct, it starts an HTTP/1.1 connection, and the response should reflect 
>>> HTTP/1.1.
>>>
 That isn't how these things typically work. New protocols are
 advanced with either deliberate backwards-compat or deliberate
 backwards-break, with an expectation that it will either do
 something useful on an older-protocol server or cause a safe
 error in an expected way.

 Hence, we might still see an HTTP/4.0 that is designed to be
 parsed like HTTP/1.1 (by an old server) while at the same time
 work perfectly for a new server. That would be some hefty magic,
 but it remains possible. Likewise, we might want to deploy a
 version of h2 or HTTP/3 that works on unix domain sockets or
 localhost over non-Internet TCP.

 This is why the existing code did not error on protocols >= 2.0.
 Doing so is both unnecessary and counterproductive. If parsing
 fails for some other reason, we want that other reason to be
 in the response (because that's what the new protocol will be
 expecting from an old protocol server). If it doesn't fail, we
 want to provide the successful response because the request
 was deliberately crafted that way to save a round trip.

 Note that the incoming request protocol version should always
 be distinct from any forwarded request protocol or response
 protocol versions.
>>>
>>> Precisely. If mod_http2 or quic/mod_http3 can do something with the 
>>> connection
>>> based on the request line, it's up to them through the hook facility to 
>>> take ownership
>>> of the connection.

What I get out of all of this is the following:

If we get a

METHOD /somepath HTTP/x.y

request line on the wire.

We could return a 505 (Version not supported) if we know that we do not support 
that major version x (e.g. HTTP/3
as mentioned below seem to be finished soon, but we do not have support for it 
yet). The 505 answer would be a
HTTP/1.1 response.
But we could also accept this request as long as it is a valid HTTP/1.1 request
and just respond with the desired HTTP/1.1 response for this resource as if 
this was sent with HTTP/1.1.
If it violates the HTTP/1.1 syntax somehow we would respond with the same 
status code we use for this case
on a HTTP/1.1 request. That second approach is IMHO the behavior we had before 
r1878708.

Provided that my above understanding is correct I see no real benefit any 
longer in returning a 505 and I would
revert r1878708 and all the follow ups (from r1878926 only the changes to 
modules/http2/h2_request.c and probably CHANGES as the
remaining changes are no functional changes to my understanding).

The positive thing I get out of this is that it revealed that we probably 
should think about splitting some functions
that do some version agnostic HTTP processing and some HTTP/1.1 specific 
processing in one place to make this distinction more
clear and the version agnostic HTTP processing code more reusable.

Many thanks to all for this constructive and focused discussion that taught me 
new things.

>>
>> That is not issue. That works well. 
>>
>>> If they cannot/do not, then the core http1 connection/request processors 
>>> remain
>>> in place and in response to "please speak in HTTP/4.0" this server will 
>>> respond,
>>> "sure, here is your HTTP/1.1 response" as expected and defined by the RFC.
>>
>> There are now several RFCs and they differentiate between HTTP/1.1 transport 
>> and
>> the pure HTTP semantics. This split is not reflected in our code, yet. We 
>> have 
>> functions that mix both. Not as a mistake, it's historical.
>>
>> ap_parse_request_line() for example, checks the initial HTTP/1.1 request 
>> line *and*
>> the method names, uri, header_only and other request_rec fields.
>>
>> We can either copy the latter into mod_http2 and maintain it in two places or
>> have a core function for it to be invoked by mod_http and mod_http2. That 
>> seems 
>> to be the design 

Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line?

[Roy T. Fielding]

> On Jun 18, 2020, at 9:03 AM, Stefan Eissing  
> wrote:
>> Am 18.06.2020 um 16:51 schrieb William A Rowe Jr :
>> 
>> 
>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
 On Jun 8, 2020, at 12:56 AM, Ruediger Pluem  wrote:
 
 I came across the question if we should not reject HTTP protocols >= 
 2.0 in the request line when we parse it
 in ap_parse_request_line.
 This does not affect mod_http2 if loaded as HTTP/2.0 connections 
 itself are not parsed via ap_parse_request_line
 and sending a
 
 GET /something HTTP/2.0
 
 as request line is not a valid way to start a HTTP 2.0 connection and 
 I doubt that it will be for future major versions.
>> 
>> Correct, it starts an HTTP/1.1 connection, and the response should reflect 
>> HTTP/1.1.
>> 
>>> That isn't how these things typically work. New protocols are
>>> advanced with either deliberate backwards-compat or deliberate
>>> backwards-break, with an expectation that it will either do
>>> something useful on an older-protocol server or cause a safe
>>> error in an expected way.
>>> 
>>> Hence, we might still see an HTTP/4.0 that is designed to be
>>> parsed like HTTP/1.1 (by an old server) while at the same time
>>> work perfectly for a new server. That would be some hefty magic,
>>> but it remains possible. Likewise, we might want to deploy a
>>> version of h2 or HTTP/3 that works on unix domain sockets or
>>> localhost over non-Internet TCP.
>>> 
>>> This is why the existing code did not error on protocols >= 2.0.
>>> Doing so is both unnecessary and counterproductive. If parsing
>>> fails for some other reason, we want that other reason to be
>>> in the response (because that's what the new protocol will be
>>> expecting from an old protocol server). If it doesn't fail, we
>>> want to provide the successful response because the request
>>> was deliberately crafted that way to save a round trip.
>>> 
>>> Note that the incoming request protocol version should always
>>> be distinct from any forwarded request protocol or response
>>> protocol versions.
>> 
>> Precisely. If mod_http2 or quic/mod_http3 can do something with the 
>> connection
>> based on the request line, it's up to them through the hook facility to take 
>> ownership
>> of the connection.
> 
> That is not issue. That works well. 
> 
>> If they cannot/do not, then the core http1 connection/request processors 
>> remain
>> in place and in response to "please speak in HTTP/4.0" this server will 
>> respond,
>> "sure, here is your HTTP/1.1 response" as expected and defined by the RFC.
> 
> There are now several RFCs and they differentiate between HTTP/1.1 transport 
> and
> the pure HTTP semantics. This split is not reflected in our code, yet. We 
> have 
> functions that mix both. Not as a mistake, it's historical.
> 
> ap_parse_request_line() for example, checks the initial HTTP/1.1 request line 
> *and*
> the method names, uri, header_only and other request_rec fields.
> 
> We can either copy the latter into mod_http2 and maintain it in two places or
> have a core function for it to be invoked by mod_http and mod_http2. That 
> seems 
> to be the design decision to make.

I think that is the right way forward, though we have always tried
to minimize overhead for the common case. I guess the extra cycles
are irrelevant now.

> I used ap_parse_request_line() from mod_http2 to *not* duplicate the other 
> code,
> and someone added checks in trunk that imply it only ever gets called for 
> HTTP/1.x.
> So, now it pukes. Which is good, as it brought up this discussion.

Yep. The new RFCs should be "finished" by next month due to HTTP/3
being in WGLC this month.  If anyone's interested:

   https://github.com/httpwg/http-core 
   https://github.com/quicwg/base-drafts 

...Roy

Re: hardening mod_write and mod_proxy like mod_jk with servletnormalize

[jean-frederic clere]

On 17/06/2020 13:26, Yann Ylavic wrote:

On Sat, Jun 13, 2020 at 11:18 AM jean-frederic clere  wrote:


On 11/06/2020 13:50, Yann Ylavic wrote:

On Thu, Jun 11, 2020 at 1:22 PM Yann Ylavic  wrote:


On Thu, Jun 11, 2020 at 9:57 AM Yann Ylavic  wrote:


On Thu, Jun 11, 2020 at 9:50 AM Yann Ylavic  wrote:


We need a way to forward non %-decoded URLs upto mod_proxy (reverse)
if we want to normalize a second time..


IOW, this block in ap_process_request_internal():

[snip]

Should go _after_ the following:

[snip]

Or we could introduce a new pre_translate_name hook which would
execute before %-decoding, and be used by mod_proxy when
"ProxyPreTranslation on" is configured, and be a prerequisite for
mapping=servlet.

I find ProxyPreTranslation also useful for the non-servlet case btw.

Something like this attached v2 patch.


Here is a v3 with the relevant pre_translate_name hooks only and
ap_getparents() preserved when the URI does not start with '/' (which
makes the patch read better too).


with this patch, how to I get:
curl -v --path-as-is "http://localhost:8000/docs/..;food=bar/test/index.jsp

Mapped to
ProxyPass  /test ajp://localhost:8009/test secret=%A1b2!@
Or rejected in case I have only:
ProxyPass  /docs ajp://localhost:8009/docs secret=%A1b2!@


Right sorry, it does not work with patch v3, I mainly focused on the
"decode at the right place" part of the issue, which is not your
point..

I just staged a more complete proposal in
https://github.com/apache/httpd/pull/128

For the proxy servlet part, I think that we need a dedicated
alias_match() for servlet mapping (called alias_match_servlet() in the
PR), we can't normalize and match separately or the matched length is
completely off wrt the original URI-path.

Can you please try with the patches there? (the last is not really
necessary, it's just to complete the PR should this be merged).

You need to set:
 ProxyMappingDecoded off
in your vhost (or directory) for servlet mapping to be active, with a
ProxyPass like:
 ProxyPass /good/ http://127.0.0.1:80/good/ mapping=servlet

I tried with paths like
"/bad/..;foo=bar/.;foo=bar//other;foo=bar//..;foo=bar/good;foo1=bar1/;foo2=bar2/.;foo3=bar3///./index.html"
which results in "/good/;foo2=bar2/.;foo3=bar3///./index.html" being
forwarded, still things that shouldn't be seem to be declined.

The code in alias_match_servlet() is not really simple, but neither is
servlet mapping..


OK we are going forward:
ProxyMappingDecoded Off
ProxyPass  /test ajp://localhost:8009/test secret=%A1b2!@
and curl -v --path-as-is 
"http://localhost:8000/docs/..;food=bar/test/index.jsp 404 httpd.


ProxyMappingDecoded Off
ProxyPass  /test ajp://localhost:8009/test secret=%A1b2!@  mapping=servlet
and curl -v --path-as-is 
"http://localhost:8000/docs/..;food=bar/test/index.jsp 200 tc URL: 
http://localhost:8000/test/index.jsp
but curl -v --path-as-is 
"http://localhost:8000/docs/..;food=bar/test;food=bar/index.jsp; 404 httpd
what is going wrong with 
"http://localhost:8000/docs/..;food=bar/test;food=bar/index.jsp;
same for "curl -v --path-as-is 
"http://localhost:8000/test;food=bar/index.jsp;


ProxyMappingDecoded On
ProxyPass  /test ajp://localhost:8009/test secret=%A1b2!@ 
mapping=servlet 404 httpd.


ProxyMappingDecoded On
ProxyPass  /test ajp://localhost:8009/test secret=%A1b2!@ 404 httpd.

Comments?




Regards;
Yann.




--
Cheers

Jean-Frederic

Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line?

[Stefan Eissing]

Stefan Eissing

bytes GmbH
Hafenweg 16
48155 Münster
www.greenbytes.de

> Am 18.06.2020 um 16:51 schrieb William A Rowe Jr :
> 
>  
>  On 6/18/20 12:09 AM, Roy T. Fielding wrote:
> >> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem  wrote:
> >>
> >> I came across the question if we should not reject HTTP protocols >= 
> >> 2.0 in the request line when we parse it
> >> in ap_parse_request_line.
> >> This does not affect mod_http2 if loaded as HTTP/2.0 connections 
> >> itself are not parsed via ap_parse_request_line
> >> and sending a
> >>
> >> GET /something HTTP/2.0
> >>
> >> as request line is not a valid way to start a HTTP 2.0 connection and 
> >> I doubt that it will be for future major versions.
> 
> Correct, it starts an HTTP/1.1 connection, and the response should reflect 
> HTTP/1.1.
> 
> > That isn't how these things typically work. New protocols are
> > advanced with either deliberate backwards-compat or deliberate
> > backwards-break, with an expectation that it will either do
> > something useful on an older-protocol server or cause a safe
> > error in an expected way.
> >
> > Hence, we might still see an HTTP/4.0 that is designed to be
> > parsed like HTTP/1.1 (by an old server) while at the same time
> > work perfectly for a new server. That would be some hefty magic,
> > but it remains possible. Likewise, we might want to deploy a
> > version of h2 or HTTP/3 that works on unix domain sockets or
> > localhost over non-Internet TCP.
> >
> > This is why the existing code did not error on protocols >= 2.0.
> > Doing so is both unnecessary and counterproductive. If parsing
> > fails for some other reason, we want that other reason to be
> > in the response (because that's what the new protocol will be
> > expecting from an old protocol server). If it doesn't fail, we
> > want to provide the successful response because the request
> > was deliberately crafted that way to save a round trip.
> >
> > Note that the incoming request protocol version should always
> > be distinct from any forwarded request protocol or response
> > protocol versions.
>  
> Precisely. If mod_http2 or quic/mod_http3 can do something with the connection
> based on the request line, it's up to them through the hook facility to take 
> ownership
> of the connection.

That is not issue. That works well. 

> If they cannot/do not, then the core http1 connection/request processors 
> remain
> in place and in response to "please speak in HTTP/4.0" this server will 
> respond,
> "sure, here is your HTTP/1.1 response" as expected and defined by the RFC.

There are now several RFCs and they differentiate between HTTP/1.1 transport and
the pure HTTP semantics. This split is not reflected in our code, yet. We have 
functions that mix both. Not as a mistake, it's historical.

ap_parse_request_line() for example, checks the initial HTTP/1.1 request line 
*and*
the method names, uri, header_only and other request_rec fields.

We can either copy the latter into mod_http2 and maintain it in two places or
have a core function for it to be invoked by mod_http and mod_http2. That seems 
to be the design decision to make.

I used ap_parse_request_line() from mod_http2 to *not* duplicate the other code,
and someone added checks in trunk that imply it only ever gets called for 
HTTP/1.x.
So, now it pukes. Which is good, as it brought up this discussion.

- Stefan

Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line?

[William A Rowe Jr]

>  On 6/18/20 12:09 AM, Roy T. Fielding wrote:
> >> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem 
> wrote:
> >>
> >> I came across the question if we should not reject HTTP protocols
> >= 2.0 in the request line when we parse it
> >> in ap_parse_request_line.
> >> This does not affect mod_http2 if loaded as HTTP/2.0 connections
> itself are not parsed via ap_parse_request_line
> >> and sending a
> >>
> >> GET /something HTTP/2.0
> >>
> >> as request line is not a valid way to start a HTTP 2.0 connection
> and I doubt that it will be for future major versions.
>

Correct, it starts an HTTP/1.1 connection, and the response should reflect
HTTP/1.1.

> That isn't how these things typically work. New protocols are
> > advanced with either deliberate backwards-compat or deliberate
> > backwards-break, with an expectation that it will either do
> > something useful on an older-protocol server or cause a safe
> > error in an expected way.
> >
> > Hence, we might still see an HTTP/4.0 that is designed to be
> > parsed like HTTP/1.1 (by an old server) while at the same time
> > work perfectly for a new server. That would be some hefty magic,
> > but it remains possible. Likewise, we might want to deploy a
> > version of h2 or HTTP/3 that works on unix domain sockets or
> > localhost over non-Internet TCP.
> >
> > This is why the existing code did not error on protocols >= 2.0.
> > Doing so is both unnecessary and counterproductive. If parsing
> > fails for some other reason, we want that other reason to be
> > in the response (because that's what the new protocol will be
> > expecting from an old protocol server). If it doesn't fail, we
> > want to provide the successful response because the request
> > was deliberately crafted that way to save a round trip.
> >
> > Note that the incoming request protocol version should always
> > be distinct from any forwarded request protocol or response
> > protocol versions.
>

Precisely. If mod_http2 or quic/mod_http3 can do something with the
connection
based on the request line, it's up to them through the hook facility to
take ownership
of the connection.

If they cannot/do not, then the core http1 connection/request processors
remain
in place and in response to "please speak in HTTP/4.0" this server will
respond,
"sure, here is your HTTP/1.1 response" as expected and defined by the RFC.

Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line?

[Ruediger Pluem]

On 6/18/20 12:21 PM, Stefan Eissing wrote:
>> Am 18.06.2020 um 11:49 schrieb Ruediger Pluem :
>>
>>
>>
>> On 6/18/20 10:37 AM, Stefan Eissing wrote:
>>>
>>> Stefan Eissing
>>>
>>> bytes GmbH
>>> Hafenweg 16
>>> 48155 Münster
>>> www.greenbytes.de
>>>
 Am 18.06.2020 um 09:48 schrieb Ruediger Pluem :



 On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem  wrote:
>>
>> I came across the question if we should not reject HTTP protocols >= 2.0 
>> in the request line when we parse it
>> in ap_parse_request_line.
>> This does not affect mod_http2 if loaded as HTTP/2.0 connections itself 
>> are not parsed via ap_parse_request_line
>> and sending a
>>
>> GET /something HTTP/2.0
>>
>> as request line is not a valid way to start a HTTP 2.0 connection and I 
>> doubt that it will be for future major versions.
>
> That isn't how these things typically work. New protocols are
> advanced with either deliberate backwards-compat or deliberate
> backwards-break, with an expectation that it will either do
> something useful on an older-protocol server or cause a safe
> error in an expected way.
>
> Hence, we might still see an HTTP/4.0 that is designed to be
> parsed like HTTP/1.1 (by an old server) while at the same time
> work perfectly for a new server. That would be some hefty magic,
> but it remains possible. Likewise, we might want to deploy a
> version of h2 or HTTP/3 that works on unix domain sockets or
> localhost over non-Internet TCP.
>
> This is why the existing code did not error on protocols >= 2.0.
> Doing so is both unnecessary and counterproductive. If parsing
> fails for some other reason, we want that other reason to be
> in the response (because that's what the new protocol will be
> expecting from an old protocol server). If it doesn't fail, we
> want to provide the successful response because the request
> was deliberately crafted that way to save a round trip.
>
> Note that the incoming request protocol version should always
> be distinct from any forwarded request protocol or response
> protocol versions.

 As always many thanks for the insights. I see two ways forward now:

 1. Roll back r1878708 and all the follow ups and be done.
>>>
>>> +1
>>>
>>> If we want the HTTP/1.x protocol handler to balk at higher protocol 
>>> versions, this
>>> check should be done in ap_read_request() (which should be placed in 
>>> modules/http
>>
>> This is an interesting proposal, but I think that the two points I mentioned 
>> below in 2.
>> would apply as well if the check is done there.
>>
>>> (which should then be named modules/http1)).
>>
>> Given Roy's comments above I think that at least in theory the stuff in 
>> modules/http could
>> still be used for protocols > HTTP/2.x. So I am not sure if this rename is 
>> justified.
> 
> The pre-condition was that *if* modules/http code refuses processing >= 
> HTTP/2.0, it should be named modules/http1.

Fair enough. I somehow missed this if. I will wait how this discussion evolves 
to see what next steps are the best ones.

Regards

Rüdiger

Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line?

[Stefan Eissing]

> Am 18.06.2020 um 11:49 schrieb Ruediger Pluem :
> 
> 
> 
> On 6/18/20 10:37 AM, Stefan Eissing wrote:
>> 
>> Stefan Eissing
>> 
>> bytes GmbH
>> Hafenweg 16
>> 48155 Münster
>> www.greenbytes.de
>> 
>>> Am 18.06.2020 um 09:48 schrieb Ruediger Pluem :
>>> 
>>> 
>>> 
>>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem  wrote:
> 
> I came across the question if we should not reject HTTP protocols >= 2.0 
> in the request line when we parse it
> in ap_parse_request_line.
> This does not affect mod_http2 if loaded as HTTP/2.0 connections itself 
> are not parsed via ap_parse_request_line
> and sending a
> 
> GET /something HTTP/2.0
> 
> as request line is not a valid way to start a HTTP 2.0 connection and I 
> doubt that it will be for future major versions.
 
 That isn't how these things typically work. New protocols are
 advanced with either deliberate backwards-compat or deliberate
 backwards-break, with an expectation that it will either do
 something useful on an older-protocol server or cause a safe
 error in an expected way.
 
 Hence, we might still see an HTTP/4.0 that is designed to be
 parsed like HTTP/1.1 (by an old server) while at the same time
 work perfectly for a new server. That would be some hefty magic,
 but it remains possible. Likewise, we might want to deploy a
 version of h2 or HTTP/3 that works on unix domain sockets or
 localhost over non-Internet TCP.
 
 This is why the existing code did not error on protocols >= 2.0.
 Doing so is both unnecessary and counterproductive. If parsing
 fails for some other reason, we want that other reason to be
 in the response (because that's what the new protocol will be
 expecting from an old protocol server). If it doesn't fail, we
 want to provide the successful response because the request
 was deliberately crafted that way to save a round trip.
 
 Note that the incoming request protocol version should always
 be distinct from any forwarded request protocol or response
 protocol versions.
>>> 
>>> As always many thanks for the insights. I see two ways forward now:
>>> 
>>> 1. Roll back r1878708 and all the follow ups and be done.
>> 
>> +1
>> 
>> If we want the HTTP/1.x protocol handler to balk at higher protocol 
>> versions, this
>> check should be done in ap_read_request() (which should be placed in 
>> modules/http
> 
> This is an interesting proposal, but I think that the two points I mentioned 
> below in 2.
> would apply as well if the check is done there.
> 
>> (which should then be named modules/http1)).
> 
> Given Roy's comments above I think that at least in theory the stuff in 
> modules/http could
> still be used for protocols > HTTP/2.x. So I am not sure if this rename is 
> justified.

The pre-condition was that *if* modules/http code refuses processing >= 
HTTP/2.0, it should be named modules/http1.

But I agree that there is mostly stuff in there that we want to use for *any* 
http request.

>> 
>> 
>>> 2. Keep r1878708 and all the follow ups but adjust the code in 
>>> ap_parse_request_line
>>>  to be more specific when to sent a 505. Given today's state I think a 505 
>>> would still be
>>>  correct in case of
>> 
>>> 
>>>  1. Protocol >= HTTP/3.0. Of course this would need to be adjusted if an 
>>> implementation of
>>> HTTP/x.y with x > 2 pops up even if provided by a 3rd party module.
>>>  2. If HTTP/2.0 is sent over the wire like a HTTP/1.x request as HTTP/2 has 
>>> that
>>> deliberate backwards-break as I understand it. OTOH you could argue 
>>> that with
>>> mod_http2 present 505 is the wrong reply since the server supports 
>>> HTTP/2 but not the
>>> way it was tried and hence another code would be the correct response 
>>> (400 ??) if we see a
>>> HTTP/1.x request with Protocol HTTP/2.0.
>>> 
> 
> Regards
> 
> Rüdiger

Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line?

[Ruediger Pluem]

On 6/18/20 10:37 AM, Stefan Eissing wrote:
> 
> Stefan Eissing
> 
> bytes GmbH
> Hafenweg 16
> 48155 Münster
> www.greenbytes.de
> 
>> Am 18.06.2020 um 09:48 schrieb Ruediger Pluem :
>>
>>
>>
>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
 On Jun 8, 2020, at 12:56 AM, Ruediger Pluem  wrote:

 I came across the question if we should not reject HTTP protocols >= 2.0 
 in the request line when we parse it
 in ap_parse_request_line.
 This does not affect mod_http2 if loaded as HTTP/2.0 connections itself 
 are not parsed via ap_parse_request_line
 and sending a

 GET /something HTTP/2.0

 as request line is not a valid way to start a HTTP 2.0 connection and I 
 doubt that it will be for future major versions.
>>>
>>> That isn't how these things typically work. New protocols are
>>> advanced with either deliberate backwards-compat or deliberate
>>> backwards-break, with an expectation that it will either do
>>> something useful on an older-protocol server or cause a safe
>>> error in an expected way.
>>>
>>> Hence, we might still see an HTTP/4.0 that is designed to be
>>> parsed like HTTP/1.1 (by an old server) while at the same time
>>> work perfectly for a new server. That would be some hefty magic,
>>> but it remains possible. Likewise, we might want to deploy a
>>> version of h2 or HTTP/3 that works on unix domain sockets or
>>> localhost over non-Internet TCP.
>>>
>>> This is why the existing code did not error on protocols >= 2.0.
>>> Doing so is both unnecessary and counterproductive. If parsing
>>> fails for some other reason, we want that other reason to be
>>> in the response (because that's what the new protocol will be
>>> expecting from an old protocol server). If it doesn't fail, we
>>> want to provide the successful response because the request
>>> was deliberately crafted that way to save a round trip.
>>>
>>> Note that the incoming request protocol version should always
>>> be distinct from any forwarded request protocol or response
>>> protocol versions.
>>
>> As always many thanks for the insights. I see two ways forward now:
>>
>> 1. Roll back r1878708 and all the follow ups and be done.
> 
> +1
> 
> If we want the HTTP/1.x protocol handler to balk at higher protocol versions, 
> this
> check should be done in ap_read_request() (which should be placed in 
> modules/http

This is an interesting proposal, but I think that the two points I mentioned 
below in 2.
would apply as well if the check is done there.

> (which should then be named modules/http1)).

Given Roy's comments above I think that at least in theory the stuff in 
modules/http could
still be used for protocols > HTTP/2.x. So I am not sure if this rename is 
justified.

> 
> 
>> 2. Keep r1878708 and all the follow ups but adjust the code in 
>> ap_parse_request_line
>>   to be more specific when to sent a 505. Given today's state I think a 505 
>> would still be
>>   correct in case of
> 
>>
>>   1. Protocol >= HTTP/3.0. Of course this would need to be adjusted if an 
>> implementation of
>>  HTTP/x.y with x > 2 pops up even if provided by a 3rd party module.
>>   2. If HTTP/2.0 is sent over the wire like a HTTP/1.x request as HTTP/2 has 
>> that
>>  deliberate backwards-break as I understand it. OTOH you could argue 
>> that with
>>  mod_http2 present 505 is the wrong reply since the server supports 
>> HTTP/2 but not the
>>  way it was tried and hence another code would be the correct response 
>> (400 ??) if we see a
>>  HTTP/1.x request with Protocol HTTP/2.0.
>>

Regards

Rüdiger

Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line?

[Stefan Eissing]

Stefan Eissing

bytes GmbH
Hafenweg 16
48155 Münster
www.greenbytes.de

> Am 18.06.2020 um 09:48 schrieb Ruediger Pluem :
> 
> 
> 
> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem  wrote:
>>> 
>>> I came across the question if we should not reject HTTP protocols >= 2.0 in 
>>> the request line when we parse it
>>> in ap_parse_request_line.
>>> This does not affect mod_http2 if loaded as HTTP/2.0 connections itself are 
>>> not parsed via ap_parse_request_line
>>> and sending a
>>> 
>>> GET /something HTTP/2.0
>>> 
>>> as request line is not a valid way to start a HTTP 2.0 connection and I 
>>> doubt that it will be for future major versions.
>> 
>> That isn't how these things typically work. New protocols are
>> advanced with either deliberate backwards-compat or deliberate
>> backwards-break, with an expectation that it will either do
>> something useful on an older-protocol server or cause a safe
>> error in an expected way.
>> 
>> Hence, we might still see an HTTP/4.0 that is designed to be
>> parsed like HTTP/1.1 (by an old server) while at the same time
>> work perfectly for a new server. That would be some hefty magic,
>> but it remains possible. Likewise, we might want to deploy a
>> version of h2 or HTTP/3 that works on unix domain sockets or
>> localhost over non-Internet TCP.
>> 
>> This is why the existing code did not error on protocols >= 2.0.
>> Doing so is both unnecessary and counterproductive. If parsing
>> fails for some other reason, we want that other reason to be
>> in the response (because that's what the new protocol will be
>> expecting from an old protocol server). If it doesn't fail, we
>> want to provide the successful response because the request
>> was deliberately crafted that way to save a round trip.
>> 
>> Note that the incoming request protocol version should always
>> be distinct from any forwarded request protocol or response
>> protocol versions.
> 
> As always many thanks for the insights. I see two ways forward now:
> 
> 1. Roll back r1878708 and all the follow ups and be done.

+1

If we want the HTTP/1.x protocol handler to balk at higher protocol versions, 
this
check should be done in ap_read_request() (which should be placed in 
modules/http
(which should then be named modules/http1)).


> 2. Keep r1878708 and all the follow ups but adjust the code in 
> ap_parse_request_line
>   to be more specific when to sent a 505. Given today's state I think a 505 
> would still be
>   correct in case of

> 
>   1. Protocol >= HTTP/3.0. Of course this would need to be adjusted if an 
> implementation of
>  HTTP/x.y with x > 2 pops up even if provided by a 3rd party module.
>   2. If HTTP/2.0 is sent over the wire like a HTTP/1.x request as HTTP/2 has 
> that
>  deliberate backwards-break as I understand it. OTOH you could argue that 
> with
>  mod_http2 present 505 is the wrong reply since the server supports 
> HTTP/2 but not the
>  way it was tried and hence another code would be the correct response 
> (400 ??) if we see a
>  HTTP/1.x request with Protocol HTTP/2.0.
> 
> Regards
> 
> Rüdiger

Re: Broken: apache/httpd#804 (trunk - 97bc128)

[Stefan Eissing]

> Am 17.06.2020 um 21:19 schrieb Ruediger Pluem :
> 
> 
> 
> On 6/17/20 3:11 PM, Ruediger Pluem wrote:
>> 
>> 
>> On 6/17/20 11:52 AM, Yann Ylavic wrote:
>>> On Wed, Jun 17, 2020 at 10:43 AM Joe Orton  wrote:
 
> And yes this makes me think if this kind of fake really makes sense. The 
> need to reset 3 request_rec fields after
> ap_parse_request_line doesn't sound good.
>>> 
>>> At this point it isn't an HTTP/2 request IMHO, it's an HTTP/1 request
>>> extracted from an h2 stream.
>>> I suppose that using r->protocol = HTTP/2.0 is for (custom-)logging
>>> purpose, or is it specified? Stefan?
>>> 
> OTOH calling ap_parse_request_line makes it possible to apply the same 
> policies to
> HTTP/2.0 and HTTP/1.x requests at least where they overlap. Maybe the 
> change to the API is the way out.
> Keen to see Yann's feedback on this :-)
>>> 
>>> I think we should not mark h2 to h1 requests using r->protocol, it's
>>> confusing whereas it somehow should be transparent for h1
>>> core/modules.
>>> If something really needs to know, at worst r->proto_num could do, but
>>> I'd prefer a separate field or notes. If it's only for logging,
>>> possibly we could override that at a later stage/hook?
>>> As seen here, they really are not h2 requests as for h1 core/modules
>>> which actually handle them.
>>> 
 
 I'm not Yann but my 2c is that ap_parse_request_line() is designed to
 safely parse an HTTP/1.x request-line off the wire and probably
 shouldn't be used in mod_h2. The complexity of that function is dealing
 with all the error cases, which is not useful since none of them will be
 hit with HTTP/2.
>>> 
>>> I'm not sure, what guarantees otherwise that the method and URI-path
>>> extracted from HTTP/2 (i.e. in h2_request_add_header) are valid?
>> 
>> I think both views are somehow correct. I think some of the checks in 
>> ap_parse_request_line() are
>> not needed for these HTTP/1 requests extracted from a HTTP/2 stream. Others 
>> could be quite useful like
>> correct URI encoding or checking that only registered methods are used and 
>> we avoid code duplication
>> here. So what I can think of here is that we either split 
>> ap_parse_request_line even further
>> in tests useful only for the line from the wire in the HTTP/1 case and the 
>> checks useful for the HTTP/2
>> use case as well and have HTTP/1 use both and HTTP/2 only one or we supply 
>> the expected HTTP version
>> to ap_parse_request_line and behave accordingly.
> 
> Even with r1878938 and r1878939 now in place and trunk back to green I think 
> no final decision on the
> approach is done. But after r1878926 I wanted to fix the shortcomings of my 
> own initial proposal
> in order to have a complete approach in trunk. I am happy to go a different 
> way if this is thought to be
> better.

Thanks, Rüdiger.

After all this, I think ap_parse_request_line() should not balk at higher HTTP 
versions. Or being split into a HTTP/1.x part and HTTP.

> 
> Regards
> 
> Rüdiger

Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line?

[Ruediger Pluem]

On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem  wrote:
>>
>> I came across the question if we should not reject HTTP protocols >= 2.0 in 
>> the request line when we parse it
>> in ap_parse_request_line.
>> This does not affect mod_http2 if loaded as HTTP/2.0 connections itself are 
>> not parsed via ap_parse_request_line
>> and sending a
>>
>> GET /something HTTP/2.0
>>
>> as request line is not a valid way to start a HTTP 2.0 connection and I 
>> doubt that it will be for future major versions.
> 
> That isn't how these things typically work. New protocols are
> advanced with either deliberate backwards-compat or deliberate
> backwards-break, with an expectation that it will either do
> something useful on an older-protocol server or cause a safe
> error in an expected way.
> 
> Hence, we might still see an HTTP/4.0 that is designed to be
> parsed like HTTP/1.1 (by an old server) while at the same time
> work perfectly for a new server. That would be some hefty magic,
> but it remains possible. Likewise, we might want to deploy a
> version of h2 or HTTP/3 that works on unix domain sockets or
> localhost over non-Internet TCP.
> 
> This is why the existing code did not error on protocols >= 2.0.
> Doing so is both unnecessary and counterproductive. If parsing
> fails for some other reason, we want that other reason to be
> in the response (because that's what the new protocol will be
> expecting from an old protocol server). If it doesn't fail, we
> want to provide the successful response because the request
> was deliberately crafted that way to save a round trip.
> 
> Note that the incoming request protocol version should always
> be distinct from any forwarded request protocol or response
> protocol versions.

As always many thanks for the insights. I see two ways forward now:

1. Roll back r1878708 and all the follow ups and be done.
2. Keep r1878708 and all the follow ups but adjust the code in 
ap_parse_request_line
   to be more specific when to sent a 505. Given today's state I think a 505 
would still be
   correct in case of

   1. Protocol >= HTTP/3.0. Of course this would need to be adjusted if an 
implementation of
  HTTP/x.y with x > 2 pops up even if provided by a 3rd party module.
   2. If HTTP/2.0 is sent over the wire like a HTTP/1.x request as HTTP/2 has 
that
  deliberate backwards-break as I understand it. OTOH you could argue that 
with
  mod_http2 present 505 is the wrong reply since the server supports HTTP/2 
but not the
  way it was tried and hence another code would be the correct response 
(400 ??) if we see a
  HTTP/1.x request with Protocol HTTP/2.0.

Regards

Rüdiger