Mailing List Archive

Reject HTTP protocols >= 2.0 in ap_parse_request_line?
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.
A possible patch could look like the following (which rejects such requests with a HTTP_VERSION_NOT_SUPPORTED status code):

Index: server/protocol.c
===================================================================
--- server/protocol.c (revision 1878470)
+++ server/protocol.c (working copy)
@@ -748,7 +748,7 @@ AP_DECLARE(int) ap_parse_request_line(request_rec
enum {
rrl_none, rrl_badmethod, rrl_badwhitespace, rrl_excesswhitespace,
rrl_missinguri, rrl_baduri, rrl_badprotocol, rrl_trailingtext,
- rrl_badmethod09, rrl_reject09
+ rrl_badmethod09, rrl_reject09, rrl_versionnotsupported
} deferred_error = rrl_none;
apr_size_t len = 0;
char *uri, *ll;
@@ -897,6 +897,11 @@ rrl_done:
r->proto_num = HTTP_VERSION(0, 9);
}

+ if (strict && deferred_error == rrl_none
+ && r->proto_num >= HTTP_VERSION(2, 0)) {
+ deferred_error = rrl_versionnotsupported;
+ }
+
/* Determine the method_number and parse the uri prior to invoking error
* handling, such that these fields are available for substitution
*/
@@ -918,6 +923,7 @@ rrl_done:
* we can safely resume any deferred error reporting
*/
if (deferred_error != rrl_none) {
+ r->status = HTTP_BAD_REQUEST;
if (deferred_error == rrl_badmethod)
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03445)
"HTTP Request Line; Invalid method token: '%.*s'",
@@ -954,7 +960,13 @@ rrl_done:
"HTTP Request Line; Unrecognized protocol '%.*s' "
"(perhaps whitespace was injected?)",
field_name_len(r->protocol), r->protocol);
- r->status = HTTP_BAD_REQUEST;
+ else if (deferred_error == rrl_versionnotsupported) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO()
+ "HTTP Request Line; Protocol '%.*s' >= HTTP/2.0 not"
+ " supported", field_name_len(r->protocol),
+ r->protocol);
+ r->status = HTTP_VERSION_NOT_SUPPORTED;
+ }
goto rrl_failed;
}



Regards

Rüdiger
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <rpluem@apache.org> 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.

Why not >= 1.2 ?

> A possible patch could look like the following (which rejects such requests with a HTTP_VERSION_NOT_SUPPORTED status code):

Looks good.


In the same category, could we reject invalid URI paths earlier
(request-target per RFC-7230 5.3)?
Today it fails in ap_core_translate(), but possibly the below would be better:

Index: server/protocol.c
===================================================================
--- server/protocol.c (revision 1878328)
+++ server/protocol.c (working copy)
@@ -627,6 +627,14 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r,
}
else {
status = apr_uri_parse(r->pool, uri, &r->parsed_uri);
+ if (status == APR_SUCCESS
+ && (r->parsed_uri.path != NULL)
+ && (r->parsed_uri.path[0] != '/')
+ && (r->method_number != M_OPTIONS
+ || strcmp(r->parsed_uri.path, "*") != 0)) {
+ /* Invalid request-target per rfc7230#section-5.3 */
+ status = APR_EINVAL;
+ }
}

if (status == APR_SUCCESS) {
--

Regards;
Yann.
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On 08.06.2020 16:59, Yann Ylavic wrote:
> On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <rpluem@apache.org> 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.
>
> Why not >= 1.2 ?

In *theory*, there could a future HTTP/1.2 version that shares the wire
format with 1.0 and 1.1.

> ...

Best regards, Julian
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On Mon, Jun 8, 2020 at 5:43 PM Julian Reschke <julian.reschke@gmx.de> wrote:
>
> On 08.06.2020 16:59, Yann Ylavic wrote:
> > On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <rpluem@apache.org> 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.
> >
> > Why not >= 1.2 ?
>
> In *theory*, there could a future HTTP/1.2 version that shares the wire
> format with 1.0 and 1.1.

Sure, but we have quite some time to adapt the check then :)


Regards;
Yann.
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On 6/8/20 6:06 PM, Yann Ylavic wrote:
> On Mon, Jun 8, 2020 at 5:43 PM Julian Reschke <julian.reschke@gmx.de> wrote:
>>
>> On 08.06.2020 16:59, Yann Ylavic wrote:
>>> On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <rpluem@apache.org> 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.
>>>
>>> Why not >= 1.2 ?
>>
>> In *theory*, there could a future HTTP/1.2 version that shares the wire
>> format with 1.0 and 1.1.

Exactly this was my reason for not >= 1.2 as this case is IMHO already handled in a compliant way by the current code.
https://tools.ietf.org/html/rfc7230#section-2.6 states:

The interpretation of a header field does not change between minor
versions of the same major HTTP version, though the default behavior
of a recipient in the absence of such a field can change. Unless
specified otherwise, header fields defined in HTTP/1.1 are defined
for all versions of HTTP/1.x. In particular, the Host and Connection
header fields ought to be implemented by all HTTP/1.x implementations
whether or not they advertise conformance with HTTP/1.1.

New header fields can be introduced without changing the protocol
version if their defined semantics allow them to be safely ignored by
recipients that do not recognize them. Header field extensibility is
discussed in Section 3.2.1.


I interpret this that if we handle a potential HTTP 1.2 request as if

- headers already known in HTTP 1.1 are handled in the same way as if the request was HTTP 1.1
- headers unknown in HTTP 1.2 and added and in HTTP 1.2 are ignored

we are on the safe side if we sent back a HTTP 1.1 response.

Furthermore I get from https://tools.ietf.org/html/rfc7231#section-6.6.6 that
505 indicates that we reject the processing of the major version which would be wrong in the HTTP 1.2 case
as we will process HTTP 1.0 and HTTP 1.1.

Regards

Rüdiger
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On 6/8/20 4:59 PM, Yann Ylavic wrote:
> On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <rpluem@apache.org> 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.
>
> Why not >= 1.2 ?
>
>> A possible patch could look like the following (which rejects such requests with a HTTP_VERSION_NOT_SUPPORTED status code):
>
> Looks good.
>
>
> In the same category, could we reject invalid URI paths earlier
> (request-target per RFC-7230 5.3)?
> Today it fails in ap_core_translate(), but possibly the below would be better:

I think we could, but I am not sure if we have ap_parse_uri callers in other parts of the code that do not pass absolute URI's

>
> Index: server/protocol.c
> ===================================================================
> --- server/protocol.c (revision 1878328)
> +++ server/protocol.c (working copy)
> @@ -627,6 +627,14 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r,
> }
> else {
> status = apr_uri_parse(r->pool, uri, &r->parsed_uri);
> + if (status == APR_SUCCESS
> + && (r->parsed_uri.path != NULL)
> + && (r->parsed_uri.path[0] != '/')
> + && (r->method_number != M_OPTIONS
> + || strcmp(r->parsed_uri.path, "*") != 0)) {
> + /* Invalid request-target per rfc7230#section-5.3 */
> + status = APR_EINVAL;
> + }
> }
>
> if (status == APR_SUCCESS) {

Don't we miss in server/protocol.c:

@@ -906,6 +911,12 @@

ap_parse_uri(r, uri);

+ if (strict && deferred_error == rrl_none
+ && r->status == HTTP_BAD_REQUEST) {
+ deferred_error = rrl_invaliduri
+ }
+
+

Plus adding rrl_invaliduri to the enum and handling later on?

Regards

Rüdiger
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On Mon, Jun 8, 2020 at 9:30 PM Ruediger Pluem <rpluem@apache.org> wrote:
>
> On 6/8/20 4:59 PM, Yann Ylavic wrote:
> > On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <rpluem@apache.org> 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.
> >
> > Why not >= 1.2 ?
> >
> >> A possible patch could look like the following (which rejects such requests with a HTTP_VERSION_NOT_SUPPORTED status code):
> >
> > Looks good.
> >
> >
> > In the same category, could we reject invalid URI paths earlier
> > (request-target per RFC-7230 5.3)?
> > Today it fails in ap_core_translate(), but possibly the below would be better:
>
> I think we could, but I am not sure if we have ap_parse_uri callers in other parts of the code that do not pass absolute URI's

This patch works with absolute URIs too since apr_parse_uri will split
it in r->parsed_uri and we consider r->parsed_uri.path only.

>
> >
> > Index: server/protocol.c
> > ===================================================================
> > --- server/protocol.c (revision 1878328)
> > +++ server/protocol.c (working copy)
> > @@ -627,6 +627,14 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r,
> > }
> > else {
> > status = apr_uri_parse(r->pool, uri, &r->parsed_uri);
> > + if (status == APR_SUCCESS
> > + && (r->parsed_uri.path != NULL)
> > + && (r->parsed_uri.path[0] != '/')
> > + && (r->method_number != M_OPTIONS
> > + || strcmp(r->parsed_uri.path, "*") != 0)) {
> > + /* Invalid request-target per rfc7230#section-5.3 */
> > + status = APR_EINVAL;
> > + }
> > }
> >
> > if (status == APR_SUCCESS) {
>
> Don't we miss in server/protocol.c:
>
> @@ -906,6 +911,12 @@
>
> ap_parse_uri(r, uri);
>
> + if (strict && deferred_error == rrl_none
> + && r->status == HTTP_BAD_REQUEST) {
> + deferred_error = rrl_invaliduri
> + }

Somehow r->status != HTTP_OK is caught below all the deferred_error
cases (which we want to report in priority?), and then jumps to the
rrl_failed label.


Regards;
Yann.
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On Mon, Jun 8, 2020 at 10:05 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> On Mon, Jun 8, 2020 at 9:30 PM Ruediger Pluem <rpluem@apache.org> wrote:
> >
> > I think we could, but I am not sure if we have ap_parse_uri callers in other parts of the code that do not pass absolute URI's
>
> This patch works with absolute URIs too since apr_parse_uri will split
> it in r->parsed_uri and we consider r->parsed_uri.path only.

But to be complete on that side, and handle the proxy case of the
asterisk-form (section 5.3.4 [1]), I think we also need this hunk:

Index: server/protocol.c
===================================================================
--- server/protocol.c (revision 1878328)
+++ server/protocol.c (working copy)
@@ -640,8 +648,15 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r,
}

r->args = r->parsed_uri.query;
- r->uri = r->parsed_uri.path ? r->parsed_uri.path
- : apr_pstrdup(r->pool, "/");
+ if (r->parsed_uri.path) {
+ r->uri = r->parsed_uri.path;
+ }
+ else if (r->method_number == M_OPTIONS) {
+ r->uri = apr_pstrdup(r->pool, "*");
+ }
+ else {
+ r->uri = apr_pstrdup(r->pool, "/");
+ }

#if defined(OS2) || defined(WIN32)
/* Handle path translations for OS/2 and plug security hole.
--

[1] https://tools.ietf.org/html/rfc7230#section-5.3.4
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On 6/8/20 10:05 PM, Yann Ylavic wrote:
> On Mon, Jun 8, 2020 at 9:30 PM Ruediger Pluem <rpluem@apache.org> wrote:
>>
>> On 6/8/20 4:59 PM, Yann Ylavic wrote:
>>> On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <rpluem@apache.org> 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.
>>>
>>> Why not >= 1.2 ?
>>>
>>>> A possible patch could look like the following (which rejects such requests with a HTTP_VERSION_NOT_SUPPORTED status code):
>>>
>>> Looks good.
>>>
>>>
>>> In the same category, could we reject invalid URI paths earlier
>>> (request-target per RFC-7230 5.3)?
>>> Today it fails in ap_core_translate(), but possibly the below would be better:
>>
>> I think we could, but I am not sure if we have ap_parse_uri callers in other parts of the code that do not pass absolute URI's
>
> This patch works with absolute URIs too since apr_parse_uri will split
> it in r->parsed_uri and we consider r->parsed_uri.path only.

I guess I was not precise enough with my concern. I meant that I haven't checked if there are callers which pass in relative URI's.

>
>>
>>>
>>> Index: server/protocol.c
>>> ===================================================================
>>> --- server/protocol.c (revision 1878328)
>>> +++ server/protocol.c (working copy)
>>> @@ -627,6 +627,14 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r,
>>> }
>>> else {
>>> status = apr_uri_parse(r->pool, uri, &r->parsed_uri);
>>> + if (status == APR_SUCCESS
>>> + && (r->parsed_uri.path != NULL)
>>> + && (r->parsed_uri.path[0] != '/')
>>> + && (r->method_number != M_OPTIONS
>>> + || strcmp(r->parsed_uri.path, "*") != 0)) {
>>> + /* Invalid request-target per rfc7230#section-5.3 */
>>> + status = APR_EINVAL;
>>> + }
>>> }
>>>
>>> if (status == APR_SUCCESS) {
>>
>> Don't we miss in server/protocol.c:
>>
>> @@ -906,6 +911,12 @@
>>
>> ap_parse_uri(r, uri);
>>
>> + if (strict && deferred_error == rrl_none
>> + && r->status == HTTP_BAD_REQUEST) {
>> + deferred_error = rrl_invaliduri
>> + }
>
> Somehow r->status != HTTP_OK is caught below all the deferred_error
> cases (which we want to report in priority?), and then jumps to the
> rrl_failed label.

Fair enough :-) I missed this one.

Regards

Rüdiger
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On Mon, Jun 8, 2020 at 10:12 PM Ruediger Pluem <rpluem@apache.org> wrote:
>
> On 6/8/20 10:05 PM, Yann Ylavic wrote:
> > On Mon, Jun 8, 2020 at 9:30 PM Ruediger Pluem <rpluem@apache.org> wrote:
> >>
> >> On 6/8/20 4:59 PM, Yann Ylavic wrote:
> >>> On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <rpluem@apache.org> 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.
> >>>
> >>> Why not >= 1.2 ?
> >>>
> >>>> A possible patch could look like the following (which rejects such requests with a HTTP_VERSION_NOT_SUPPORTED status code):
> >>>
> >>> Looks good.
> >>>
> >>>
> >>> In the same category, could we reject invalid URI paths earlier
> >>> (request-target per RFC-7230 5.3)?
> >>> Today it fails in ap_core_translate(), but possibly the below would be better:
> >>
> >> I think we could, but I am not sure if we have ap_parse_uri callers in other parts of the code that do not pass absolute URI's
> >
> > This patch works with absolute URIs too since apr_parse_uri will split
> > it in r->parsed_uri and we consider r->parsed_uri.path only.
>
> I guess I was not precise enough with my concern. I meant that I haven't checked if there are callers which pass in relative URI's.

Oh no, actually _I_ missed the "do _not_ pass" :)

I don't think ap_parse_uri() should accept an URI-path which is not
HTTP compliant (i.e. does not start with '/'), that's where we
initialize the request_rec after all.. So yes apr_uri_parse() accepts
that and sets a relative path in r->parsed_uri.path, but
ap_parse_uri() if for HTTP parsing IMHO.

Regards;
Yann.
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On Mon, Jun 8, 2020 at 8:38 PM Ruediger Pluem <rpluem@apache.org> wrote:
>
> On 6/8/20 6:06 PM, Yann Ylavic wrote:
> > On Mon, Jun 8, 2020 at 5:43 PM Julian Reschke <julian.reschke@gmx.de> wrote:
> >>
> >> On 08.06.2020 16:59, Yann Ylavic wrote:
> >>> On Mon, Jun 8, 2020 at 9:56 AM Ruediger Pluem <rpluem@apache.org> 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.
> >>>
> >>> Why not >= 1.2 ?
> >>
> >> In *theory*, there could a future HTTP/1.2 version that shares the wire
> >> format with 1.0 and 1.1.
>
> Exactly this was my reason for not >= 1.2 as this case is IMHO already handled in a compliant way by the current code.
> https://tools.ietf.org/html/rfc7230#section-2.6 states:
>
> The interpretation of a header field does not change between minor
> versions of the same major HTTP version, though the default behavior
> of a recipient in the absence of such a field can change. Unless
> specified otherwise, header fields defined in HTTP/1.1 are defined
> for all versions of HTTP/1.x. In particular, the Host and Connection
> header fields ought to be implemented by all HTTP/1.x implementations
> whether or not they advertise conformance with HTTP/1.1.
>
> New header fields can be introduced without changing the protocol
> version if their defined semantics allow them to be safely ignored by
> recipients that do not recognize them. Header field extensibility is
> discussed in Section 3.2.1.
>
>
> I interpret this that if we handle a potential HTTP 1.2 request as if
>
> - headers already known in HTTP 1.1 are handled in the same way as if the request was HTTP 1.1
> - headers unknown in HTTP 1.2 and added and in HTTP 1.2 are ignored
>
> we are on the safe side if we sent back a HTTP 1.1 response.
>
> Furthermore I get from https://tools.ietf.org/html/rfc7231#section-6.6.6 that
> 505 indicates that we reject the processing of the major version which would be wrong in the HTTP 1.2 case
> as we will process HTTP 1.0 and HTTP 1.1.

Fair enough ;)

Regards;
Yann.
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org> 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.

....Roy
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org> 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
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
Stefan Eissing

<green/>bytes GmbH
Hafenweg 16
48155 Münster
www.greenbytes.de

> Am 18.06.2020 um 09:48 schrieb Ruediger Pluem <rpluem@apache.org>:
>
>
>
> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org> 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: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On 6/18/20 10:37 AM, Stefan Eissing wrote:
>
> Stefan Eissing
>
> <green/>bytes GmbH
> Hafenweg 16
> 48155 Münster
> www.greenbytes.de
>
>> Am 18.06.2020 um 09:48 schrieb Ruediger Pluem <rpluem@apache.org>:
>>
>>
>>
>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>>>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org> 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? [ In reply to ]
> Am 18.06.2020 um 11:49 schrieb Ruediger Pluem <rpluem@apache.org>:
>
>
>
> On 6/18/20 10:37 AM, Stefan Eissing wrote:
>>
>> Stefan Eissing
>>
>> <green/>bytes GmbH
>> Hafenweg 16
>> 48155 Münster
>> www.greenbytes.de
>>
>>> Am 18.06.2020 um 09:48 schrieb Ruediger Pluem <rpluem@apache.org>:
>>>
>>>
>>>
>>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>>>>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org> 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? [ In reply to ]
On 6/18/20 12:21 PM, Stefan Eissing wrote:
>> Am 18.06.2020 um 11:49 schrieb Ruediger Pluem <rpluem@apache.org>:
>>
>>
>>
>> On 6/18/20 10:37 AM, Stefan Eissing wrote:
>>>
>>> Stefan Eissing
>>>
>>> <green/>bytes GmbH
>>> Hafenweg 16
>>> 48155 Münster
>>> www.greenbytes.de
>>>
>>>> Am 18.06.2020 um 09:48 schrieb Ruediger Pluem <rpluem@apache.org>:
>>>>
>>>>
>>>>
>>>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>>>>>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org> 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? [ In reply to ]
> >>>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
> >>>>>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org>
> 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? [ In reply to ]
Stefan Eissing

<green/>bytes GmbH
Hafenweg 16
48155 Münster
www.greenbytes.de

> Am 18.06.2020 um 16:51 schrieb William A Rowe Jr <wrowe@rowe-clan.net>:
>
>
> >>>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
> >>>>>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org> 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? [ In reply to ]
> On Jun 18, 2020, at 9:03 AM, Stefan Eissing <stefan.eissing@greenbytes.de> wrote:
>> Am 18.06.2020 um 16:51 schrieb William A Rowe Jr <wrowe@rowe-clan.net>:
>>
>>
>>>>>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>>>>>>>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org> 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/httpwg/http-core>
https://github.com/quicwg/base-drafts <https://github.com/quicwg/base-drafts>

...Roy
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On 6/18/20 8:55 PM, Roy T. Fielding wrote:
>> On Jun 18, 2020, at 9:03 AM, Stefan Eissing <stefan.eissing@greenbytes.de <mailto:stefan.eissing@greenbytes.de>> wrote:
>>> Am 18.06.2020 um 16:51 schrieb William A Rowe Jr <wrowe@rowe-clan.net <mailto:wrowe@rowe-clan.net>>:
>>>
>>>
>>>>>>> On 6/18/20 12:09 AM, Roy T. Fielding wrote:
>>>>>>>>> On Jun 8, 2020, at 12:56 AM, Ruediger Pluem <rpluem@apache.org <mailto:rpluem@apache.org>> 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 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
>

Regards

R?diger
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On 6/18/20 9:58 PM, Ruediger Pluem wrote:

>
> 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).

Done as r1878984 and r1878985

Regards

Rüdiger
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
Thanks!

> Am 19.06.2020 um 14:02 schrieb Ruediger Pluem <rpluem@apache.org>:
>
>
>
> On 6/18/20 9:58 PM, Ruediger Pluem wrote:
>
>>
>> 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).
>
> Done as r1878984 and r1878985
>
> Regards
>
> Rüdiger
>
Re: Reject HTTP protocols >= 2.0 in ap_parse_request_line? [ In reply to ]
On Thu, Jun 18, 2020 at 6:03 PM Stefan Eissing
<stefan.eissing@greenbytes.de> wrote:
>
> 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.

How about this attached patch?

ap_parse_request_line() will only parse the request line (extract the
fields, which mod_h2 doesn't need), and ap_check_request_line() will
do the validation.

Since ap_parse_request_line() becomes local to "protocol.c" only, it
can be unexported (not in this patch to avoid a MAJOR bump and thus
ease backport).

Thoughts?