You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by William A Rowe Jr <wr...@rowe-clan.net> on 2015/12/07 17:38:35 UTC

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Hi folks, sorry for the late interruption after we have already shipped
2.4.16, but there seems to be an issue that merits revisiting before the
2.4.16 API schema is widely adopted.

We seem to have misplaced the upgrade handler in the wrong hook.  This is
easily shown by the fact that we had two different upgrade paths between
mod_ssl TLS upgrade, and h2c upgrade paths, and a crufty patch to solve the
OPTIONS * request exception which isn't an exception at all per RFC7230.

In mod_ssl connection upgrade we once emitted the TLS upgrade header in
fixups.  Fixups are bypassed by OPTIONS *.  My trunk patch moved this
mod_ssl TLS upgrade handling to post read req, leaving OPTIONS * with its
simple behavior.

In h2c this exists in an upgrade handler, and OPTIONS * was hacked to
invoke this function.  Problem, it is a transport (hop by hop) detail and
should not be presented in the request, but rather in the connection
hooks.  Moreover, none of the other *request* processing phases appear
valid prior to conducting the h2c upgrade, and some appear that they can be
harmful to the admin attempting to configure Protocol h2c.

The relevant specs are clear, it is a hop-by-hop connection transport offer
carried over the request headers.  It is not generally subject to auth
processing, certainly not resource processing.  The underlying request must
still be satisfied.  The current Protocols logic doesn't appear to do this
because it was based off of tangential specs such as h2c that don't
override 2616bis.  If we consider that a "method" has a "handler", and
transfer "headers" are "filtered" it becomes more obvious.  This is not the
UPGRADE method.

https://tools.ietf.org/html/rfc7230#section-6.7 makes things more
interesting, it calls out that 101-continue and the request body read
precedes the 101-switching protocols.  Not sure who decided that would be a
good idea, sigh... but mod_ssl TLS upgrade has these reversed for several
good reasons including the intent to encrypt the request body if present
and simple economics of processing.

I think that handling upgrade advertisement and alerting must be in post
read req, bypassing all request hooks until the 101-continue is presented,
any small request body read and set aside for the http input brigade, and
101-switching protocols is presented.  This allows the request to still be
processed for tls-style upgrades, or discarded for relevant protocols.

The beginning of a patch is attached, I note that the ap_switch_protocol
return value was never properly interpreted and my patch doesn't begin to
catch all of the exceptional cases.  Because 101-switching protocols has
already been sent, if the call fails to perform an upgrade, it seems we
need to respond with a generic 400 switch failed unless the registered
protocol switch handler is going to ap_die with a different sort of
specific error (e.g. "A server MUST NOT switch protocols unless the
received message semantics can be honored by the new protocol; an OPTIONS
request can be honored by any protocol.")

Stefan and other protocol gurus, please have a look at the proposed patch,
thanks.

Bill

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Jacob Champion <ch...@gmail.com>.
On Dec 7, 2015 12:44 PM, "Stefan Eissing" <st...@greenbytes.de>
wrote:
>
> There can be no 100 after a 101. After a 101, the downstream speaks the
new protocol, immediately.

I suspect Bill is talking about the very specific case of an upgrade to
HTTP/1.1-over-TLS, in which case I think his proposed "101/100 handshake"
logic could work. IMO, that's too specific a use case to work well with the
rest of the standard, but like he said, it's completely academic now that
723x clarifies the required order.

--Jacob

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 4:12 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Mon, Dec 7, 2015 at 2:39 PM, Stefan Eissing <
> stefan.eissing@greenbytes.de> wrote:
>
>> There can be no 100 after a 101. After a 101, the downstream speaks the
>> new protocol, immediately.
>>
>
> Right, and the new protocol is TLS/1.x HTTP/1.1 in the mod_ssl case.
> What's
> so confusing?
>

And in that case, define 'immediately' :)  That's where the RFC2817
ambiguity came in, 100 before/after 101, which was then resolved
by RFC7230 as noted earlier (but not in the manner one at least
I might have hoped for).

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 2:39 PM, Stefan Eissing <stefan.eissing@greenbytes.de
> wrote:

> There can be no 100 after a 101. After a 101, the downstream speaks the
> new protocol, immediately.
>

Right, and the new protocol is TLS/1.x HTTP/1.1 in the mod_ssl case.  What's
so confusing?



> Am 07.12.2015 um 21:29 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
>
> On Mon, Dec 7, 2015 at 2:15 PM, Jacob Champion <ch...@gmail.com>
> wrote:
>
>> On Dec 7, 2015 8:43 AM, "William A Rowe Jr" <wr...@rowe-clan.net> wrote:
>> >
>> > https://tools.ietf.org/html/rfc7230#section-6.7 makes things more
>> interesting, it calls out that 101-continue and the request body read
>> precedes the 101-switching protocols.  Not sure who decided that would be a
>> good idea, sigh...
>>
>> 100-continue can't be after the switch; the new protocol may not have any
>> idea what continue semantics are. Similarly, the request body sent is still
>> part of an HTTP/1.1 request and should be processed as such.
>>
>> > but mod_ssl TLS upgrade has these reversed for several good reasons
>> including the intent to encrypt the request body if present and simple
>> economics of processing.
>>
>> Did you mean "decrypt the request body"?
>>
> Yes, my bad...
>
>> The client needs to send its request body in plaintext since servers are
>> free to completely ignore the Upgrade, right? My reading of the specs is
>> that an Upgrade request is still a self-contained HTTP/1.1 request, body
>> included.
>>
> No.  If the upgrade is rejected, no 101-switching protocols occurs.  If
> there was no Expect: 100-continue then yes, the body seems part of the
> request headers with no way to anticipate how to proceed except plaintext,
> but in the case of an Expect: 100-continue, an an interim 101-switching
> protocols, a tls handshake, followed by a 100-continue seemed entirely
> reasonable.
>
> It appears we did the wrong thing in the absence of an Expect:
> 100-continue, but as a practical matter this seems fairly academic since
> RFC7230 codifies a specific sequence that we must adopt, and the instances
> of request bodies in upgrade requests seems philosophical.
>
>
>
>
>

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 6:05 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> On Mon, Dec 7, 2015 at 2:54 PM, Stefan Eissing <
> stefan.eissing@greenbytes.de> wrote:
>
>> ok, after some more thinking. if a tls+http/1.1 upgrade together with
>> Expect is indeed a use case, then, to make that work, sending the 101 needs
>> to become the task of the switch protocol handler itself. Then the order in
>> which intermediate responses are sent depends on the switched protocol.
>>
>> When writing the current code, I wanted the common tasks in core. But if
>> that needs to vary, we need to shift that.
>>
>
> 100-continue should be simple enough to implement; we can even defer
> the 101-switching protocols until we determine that the chunked request
> body "fits in memory".  Roughly...
>
> 1. in the core protocol implementation, we determine that the provider
>     is willing, so far, based on a request body of known C-L body size
>     or of indeterminate length (no bytes read, chunked encoding)
>
> 2. we invoking a speculative connection filter read at the http protocol
>     to network layer - that http input filter sends the 100-continue where
>     it aught to be. That speculative read sets aside bytes read for later
>     consumption based on the buffer it is willing to offer.  The body has
>     been read complete from the network or is still in an incomplete
>     chunk (or would block waiting for the next chunk).
>

By 'speculative read' here, I mean a look-ahead read.


> 3. back in the core protocol implementation, if the http input filter still
>     must read from the network beyond it's small buffer, no upgrade is
>     realistically possible, then we fall out before processing the
> protocol
>     switch and proceed as http/1.1.
>
> 4. alternately, if the input filter read completely, the core protocol
>     implementation now sends the 101-switching protocols and lets
>     the protocol implementation determine whether it injects the
>     appropriate filters and resumes (e.g. tls), assumes authority
>     over the core request handling and protocol (e.g. h2c), or jumps
>     the shark entirely (e.g. echo) and has nothing more to do with
>     the request.
>
>
>

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 2:54 PM, Stefan Eissing <stefan.eissing@greenbytes.de
> wrote:

> ok, after some more thinking. if a tls+http/1.1 upgrade together with
> Expect is indeed a use case, then, to make that work, sending the 101 needs
> to become the task of the switch protocol handler itself. Then the order in
> which intermediate responses are sent depends on the switched protocol.
>
> When writing the current code, I wanted the common tasks in core. But if
> that needs to vary, we need to shift that.
>

100-continue should be simple enough to implement; we can even defer
the 101-switching protocols until we determine that the chunked request
body "fits in memory".  Roughly...

1. in the core protocol implementation, we determine that the provider
    is willing, so far, based on a request body of known C-L body size
    or of indeterminate length (no bytes read, chunked encoding)

2. we invoking a speculative connection filter read at the http protocol
    to network layer - that http input filter sends the 100-continue where
    it aught to be. That speculative read sets aside bytes read for later
    consumption based on the buffer it is willing to offer.  The body has
    been read complete from the network or is still in an incomplete
    chunk (or would block waiting for the next chunk).

3. back in the core protocol implementation, if the http input filter still
    must read from the network beyond it's small buffer, no upgrade is
    realistically possible, then we fall out before processing the protocol
    switch and proceed as http/1.1.

4. alternately, if the input filter read completely, the core protocol
    implementation now sends the 101-switching protocols and lets
    the protocol implementation determine whether it injects the
    appropriate filters and resumes (e.g. tls), assumes authority
    over the core request handling and protocol (e.g. h2c), or jumps
    the shark entirely (e.g. echo) and has nothing more to do with
    the request.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Stefan Eissing <st...@greenbytes.de>.
ok, after some more thinking. if a tls+http/1.1 upgrade together with Expect is indeed a use case, then, to make that work, sending the 101 needs to become the task of the switch protocol handler itself. Then the order in which intermediate responses are sent depends on the switched protocol. 

When writing the current code, I wanted the common tasks in core. But if that needs to vary, we need to shift that. 

> Am 07.12.2015 um 21:39 schrieb Stefan Eissing <st...@greenbytes.de>:
> 
> There can be no 100 after a 101. After a 101, the downstream speaks the new protocol, immediately. 
> 
>> Am 07.12.2015 um 21:29 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
>> 
>>> On Mon, Dec 7, 2015 at 2:15 PM, Jacob Champion <ch...@gmail.com> wrote:
>>> On Dec 7, 2015 8:43 AM, "William A Rowe Jr" <wr...@rowe-clan.net> wrote:
>>> >
>>> > https://tools.ietf.org/html/rfc7230#section-6.7 makes things more interesting, it calls out that 101-continue and the request body read precedes the 101-switching protocols.  Not sure who decided that would be a good idea, sigh...
>>> 
>>> 100-continue can't be after the switch; the new protocol may not have any idea what continue semantics are. Similarly, the request body sent is still part of an HTTP/1.1 request and should be processed as such.
>>> 
>>> > but mod_ssl TLS upgrade has these reversed for several good reasons including the intent to encrypt the request body if present and simple economics of processing.
>>> 
>>> Did you mean "decrypt the request body"?
>>> 
>> Yes, my bad... 
>>> The client needs to send its request body in plaintext since servers are free to completely ignore the Upgrade, right? My reading of the specs is that an Upgrade request is still a self-contained HTTP/1.1 request, body included.
>>> 
>> No.  If the upgrade is rejected, no 101-switching protocols occurs.  If there was no Expect: 100-continue then yes, the body seems part of the request headers with no way to anticipate how to proceed except plaintext, but in the case of an Expect: 100-continue, an an interim 101-switching protocols, a tls handshake, followed by a 100-continue seemed entirely reasonable.
>> 
>> It appears we did the wrong thing in the absence of an Expect: 100-continue, but as a practical matter this seems fairly academic since RFC7230 codifies a specific sequence that we must adopt, and the instances of request bodies in upgrade requests seems philosophical.
>> 
>> 
>> 
>> 

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Stefan Eissing <st...@greenbytes.de>.
There can be no 100 after a 101. After a 101, the downstream speaks the new protocol, immediately. 

> Am 07.12.2015 um 21:29 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
> 
>> On Mon, Dec 7, 2015 at 2:15 PM, Jacob Champion <ch...@gmail.com> wrote:
>> On Dec 7, 2015 8:43 AM, "William A Rowe Jr" <wr...@rowe-clan.net> wrote:
>> >
>> > https://tools.ietf.org/html/rfc7230#section-6.7 makes things more interesting, it calls out that 101-continue and the request body read precedes the 101-switching protocols.  Not sure who decided that would be a good idea, sigh...
>> 
>> 100-continue can't be after the switch; the new protocol may not have any idea what continue semantics are. Similarly, the request body sent is still part of an HTTP/1.1 request and should be processed as such.
>> 
>> > but mod_ssl TLS upgrade has these reversed for several good reasons including the intent to encrypt the request body if present and simple economics of processing.
>> 
>> Did you mean "decrypt the request body"?
>> 
> Yes, my bad... 
>> The client needs to send its request body in plaintext since servers are free to completely ignore the Upgrade, right? My reading of the specs is that an Upgrade request is still a self-contained HTTP/1.1 request, body included.
>> 
> No.  If the upgrade is rejected, no 101-switching protocols occurs.  If there was no Expect: 100-continue then yes, the body seems part of the request headers with no way to anticipate how to proceed except plaintext, but in the case of an Expect: 100-continue, an an interim 101-switching protocols, a tls handshake, followed by a 100-continue seemed entirely reasonable.
> 
> It appears we did the wrong thing in the absence of an Expect: 100-continue, but as a practical matter this seems fairly academic since RFC7230 codifies a specific sequence that we must adopt, and the instances of request bodies in upgrade requests seems philosophical.
> 
> 
> 
> 

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 2:15 PM, Jacob Champion <ch...@gmail.com> wrote:

> On Dec 7, 2015 8:43 AM, "William A Rowe Jr" <wr...@rowe-clan.net> wrote:
> >
> > https://tools.ietf.org/html/rfc7230#section-6.7 makes things more
> interesting, it calls out that 101-continue and the request body read
> precedes the 101-switching protocols.  Not sure who decided that would be a
> good idea, sigh...
>
> 100-continue can't be after the switch; the new protocol may not have any
> idea what continue semantics are. Similarly, the request body sent is still
> part of an HTTP/1.1 request and should be processed as such.
>
> > but mod_ssl TLS upgrade has these reversed for several good reasons
> including the intent to encrypt the request body if present and simple
> economics of processing.
>
> Did you mean "decrypt the request body"?
>
Yes, my bad...

> The client needs to send its request body in plaintext since servers are
> free to completely ignore the Upgrade, right? My reading of the specs is
> that an Upgrade request is still a self-contained HTTP/1.1 request, body
> included.
>
No.  If the upgrade is rejected, no 101-switching protocols occurs.  If
there was no Expect: 100-continue then yes, the body seems part of the
request headers with no way to anticipate how to proceed except plaintext,
but in the case of an Expect: 100-continue, an an interim 101-switching
protocols, a tls handshake, followed by a 100-continue seemed entirely
reasonable.

It appears we did the wrong thing in the absence of an Expect:
100-continue, but as a practical matter this seems fairly academic since
RFC7230 codifies a specific sequence that we must adopt, and the instances
of request bodies in upgrade requests seems philosophical.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Jacob Champion <ch...@gmail.com>.
On Dec 7, 2015 8:43 AM, "William A Rowe Jr" <wr...@rowe-clan.net> wrote:
>
> https://tools.ietf.org/html/rfc7230#section-6.7 makes things more
interesting, it calls out that 101-continue and the request body read
precedes the 101-switching protocols.  Not sure who decided that would be a
good idea, sigh...

100-continue can't be after the switch; the new protocol may not have any
idea what continue semantics are. Similarly, the request body sent is still
part of an HTTP/1.1 request and should be processed as such.

> but mod_ssl TLS upgrade has these reversed for several good reasons
including the intent to encrypt the request body if present and simple
economics of processing.

Did you mean "decrypt the request body"? The client needs to send its
request body in plaintext since servers are free to completely ignore the
Upgrade, right? My reading of the specs is that an Upgrade request is still
a self-contained HTTP/1.1 request, body included.

Sorry if I've misunderstood your wording.

--Jacob

Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
Bert,

why can't you use h2c in direct mode? I assume you could store server support for this in the checkout meta data somewhere.

//Stefan

> Am 08.12.2015 um 11:30 schrieb Bert Huijben <be...@qqmail.nl>:
> 
>> -----Original Message-----
>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>> Sent: dinsdag 8 december 2015 11:07
>> To: dev@httpd.apache.org
>> Subject: Upgrade Summary
>> 
>> Trying to summarize the status of the discussion and where the issues are
>> with the current Upgrade implementation.
>> 
>> Clarified:
>> A. any 100 must be sent out *before* a 101 response
>> B. request bodies are to be read in the original protocol, input filters
> like
>> chunk can be used, indeed are necessary, as if the request is being
>> processed normally
>> C. if a protocol supports upgrade on request bodies is up to the protocol
>> implementation and needs to be checked in the "propose" phase
>> 
>> Open:
>> 1. Protocols like Websocket need to take over the 101 sending themselves
> in
>> the "switch protocol" phase. (correct, Jacob?). Should we delegate the
>> sending of the 101 to the protocol switch handler?
> 
> If possible I would recommend avoiding this. I think the original protocol
> should setup the response and then the final protocol should somehow be able
> to annotate this.
> 
> The problems we try to solve now originate from the problem of doing things
> different in different protocol handlers, while in theory many upgrades are
> very similar.
> 
> The TLS and H2C upgrades both begin in one form and end in a different form.
> Websockets are kind of different in that they require a bad request response
> in a specific case. I'm not sure in which protocol this error needs to be
> send though.
> 
> In TLS and H2C further errors can always be produced in the new protocol, so
> when the handshake succeeds things can just go on.
> 
>> 2. General handling of request bodies. Options:
>>  a setaside in core of up to nnn bytes before switch invocation
>>  b do nothing, let protocol switch handler care about it
> 
> For Subversion to be able to use upgrade we would need to support a small
> body on a request (few hundred bytes. Content-Length header provided).
> 
> During our current (already optimized) handshake all requests have bodies,
> and introducing an additional request just to upgrade will slow things down
> quite measurable on operation like 'svn log', that are mostly bound by the
> handshake time.
> 
> We can't just switch the handshake to be something else... our handshake was
> built upon WEBDAV and DELTA/V. We added several headers to avoid many
> requests we previously performed, but we can't move away from that initial
> OPTIONS request without slowing down against all older servers.
> 
> 	Bert


Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
On 12/08/2015 02:30 AM, Bert Huijben wrote:
> The TLS and H2C upgrades both begin in one form and end in a different form.
> Websockets are kind of different in that they require a bad request response
> in a specific case. I'm not sure in which protocol this error needs to be
> send though.

For WebSocket, any upgrade failures have to be sent in the original 
HTTP/1.1 protocol using HTTP error codes.

WebSocket is different from TLS/h2c because, conceptually, you're 
upgrading the connection to a specific request target (a ws:// URL). The 
"response" sent back to the client after the upgrade (if you want to 
call it a response) is simply the establishment of a WebSocket 
connection for that URL.

--Jacob

RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> Sent: dinsdag 8 december 2015 11:07
> To: dev@httpd.apache.org
> Subject: Upgrade Summary
> 
> Trying to summarize the status of the discussion and where the issues are
> with the current Upgrade implementation.
> 
> Clarified:
> A. any 100 must be sent out *before* a 101 response
> B. request bodies are to be read in the original protocol, input filters
like
> chunk can be used, indeed are necessary, as if the request is being
> processed normally
> C. if a protocol supports upgrade on request bodies is up to the protocol
> implementation and needs to be checked in the "propose" phase
> 
> Open:
> 1. Protocols like Websocket need to take over the 101 sending themselves
in
> the "switch protocol" phase. (correct, Jacob?). Should we delegate the
> sending of the 101 to the protocol switch handler?

If possible I would recommend avoiding this. I think the original protocol
should setup the response and then the final protocol should somehow be able
to annotate this.

The problems we try to solve now originate from the problem of doing things
different in different protocol handlers, while in theory many upgrades are
very similar.

The TLS and H2C upgrades both begin in one form and end in a different form.
Websockets are kind of different in that they require a bad request response
in a specific case. I'm not sure in which protocol this error needs to be
send though.

In TLS and H2C further errors can always be produced in the new protocol, so
when the handshake succeeds things can just go on.

> 2. General handling of request bodies. Options:
>   a setaside in core of up to nnn bytes before switch invocation
>   b do nothing, let protocol switch handler care about it

For Subversion to be able to use upgrade we would need to support a small
body on a request (few hundred bytes. Content-Length header provided).

During our current (already optimized) handshake all requests have bodies,
and introducing an additional request just to upgrade will slow things down
quite measurable on operation like 'svn log', that are mostly bound by the
handshake time.

We can't just switch the handshake to be something else... our handshake was
built upon WEBDAV and DELTA/V. We added several headers to avoid many
requests we previously performed, but we can't move away from that initial
OPTIONS request without slowing down against all older servers.

	Bert



Re: Upgrades

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 10.12.2015 um 02:07 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
> 
> There is a case to be made for the h2c to also be an 'early' upgrade decision,
> AIUI (correct me if I'm wrong Stefan) the mod_http2 module sees a request,
> grabs ahold of the network in the main request thread, and then 're-processes'
> the request, repeating all of the request processing hooks for the initial request
> on the secondary h2c 'worker' thread.  By interrupting very early, during the
> post_read_request phase, these steps will no longer be duplicated.

Yes, correct. Good point for making the upgrade earlier for protocols like h2c.

Re: Upgrades

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Dec 9, 2015 at 6:16 PM, Tim Bannister <is...@c8h10n4o2.org.uk>
wrote:

> On 9 Dec 2015, at 23:19, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> > Because the request body is inbound already at some state of completion
> > or incomplete transmission, it is competing with the TLS handshake, which
> > is a bidirectional engagement with the same socket between the user agent
> > and server.  Unlike h2c and websocket, where Roy suggests we leave the
> > http/1 input filter in place and inject the http/2 output filter for the
> duration
> > of the initial Upgrade: request, we must pass TLS traffic in both
> directions
> > at once during Upgrade: TLS.
> …
> > Please remember that a request handler is based on a method and location,
> > while Upgrade is not the request itself, but a proposal to switch
> protocols.
> > In the case of TLS and h2c, that request is satisfied over the
> corresponding
> > HTTP/1 or HTTP/2 output filter, but I'm not clear whether websocket has
> > any equivalence in terms of a content handler phase.
>
> In a sense, all upgrades happen after of an existing request (& response).
> So how about this high level behaviour model:
> httpd keeps note, for each connection, of what upgrades are (1) feasible
> and (2) agreed upon.
> At end of any given request, either zero or one of the feasible upgrades
> will have been agreed between web server and client. That's when the
> upgrade should happen if it has been negotiated.
>

I think that is the design we are arriving at, that each module is queried
early
if an upgrade should be advertised, and given the chance to begin the
upgrade,
and then immediately before the content handler (after auth) these steps
are
repeated, giving the protocol modules a chance to offer an upgrade or back
out of an intended offer.  The offers must be assembled before the content
handler causes the http/1 protocol filter to push out the response headers.

In the TLS case, it must happen very early for TLS
handshake/certificates/SNI
inspection for auth purposes, and prior to beginning the response.  No
upgrade
can happen after a response (except within the next request) because the
upgrade is a product of the 101-switching protocols.

There is a case to be made for the h2c to also be an 'early' upgrade
decision,
AIUI (correct me if I'm wrong Stefan) the mod_http2 module sees a request,
grabs ahold of the network in the main request thread, and then
're-processes'
the request, repeating all of the request processing hooks for the initial
request
on the secondary h2c 'worker' thread.  By interrupting very early, during
the
post_read_request phase, these steps will no longer be duplicated.

The main thread can still leave the http/1 input filter in place to provide
the
initial request body to that initial worker thread, but won't see
additional http/2
requests until that request body is consumed, completing the 101-switching
protocols contract.

The reason not to leave the initial http/1 request in the primary thread
until
it has been satisfied is that the connection would block on both the initial
request body and the initial response, which would be a huge penalty for
an initial request that performed some back-end processing and might
have database or other back-end contention.

Eg, for a new, inbound port 80 connection: an upgrade to TLS or h2c would
> be feasible. During the first request it may transpire that an upgrade to
> WebSocket is feasible too (authz having been satisfied). Once the request
> in which the upgrade has been negotiated is complete, said upgrade takes
> place.
>

For every request within a connection, such an upgrade remains feasible.


> Any subsequent requests on the same TCP connection won't be eligible for
> upgrade to WebSocket. This kind of rule ought to live outside the HTTP/1.x
> implementation as it has more to do with WebSocket than HTTP.
>

I'm wondering the same thing, why not on the third or twentieth request?
It simply can't be downgraded back to HTTP/1, if I understand the spec
correctly.

Re: Upgrades

Posted by Jacob Champion <ch...@gmail.com>.
On 12/09/2015 04:16 PM, Tim Bannister wrote:
> Any subsequent requests on the same TCP connection won't be eligible for upgrade to WebSocket.

Why not?

--Jacob


Re: Upgrades

Posted by Tim Bannister <is...@c8h10n4o2.org.uk>.
On 9 Dec 2015, at 23:19, William A Rowe Jr <wr...@rowe-clan.net> wrote:

> Because the request body is inbound already at some state of completion
> or incomplete transmission, it is competing with the TLS handshake, which
> is a bidirectional engagement with the same socket between the user agent
> and server.  Unlike h2c and websocket, where Roy suggests we leave the
> http/1 input filter in place and inject the http/2 output filter for the duration
> of the initial Upgrade: request, we must pass TLS traffic in both directions 
> at once during Upgrade: TLS.
…
> Please remember that a request handler is based on a method and location,
> while Upgrade is not the request itself, but a proposal to switch protocols.
> In the case of TLS and h2c, that request is satisfied over the corresponding 
> HTTP/1 or HTTP/2 output filter, but I'm not clear whether websocket has
> any equivalence in terms of a content handler phase.

In a sense, all upgrades happen after of an existing request (& response). So how about this high level behaviour model:
httpd keeps note, for each connection, of what upgrades are (1) feasible and (2) agreed upon.
At end of any given request, either zero or one of the feasible upgrades will have been agreed between web server and client. That's when the upgrade should happen if it has been negotiated.

Eg, for a new, inbound port 80 connection: an upgrade to TLS or h2c would be feasible. During the first request it may transpire that an upgrade to WebSocket is feasible too (authz having been satisfied). Once the request in which the upgrade has been negotiated is complete, said upgrade takes place.

Any subsequent requests on the same TCP connection won't be eligible for upgrade to WebSocket. This kind of rule ought to live outside the HTTP/1.x implementation as it has more to do with WebSocket than HTTP.


-- 
Tim Bannister – isoma@c8h10n4o2.org.uk


Re: Upgrade Summary

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Dec 11, 2015 at 2:22 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> Here I would use ap_add_output_filter(switch_protocol_filter, r); with
> switch_protocol_filter() which would flush out the 101 response (or
> not) based on r->need_upgrade and r->current_protocol, before any
> Protocols data.

This filter would be AP_FILTER_PROTOCOL+x or (low)
AP_FILTER_CONNECTION (before mod_ssl).

>
> Wouldn't that work for every case we discussed?

I did not mentioned h2c, but it could run in post_read_request right?

Re: Upgrade Summary

Posted by Yann Ylavic <yl...@gmail.com>.
On Sat, Dec 12, 2015 at 12:41 AM, Yann Ylavic <yl...@gmail.com> wrote:
> Hi Jacob,
>
> On Fri, Dec 11, 2015 at 11:47 PM, Jacob Champion <ch...@gmail.com> wrote:
>>
>> This is where we disagree, for the case of WebSocket and WebSocket alone.
>> RFC 6455 4.2.1:
>>
>>    The client's opening handshake consists of the following parts.  If
>>    the server, while reading the handshake, finds that the client did
>>    not send a handshake that matches the description below (note that as
>>    per [RFC2616], the order of the header fields is not important),
>>    including but not limited to any violations of the ABNF grammar
>>    specified for the components of the handshake, the server MUST stop
>>    processing the client's handshake and return an HTTP response with an
>>    appropriate error code (such as 400 Bad Request).
>
> I guess the RFC is talking about a Websocket server that also
> implements HTTP/1 for Websocket purpose only.
> In httpd, the protocol parsing is done by the core http module before
> any websocket module is called, so there can't (at least shouldn't) be
> an invalid HTTP/1 grammar at that level (post_read_request[_header]),
> HTTP/1 failures ought to be handled by the core (http module),
> including ill/invalid body transfer encoding/length.
> So the error could be an unacceptable handshake for the websocket
> module, which should then let httpd handle the HTTP/1 request
> (ignoring the handshake and probably websocket headers too).

Please note that httpd's default (HTTP/1) handler can always catch
"http/1" not in Protocols and return "505 Not Supported" if the
request reaches it.
The administrator configures acceptable protocols, and httpd gives
each one's module a chance, defaulting to HTTP/1 (or 505) if none
accepts the handshake.

Re: Upgrade Summary

Posted by Yann Ylavic <yl...@gmail.com>.
Hi Jacob,

On Fri, Dec 11, 2015 at 11:47 PM, Jacob Champion <ch...@gmail.com> wrote:
>
> This is where we disagree, for the case of WebSocket and WebSocket alone.
> RFC 6455 4.2.1:
>
>    The client's opening handshake consists of the following parts.  If
>    the server, while reading the handshake, finds that the client did
>    not send a handshake that matches the description below (note that as
>    per [RFC2616], the order of the header fields is not important),
>    including but not limited to any violations of the ABNF grammar
>    specified for the components of the handshake, the server MUST stop
>    processing the client's handshake and return an HTTP response with an
>    appropriate error code (such as 400 Bad Request).

I guess the RFC is talking about a Websocket server that also
implements HTTP/1 for Websocket purpose only.
In httpd, the protocol parsing is done by the core http module before
any websocket module is called, so there can't (at least shouldn't) be
an invalid HTTP/1 grammar at that level (post_read_request[_header]),
HTTP/1 failures ought to be handled by the core (http module),
including ill/invalid body transfer encoding/length.
So the error could be an unacceptable handshake for the websocket
module, which should then let httpd handle the HTTP/1 request
(ignoring the handshake and probably websocket headers too).

That does not mean that Protocols handlers can't return "applicative"
errors in handshake (eg. Protocols' auth[nz] failures), but if it
accepts/handles the handshake it MUST respond in the Upgraded
protocol, not in HTTP/1.

Regards,
Yann.

Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
n 12/11/2015 02:24 PM, William A Rowe Jr wrote:
> On Fri, Dec 11, 2015 at 2:55 PM, Jacob Champion <champion.p@gmail.com
> <ma...@gmail.com>> wrote:
>     That additional constraint doesn't conflict with any part of
>     HTTP/1.1, as far as I can tell. Returning 4xx directly to an Upgrade
>     request is allowed within HTTP/1.1, right? The spec says nothing
>     about the internal server architecture.
>
> The spec describes the exception handling;
>    "A client MAY send a list of protocols in the Upgrade
>     header field of a request to invite the server to switch to one or
>     more of those protocols, in order of descending preference, before
>     sending the final response.  A server MAY ignore a received Upgrade
>     header field if it wishes to continue using the current protocol on
>     that connection.  Upgrade cannot be used to insist on a protocol
>     change."
> Unambiguous, there is no "error" in not upgrading, period.

Is your argument that, because the spec does not explicitly say that the 
server may take some action, such an action is prohibited?

> To the extent that any RFC6455
> contradiction to RFC7230 is invalid, RFC7230 is binding,

Agreed.

> but it doesn't look that way to me.  From RFC6455 4.1,
>
>     Once the client's opening handshake has been sent, the client MUST
>     wait for a response from the server before sending any further data.
>     The client MUST validate the server's response as follows:
>
>     1.  If the status code received from the server is not 101, the
>         client handles the response per HTTP [RFC2616 <https://tools.ietf.org/html/rfc2616>] procedures.  In
>         particular, the client might perform authentication if it
>         receives a 401 status code; the server might redirect the client
>         using a 3xx status code (but clients are not required to follow
>         them), etc.  Otherwise, proceed as follows.
>
> In other words, in the absence of the websocket module declining to honor
> the upgrade request, it still remains an http/1 request and response.

Agreed. A properly implemented WebSocket client is also, by necessity, 
an HTTP/1.1 client, because the server may not have any idea what 
WebSocket is.

> Whatever auth or other technical details cause that request to fail will be
> represented as an http/1 response.

Yes.

> If the implementer wants to present a successful http/1
> response to the request in lieu of establishing a websocket connection with
> the corresponding 101-switching protocols, by offering a resource at the
> given URL, that option remains.

This is where we disagree, for the case of WebSocket and WebSocket 
alone. RFC 6455 4.2.1:

    The client's opening handshake consists of the following parts.  If
    the server, while reading the handshake, finds that the client did
    not send a handshake that matches the description below (note that as
    per [RFC2616], the order of the header fields is not important),
    including but not limited to any violations of the ABNF grammar
    specified for the components of the handshake, the server MUST stop
    processing the client's handshake and return an HTTP response with an
    appropriate error code (such as 400 Bad Request).

I maintain that this requirement (stop, do not pass Go, return an error 
immediately) is not in conflict with 723x, but I am open to persuasion 
otherwise. At the moment, I just don't agree that the paragraph you 
cited above (RFC 7230 6.7)  prohibits this behavior. Are there other 
cases you can show me where omission is tantamount to prohibition?

--Jacob

Re: Upgrade Summary

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Dec 11, 2015 at 2:55 PM, Jacob Champion <ch...@gmail.com>
wrote:

> On 12/11/2015 12:12 PM, William A Rowe Jr wrote:
>
>> On Fri, Dec 11, 2015 at 1:13 PM, Jacob Champion <champion.p@gmail.com
>> <ma...@gmail.com>> wrote:
>>
>>     On 12/11/2015 02:36 AM, Bert Huijben wrote:
>>         Not upgrading is 100% better than failing.
>>
>>     ...except for those protocols for which failure instead of upgrade
>>     is *required* by their spec (e.g. WebSocket).
>>
>> No, no protocol overrides the HTTP/1 RFCs *until* 101-switching
>> protocols has occurred at which point the new protocol definition is in
>> force, this behavior is all defined by RFC7230 section 6.7.
>>
>
> That additional constraint doesn't conflict with any part of HTTP/1.1, as
> far as I can tell. Returning 4xx directly to an Upgrade request is allowed
> within HTTP/1.1, right? The spec says nothing about the internal server
> architecture.


The spec describes the exception handling;
  "A client MAY send a list of protocols in the Upgrade
   header field of a request to invite the server to switch to one or
   more of those protocols, in order of descending preference, before
   sending the final response.  A server MAY ignore a received Upgrade
   header field if it wishes to continue using the current protocol on
   that connection.  Upgrade cannot be used to insist on a protocol
   change."
Unambiguous, there is no "error" in not upgrading, period.

If there is no underlying HTTP/1.1 resource resulting from -not- upgrading
the response, or additional constraints are in place (e.g. upgrade required,
http/1 not honored etc) then those are outside of the scope of the Protocol
switch API and fall in the domain of the normal httpd request hook scope,
IMHO.


> If there is an http:// representation of the websocket resource, that is
>> served instead of the 101-switching protocols.  If there isn't such a
>> representation, then the request of course does fail :)
>>
>
> I have to serve two masters here. All of the following are allowed by RFCs
> 723x, upon receiving a WebSocket upgrade offer:
>
> 1) Upgrade the connection to WebSocket.
> 2) Ignore the upgrade entirely, and continue processing as a normal HTTP
> request, which may or may not fail for any number of reasons.
> 3) Return a 4xx *immediately*. (This is allowed, in that it is not
> disallowed: from the point of view of 723x, it's as if the server ignored
> the upgrade but found something otherwise objectionable in the request.)
>
> RFC 6455 removes 2) from my list of choices. There is no conflict between
> the specifications here, only a reduction in available options for the
> server.
>
>     Protocol switch implementations need to have the opportunity to fail
>>     after they have been selected for that reason, but of course the TLS
>>     and HTTP/2 implementations should/would not make use of that.
>>
>> Any module can cause a request to fail for any of hundreds of reasons,
>> we must avoid that logic in the Protocol API schema because the protocol
>> handler module can deal with this in any of the request hooks it likes
>> and is most appropriate for the failure case.
>>
>
> As long as I can still satisfy 6455, I have no objection.
>
> In other words, there is one and only one recognized mechanism to establish
>> a TLS h2 connection.  I don't think we want to honor other mechanics.
>>
>
> I'm inclined to agree.
>
>     IMHO, dedicated clients who are serious about WebSocket security
>>     will have connected via a wss:// connection instead of upgrading
>>     from an initially unsecure connection. (I'm not saying it's
>>     impossible to design a secure system that uses an upgrade, but the
>>     percentage of users who can get it 100% right is likely to be low.)
>>
>> Your call as a websocket implementer, but I'd be inclined to ignore this
>> combination just as with TLS+h2c.
>>
>
> +1.


I think we are essentially on the same page.  To the extent that any RFC6455
contradiction to RFC7230 is invalid, RFC7230 is binding, but it doesn't
look that
way to me.  From RFC6455 4.1,

   Once the client's opening handshake has been sent, the client MUST
   wait for a response from the server before sending any further data.
   The client MUST validate the server's response as follows:

   1.  If the status code received from the server is not 101, the
       client handles the response per HTTP [RFC2616
<https://tools.ietf.org/html/rfc2616>] procedures.  In
       particular, the client might perform authentication if it
       receives a 401 status code; the server might redirect the client
       using a 3xx status code (but clients are not required to follow
       them), etc.  Otherwise, proceed as follows.


In other words, in the absence of the websocket module declining to honor
the upgrade request, it still remains an http/1 request and response.
Whatever
auth or other technical details cause that request to fail will be
represented as
an http/1 response.  If the implementer wants to present a successful http/1
response to the request in lieu of establishing a websocket connection with
the corresponding 101-switching protocols, by offering a resource at the
given
URL, that option remains.  If there is no resource except a websocket at
that
URL, an error will occur.

I hope that makes things clearer?

Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
On 12/11/2015 12:12 PM, William A Rowe Jr wrote:
> On Fri, Dec 11, 2015 at 1:13 PM, Jacob Champion <champion.p@gmail.com
> <ma...@gmail.com>> wrote:
>
>     On 12/11/2015 02:36 AM, Bert Huijben wrote:
>         Not upgrading is 100% better than failing.
>
>     ...except for those protocols for which failure instead of upgrade
>     is *required* by their spec (e.g. WebSocket).
>
> No, no protocol overrides the HTTP/1 RFCs *until* 101-switching
> protocols has occurred at which point the new protocol definition is in
> force, this behavior is all defined by RFC7230 section 6.7.

That additional constraint doesn't conflict with any part of HTTP/1.1, 
as far as I can tell. Returning 4xx directly to an Upgrade request is 
allowed within HTTP/1.1, right? The spec says nothing about the internal 
server architecture.

> If there is an http:// representation of the websocket resource, that is
> served instead of the 101-switching protocols.  If there isn't such a
> representation, then the request of course does fail :)

I have to serve two masters here. All of the following are allowed by 
RFCs 723x, upon receiving a WebSocket upgrade offer:

1) Upgrade the connection to WebSocket.
2) Ignore the upgrade entirely, and continue processing as a normal HTTP 
request, which may or may not fail for any number of reasons.
3) Return a 4xx *immediately*. (This is allowed, in that it is not 
disallowed: from the point of view of 723x, it's as if the server 
ignored the upgrade but found something otherwise objectionable in the 
request.)

RFC 6455 removes 2) from my list of choices. There is no conflict 
between the specifications here, only a reduction in available options 
for the server.

>     Protocol switch implementations need to have the opportunity to fail
>     after they have been selected for that reason, but of course the TLS
>     and HTTP/2 implementations should/would not make use of that.
>
> Any module can cause a request to fail for any of hundreds of reasons,
> we must avoid that logic in the Protocol API schema because the protocol
> handler module can deal with this in any of the request hooks it likes
> and is most appropriate for the failure case.

As long as I can still satisfy 6455, I have no objection.

> In other words, there is one and only one recognized mechanism to establish
> a TLS h2 connection.  I don't think we want to honor other mechanics.

I'm inclined to agree.

>     IMHO, dedicated clients who are serious about WebSocket security
>     will have connected via a wss:// connection instead of upgrading
>     from an initially unsecure connection. (I'm not saying it's
>     impossible to design a secure system that uses an upgrade, but the
>     percentage of users who can get it 100% right is likely to be low.)
>
> Your call as a websocket implementer, but I'd be inclined to ignore this
> combination just as with TLS+h2c.

+1.

--Jacob


Re: Upgrade Summary

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Dec 11, 2015 at 1:13 PM, Jacob Champion <ch...@gmail.com>
wrote:

> On 12/11/2015 02:36 AM, Bert Huijben wrote:
>
>> -----Original Message-----
>>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>>> Protocol implementations should make up their minds in the "propose"
>>> phase, I think,
>>> because once a protocol gets selected, the upgrade *should* succeed
>>> unless
>>> the
>>> connection catches fire or something. Not upgrading is better than
>>>
>> failing.
>>
>> +1
>> Not upgrading is 100% better than failing.
>>
>
> ...except for those protocols for which failure instead of upgrade is
> *required* by their spec (e.g. WebSocket).


No, no protocol overrides the HTTP/1 RFCs *until* 101-switching protocols
has occurred at which point the new protocol definition is in force, this
behavior is all defined by RFC7230 section 6.7.

If there is an http:// representation of the websocket resource, that is
served instead of the 101-switching protocols.  If there isn't such a
representation, then the request of course does fail :)


> Protocol switch implementations need to have the opportunity to fail after
> they have been selected for that reason, but of course the TLS and HTTP/2
> implementations should/would not make use of that.
>

Any module can cause a request to fail for any of hundreds of reasons, we
must avoid that logic in the Protocol API schema because the protocol
handler module can deal with this in any of the request hooks it likes and
is most appropriate for the failure case.

One other thing that sticks in my mind:
>> Is it possible to upgrade to TLS in one request and to h2c later?
>>
>
> I believe so; is there anything in the specs that would make you think
> otherwise? An Upgrade: TLS by itself still results in an HTTP/1.1
> connection, which can respond to further upgrades, right?
>

I was under the impression that particular case was disallowed by the
HTTP/2 RFC, but I could be mistaken.


> I'll go further though: is it possible to upgrade to TLS and then to h2c
> in the *same* request?:
>
>    Upgrade: TLS/1.2, h2c


By RFC 7230, yes (and in the order you illustrated - "if multiple protocol
layers are being switched, the sender MUST list the protocols in
layer-ascending order.").

But by RFC 7540 I think the answer is a clear "no"...

   A server MUST ignore an "h2" token in an Upgrade header field.
   Presence of a token with "h2" implies HTTP/2 over TLS, which is
   instead negotiated as described in Section 3.3
<https://tools.ietf.org/html/rfc7540#section-3.3>.


3.3 <https://tools.ietf.org/html/rfc7540#section-3.3>.  Starting
HTTP/2 for "https" URIs

   A client that makes a request to an "https" URI uses TLS [TLS12
<https://tools.ietf.org/html/rfc7540#ref-TLS12>] with
   the application-layer protocol negotiation (ALPN) extension
   [TLS-ALPN <https://tools.ietf.org/html/rfc7540#ref-TLS-ALPN>].

   HTTP/2 over TLS uses the "h2" protocol identifier.  The "h2c"
   protocol identifier MUST NOT be sent by a client or selected by a
   server; the "h2c" protocol identifier describes a protocol that does
   not use TLS.


In other words, there is one and only one recognized mechanism to establish
a TLS h2 connection.  I don't think we want to honor other mechanics.

TLS and then websockets...
>> I don't think that scenario should be supported either. Webbrowsers aren't
>> going to use it... and dedicated clients probably use a direct connection
>> of
>> some sort instead of websockets.
>>
>
> IMHO, dedicated clients who are serious about WebSocket security will have
> connected via a wss:// connection instead of upgrading from an initially
> unsecure connection. (I'm not saying it's impossible to design a secure
> system that uses an upgrade, but the percentage of users who can get it
> 100% right is likely to be low.)


Your call as a websocket implementer, but I'd be inclined to ignore this
combination
just as with TLS+h2c.

Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
On 12/11/2015 02:36 AM, Bert Huijben wrote:
>> -----Original Message-----
>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>> Protocol implementations should make up their minds in the "propose"
>> phase, I think,
>> because once a protocol gets selected, the upgrade *should* succeed unless
>> the
>> connection catches fire or something. Not upgrading is better than
> failing.
>
> +1
> Not upgrading is 100% better than failing.

...except for those protocols for which failure instead of upgrade is 
*required* by their spec (e.g. WebSocket). Protocol switch 
implementations need to have the opportunity to fail after they have 
been selected for that reason, but of course the TLS and HTTP/2 
implementations should/would not make use of that.

> One other thing that sticks in my mind:
> Is it possible to upgrade to TLS in one request and to h2c later?

I believe so; is there anything in the specs that would make you think 
otherwise? An Upgrade: TLS by itself still results in an HTTP/1.1 
connection, which can respond to further upgrades, right?

I'll go further though: is it possible to upgrade to TLS and then to h2c 
in the *same* request?:

    Upgrade: TLS/1.2, h2c

> TLS and then websockets...
> I don't think that scenario should be supported either. Webbrowsers aren't
> going to use it... and dedicated clients probably use a direct connection of
> some sort instead of websockets.

IMHO, dedicated clients who are serious about WebSocket security will 
have connected via a wss:// connection instead of upgrading from an 
initially unsecure connection. (I'm not saying it's impossible to design 
a secure system that uses an upgrade, but the percentage of users who 
can get it 100% right is likely to be low.)

--Jacob

RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> Sent: vrijdag 11 december 2015 10:20
> To: dev@httpd.apache.org
> Subject: Re: Upgrade Summary


> Regarding request bodies:
> - websocket will never switch on request bodies
> - h2c currently does not, but clients like serf (and curl) really would
prefer it to
> - TLS could switch with arbitrary request bodies, but maybe need not to
> Protocol implementations should make up their minds in the "propose"
phase, I
> think,
> because once a protocol gets selected, the upgrade *should* succeed unless
> the
> connection catches fire or something. Not upgrading is better than
failing.

+1
Not upgrading is 100% better than failing.

But for Subversion not upgrading will most likely also imply not retrying
later on the same connection.

The way Subversion uses requests implies that it needs to pipeline requests.
With h2 it can do this much more efficient and stable than with http/1.1,
but not pipelining will make operations to remote repositories unusable
slow. (And I think most of us at least sometimes use the ASF master
repository from a distance:-)


So once we get past that initial request on an operation like update,
checkout or merge we will start opening multiple http/1.1 connections and
pipelining requests on those connections without further upgrade headers.
With requests leaving and responses arriving out of sync there is no way to
upgrade later on.




One other thing that sticks in my mind:
Is it possible to upgrade to TLS in one request and to h2c later?
(The other direction is explicitly forbidden by h2)

I'm not going to implement this for the pipelining reasons noted above, but
perhaps other scenarios might want this. But more likely  we might want to
explicitly block this scenario.

TLS and then websockets...
I don't think that scenario should be supported either. Webbrowsers aren't
going to use it... and dedicated clients probably use a direct connection of
some sort instead of websockets.

	Bert


Re: Upgrade Summary

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Dec 11, 2015 at 10:20 AM, Stefan Eissing
<st...@greenbytes.de> wrote:
>
>> Am 11.12.2015 um 02:22 schrieb Yann Ylavic <yl...@gmail.com>:
>>
>> On Thu, Dec 10, 2015 at 11:46 AM, Stefan Eissing
>> <st...@greenbytes.de> wrote:
>>> Given all the input on this thread, I arrive at the following pseudo code:
>>
>> Thanks for _compiling_ this thread, quite exhaustive :)
>
> Code often says more than a thousand...tokens.

++1 (though I won't propose code this time, possibly soon :p )

>
> Currently, TLS and current core step on each others toes when it
> comes to the Upgrade response header. This we should fix.

Agreed.

>
> If it turns out that TLS, h2c and websocket all need the actual
> switching at different phases of request processing, then we need
> to accomodate for that.
>
> What I can see:
> - TLS and h2c should switch in post_read_req
> - websocket should switch just before the handler phase

Yes, they could use one of those phase, their choice to hook wherever
they need to.

In my proposal, they'd rely on the core's Upgrade output filter being
in the chain (which will be the case since the core's
post_read_request (really-)first hook would have run).
The Upgrade output filter would catch the first brigade sent to the
client and issue the 101-Upgrade response first before removing itself
from the chain and forwarding the brigade.

- TLS (post_read_request):
0. make a decision when a request body exists (either bail out,
discard or return 413),
1. enable SSL/TLS engine and filters,
2. call ap_pass_brigade(FLUSH) for the Upgrade to occur,
3. return DECLINED.

- h2c/ws (wherever, post_read_request/[pre_]handler):
0. make a decision when a request body exists (either bail out,
discard or use it!),
1. run their process_request(),
2. return DONE.

As far as they use ap_{get,pass}_brigade() for reading/writing data
this should work.
They wouldn't do anything (ie. return DECLINED early) if
r->selected_protocol != their protocol.
If some headers are needed in the 101 response, one can fill in
r->headers_out for the core Upgrade output filter to use that when
Upgrading.

>
> Protocol implementations should make up their minds in the "propose" phase, I think,
> because once a protocol gets selected, the upgrade *should* succeed unless the
> connection catches fire or something. Not upgrading is better than failing.

Yes, all could be based on (communication via) current/selected proto
(set by the core!) for decisions, each module can use that to
determine if it was elected or not.
Whether the core Upgrade output filter would do the Upgrade or not can
be driven by a r->flag too (or anything else, special bucket, ...),
the module really handling the Protocols would set it unless the offer
is finally DECLINED (for some reason).

>> +        && ap_find_token(r->pool, upgrade, "TLS")
>> +        && !ap_has_request_body(r)) {
>> []
>
> I think the first way, don't Upgrade if you know it will not work, is better.

Agreed, let's do that in ssl_hook_ReadReq(), along with:
+        && !ap_has_request_body(r)
+        && !strcasecmp(r->selected_protocol, "TLS")) {
...

>>> 1. Post Read Request Hook:
>>>
>>>        if (Upgrade: request header present) {
>>>                collect protocol proposals;
>>>                ps = protocol with highest preference from proposals;
>>>                if (ps && ps != current) {
>>
>> Here I would use ap_add_output_filter(switch_protocol_filter, r); with
>> switch_protocol_filter() which would flush out the 101 response (or
>> not) based on r->need_upgrade and r->current_protocol, before any
>> Protocols data.
>
> I am not convinced that a flushing is necessary. If a protocol needs
> this, it can just pass out a flush bucket, or?

Yes, this is what I meant, as maybe better explained above.

>
> h2c needs to receive request body separate, get an indication
> that EOS for that body has been reached,

ap_discard_request_body()?

> and then continue reading
> HTTP/2 protocol data on the connection. I am not sure what is the
> best way to do that and, when upgrading on post_read_req hook, if
> all necessary request filters are in place by then.

I think so with my proposal, unless I'm missing something (which is
ofter the case ;)

>
>> For the headers in the 101 response, another hook called from
>> switch_protocol_filter() could be used.
>
> That'd be the pre_switch hook Jacob and I talked about and where he
> posted his implementation yesterday.

We wouldn't need it actually, as said above, why not simply use
r->headers_out while we are still HTTP/1...


Thanks for the feedbacks, Stefan.

Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 11.12.2015 um 02:22 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Thu, Dec 10, 2015 at 11:46 AM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>> Given all the input on this thread, I arrive at the following pseudo code:
> 
> Thanks for _compiling_ this thread, quite exhaustive :)

Code often says more than a thousand...tokens.

> I wonder if we could let each Protocols module hook wherever
> appropriate in the current hooking set.
> 
> TLS handshake is already at the right place IMO, it possibly needs a
> simple fix like,

I am not an expert on where TLS handshake would fit best. It would
be good, and we are on the way of working on it, to have a good
Upgrade handling in core, where TLS, h2c and websocket all are
comfortable with.

Currently, TLS and current core step on each others toes when it
comes to the Upgrade response header. This we should fix.

If it turns out that TLS, h2c and websocket all need the actual 
switching at different phases of request processing, then we need
to accomodate for that.

What I can see:
- TLS and h2c should switch in post_read_req
- websocket should switch just before the handler phase

Regarding request bodies:
- websocket will never switch on request bodies
- h2c currently does not, but clients like serf (and curl) really would prefer it to
- TLS could switch with arbitrary request bodies, but maybe need not to
Protocol implementations should make up their minds in the "propose" phase, I think,
because once a protocol gets selected, the upgrade *should* succeed unless the
connection catches fire or something. Not upgrading is better than failing.



> Index: modules/ssl/ssl_engine_kernel.c
> ===================================================================
> --- modules/ssl/ssl_engine_kernel.c    (revision 1718341)
> +++ modules/ssl/ssl_engine_kernel.c    (working copy)
> @@ -233,7 +233,8 @@ int ssl_hook_ReadReq(request_rec *r)
>      * has sent a suitable Upgrade header. */
>     if (sc->enabled == SSL_ENABLED_OPTIONAL && !myConnConfig(r->connection)
>         && (upgrade = apr_table_get(r->headers_in, "Upgrade")) != NULL
> -        && ap_find_token(r->pool, upgrade, "TLS/1.0")) {
> +        && ap_find_token(r->pool, upgrade, "TLS")
> +        && !ap_has_request_body(r)) {
>         if (upgrade_connection(r)) {
>             return AP_FILTER_ERROR;
>         }
> --
> so that we don't Upgrade when a body is given (looks like it's RFC
> compliant since Upgrade is not mandatory).
> 
> Or maybe,
> -        && ap_find_token(r->pool, upgrade, "TLS/1.0")) {
> +        && ap_find_token(r->pool, upgrade, "TLS")) {
> +        if (ap_has_request_body(r)) {
> +            return HTTP_REQUEST_ENTITY_TOO_LARGE;
> +        }
>         if (upgrade_connection(r)) {
>             return AP_FILTER_ERROR;
>         }
> --
> because a body in clear text while requiring TLS makes few sense, and
> clients that send TLS body directly (no header) after the Upgrade
> would notice (now), that looks safer.
> But stricly speaking this looks not very RFC7230 compliant too, I
> could not find an "Upgrade: TLS" exception there (the whole HTTP/1
> request must be read otherwise before upgrade)...
> 
> Possibly we could also,
> +            ap_discard_request_body(r);
> but I'd feel safer with the previous patch, WDYT?

I think the first way, don't Upgrade if you know it will not work, is better.

>> [...]
>> 1. Post Read Request Hook:
>> 
>>        if (Upgrade: request header present) {
>>                collect protocol proposals;
>>                ps = protocol with highest preference from proposals;
>>                if (ps && ps != current) {
> 
> Here I would use ap_add_output_filter(switch_protocol_filter, r); with
> switch_protocol_filter() which would flush out the 101 response (or
> not) based on r->need_upgrade and r->current_protocol, before any
> Protocols data.

I am not convinced that a flushing is necessary. If a protocol needs
this, it can just pass out a flush bucket, or?

> Maybe this filter could also call ap_discard_request_body(r) when
> needed/asked to (eg. r->discard_body), that'd need CONN_SENSE handling
> with MPM event possibly.

Some safeguard+comfort filter magic to make protocol's life with 
Upgrade request bodies easier would be nice. 

h2c needs to receive request body separate, get an indication 
that EOS for that body has been reached, and then continue reading 
HTTP/2 protocol data on the connection. I am not sure what is the 
best way to do that and, when upgrading on post_read_req hook, if
all necessary request filters are in place by then.

> For the headers in the 101 response, another hook called from
> switch_protocol_filter() could be used.

That'd be the pre_switch hook Jacob and I talked about and where he
posted his implementation yesterday.

>> [...]
> 
> A handler could use ap_get_brigade(r->input_filters) and friends more
> easily, or ap_discard_request_body() explicitely, and/or
> ap_pass_brigade(FLUSH) to force the 101 response, ..., and/or go to :

Yes, maybe that is easier.

> 
>>                // c->output_filters can be written
>>                // c->input_filters can be read [...]
>>                process protocol;
> 
> Followed by:
>                  r->keepalive = AP_CONN_CLOSE;
>                  return OK;
>>        }
> 
> Wouldn't that work for every case we discussed?

I though the TLS wanted to switch protocol and then let normal HTTP/1
processing continue on the connection?

//Stefan


RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.
I’m not talking about why the client does want the upgrade…

Perhaps it doesn’t know if port 443 uses TLS or hosts the same content. In general it is just a lucky guess that it does have the same content. The Alt-Svc spec might help here, but currently this still requires the admin to configure things… and I don’t see that change anytime soon. (And if it would be automatic, it would still assume no firewalls, certificate availability, etc., etc.)


What I’m trying to say is: we should not just deny connections an upgrade for no reason. If we can upgrade the connection with a body… why explicitly deny it *after* we already received the body? If we want to reuse the connection we have to read the body anyway.


We can’t make the connection more secure by asking the client to retry the request… We can only make things less secure that way.


Returning a 4XX error just makes the client retry the request (perhaps after asking the user). The 4XX will be more work for the server, more work for the client and perhaps even for the end user.

We don’t make the web more secure by denying upgrades after the request is already sent. And as http/1.1 doesn’t have a way to stop a request before it is sent without resetting the connection I would say that we upgrade to TLS and/or H2c whenever possible.

The server can’t decide if the client should send the request unencrypted or not *after* it is already on the wire unencrypted.

By not upgrading we *as server* decide that the next request on the same connection will be unencrypted as well… By returning a 4XX (or other error) we ask the client to retry again; most likely on the same connection and just as unencrypted as the previous request.

On disconnecting we do the same thing….

The only way to ensure encryption or more advanced protocol support is honoring the upgrade request as soon as possible.

The deadlock scenarios on bodies that can’t be sent while the request is read are no reason to just block upgrades… These same deadlocks can occur for dozens of different reasons and in case of h2 even after upgrade. (Don’t send window updates and everything stalls… by design). At that point we can handle the error, just like how we handle timeouts on http/1.1

With h2 we can even force stream 1 (the response for the upgraded request) to close with an appropriate stream error in that case… so we can still reuse the connection.


With h2 you can in theory start the response in h2 while you are still reading the http/1.1 body, while with TLS you can’t as you need a two way handshake.

There will be cases where the upgrade path definitely breaks (e.g. huge/infinite chunked request echoed as response), but I think in most cases it can succeed without a problem. Just returning an error in easy cases for consistency with the cases where it can’t work doesn’t really help.



The problem with errors like 413 is that in clients like Subversion we can only handle them by showing them as fatal error to the user. The printer scenario for upgrades to TLS probably does the same thing. Printing works or doesn’t work… It doesn’t allow retrying with a different request.

Not upgrading is an option… But upgrading under less favorable conditions is what makes things work.

      Bert

Sent from Mail for Windows 10



From: Yann Ylavic
Sent: zaterdag 12 december 2015 01:46
To: Bert Huijben
Subject: Re: Upgrade Summary


On Sat, Dec 12, 2015 at 12:48 AM, Bert Huijben <be...@qqmail.nl> wrote:
> If you request an upgrade to TLS on your initial request, upgrading with a
> body might still make sense. Especially if the server would respond with a
> 401. But also if the request can be public, but the response needs to be
> secured.

Why using Upgrade at the first place if you are confident enough (to
play auth) with the server being TLS ready?
Wouldn't a direct https connection be better?

Is there such an authentication protocol?
If so, I guess we can do the TLS Upgrade+handshake in the output
filter (as proposed) like any other Protocols Upgrade, letting httpd
handle that first clear text request body...
That's possible without setting aside the body anyway, still I doubt
there is a real need for it (if a TLS Upgrade is asked, the client
shouldn't mind the first (half) round trip and play it's auth on the
second (TLSed) request.

>
> If we blindly ignore the upgrade as ‘doesn’t make sense’, the next request
> wouldn’t use encryption… but if we upgraded to TLS after the request, the
> next request… with the authentication headers could be sent encrypted.

My preference would be to return an error (413), so that the client
notices what's "better" for a TLS Upgrade (no body), but if there
really exist such a cleartext body use case I'm fine with honoring the
body too.

>
> If upgrading the first request doesn’t make sense the client should use a
> different request (Like options *, or HEAD /).

That are, AFAICT, the only use cases so far.

>
> If the server denies the request at least some parts have already travelled
> the network unencrypted. Returning an error or not upgrading will only make
> sure more requests will travel unencrypted.

The client would not send encrypted vs unencrypted (Upgrade) request
depending on the response status, it must send plaintext request
anyway, it ought to know that.
IOW the error response is not a security issue at all, on the contrary
if a client relies on current httpd behaviour to do the handshake
before reading the body, and hence sends private things in the body of
the Upgrade request, it would notice with a response error.
Anyway if the RFC is respected, the TLS handshake doesn't happen
before any body is (fully) received on the server, hence such client
would wait for the handshake before sending anything, and that would
timeout.
Again, an error response is better here...

>
> The response is not the only thing encrypted when upgrading… all future
> requests are. Upgrade is a connection level request, sent via a request.

Agreed.

Regards,
Yann.



RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.
If you request an upgrade to TLS on your initial request, upgrading with a body might still make sense. Especially if the server would respond with a 401. But also if the request can be public, but the response needs to be secured.

If we blindly ignore the upgrade as ‘doesn’t make sense’, the next request wouldn’t use encryption… but if we upgraded to TLS after the request, the next request… with the authentication headers could be sent encrypted.

The server can’t choose if that first request makes sense or not… It has to decide if upgrading the connection during the request makes sense… and if that is possible.

If upgrading the first request doesn’t make sense the client should use a different request (Like options *, or HEAD /).

If the server denies the request at least some parts have already travelled the network unencrypted. Returning an error or not upgrading will only make sure more requests will travel unencrypted.

The response is not the only thing encrypted when upgrading… all future requests are. Upgrade is a connection level request, sent via a request.


Bert


Sent from Mail for Windows 10



From: Yann Ylavic
Sent: vrijdag 11 december 2015 02:22
To: httpd-dev
Subject: Re: Upgrade Summary


On Thu, Dec 10, 2015 at 11:46 AM, Stefan Eissing
<st...@greenbytes.de> wrote:
> Given all the input on this thread, I arrive at the following pseudo code:

Thanks for _compiling_ this thread, quite exhaustive :)

I wonder if we could let each Protocols module hook wherever
appropriate in the current hooking set.

TLS handshake is already at the right place IMO, it possibly needs a
simple fix like,

Index: modules/ssl/ssl_engine_kernel.c
===================================================================
--- modules/ssl/ssl_engine_kernel.c    (revision 1718341)
+++ modules/ssl/ssl_engine_kernel.c    (working copy)
@@ -233,7 +233,8 @@ int ssl_hook_ReadReq(request_rec *r)
      * has sent a suitable Upgrade header. */
     if (sc->enabled == SSL_ENABLED_OPTIONAL && !myConnConfig(r->connection)
         && (upgrade = apr_table_get(r->headers_in, "Upgrade")) != NULL
-        && ap_find_token(r->pool, upgrade, "TLS/1.0")) {
+        && ap_find_token(r->pool, upgrade, "TLS")
+        && !ap_has_request_body(r)) {
         if (upgrade_connection(r)) {
             return AP_FILTER_ERROR;
         }
--
so that we don't Upgrade when a body is given (looks like it's RFC
compliant since Upgrade is not mandatory).

Or maybe,
-        && ap_find_token(r->pool, upgrade, "TLS/1.0")) {
+        && ap_find_token(r->pool, upgrade, "TLS")) {
+        if (ap_has_request_body(r)) {
+            return HTTP_REQUEST_ENTITY_TOO_LARGE;
+        }
         if (upgrade_connection(r)) {
             return AP_FILTER_ERROR;
         }
--
because a body in clear text while requiring TLS makes few sense, and
clients that send TLS body directly (no header) after the Upgrade
would notice (now), that looks safer.
But stricly speaking this looks not very RFC7230 compliant too, I
could not find an "Upgrade: TLS" exception there (the whole HTTP/1
request must be read otherwise before upgrade)...

Possibly we could also,
+            ap_discard_request_body(r);
but I'd feel safer with the previous patch, WDYT?


Regarding WebSocket, we already have proxy_wstunnel that handshakes
successfully as a (proxy_)handler.
I don't know if the upcoming body is to be HTTP/1 or not, but should
this be enforced we could use ap_get_brigade() to forward it until
complete (while still detecting HTTP/1 "errors"), and then use the
current TCP forwarding until EOF on either side.


>
> 1. Post Read Request Hook:
>
>         if (Upgrade: request header present) {
>                 collect protocol proposals;
>                 ps = protocol with highest preference from proposals;
>                 if (ps && ps != current) {

Here I would use ap_add_output_filter(switch_protocol_filter, r); with
switch_protocol_filter() which would flush out the 101 response (or
not) based on r->need_upgrade and r->current_protocol, before any
Protocols data.
Maybe this filter could also call ap_discard_request_body(r) when
needed/asked to (eg. r->discard_body), that'd need CONN_SENSE handling
with MPM event possibly.
For the headers in the 101 response, another hook called from
switch_protocol_filter() could be used.

If I'm talking about new r-> field, that could also be in
core_request_config, reachable with
ap_get_core_module_config(r->request_config), for possibly a better
backportabily (but that's an implementation detail).

So now I think we can let the request fall through.
If a Protocols' module needs to bypass/handle auth[nz] itself, it
would hook after this one (post_read_request still, and return DONE).
Otherwise, as a handler, each Protocols' module would check
r->current_protocol against its own one, and either handle it or
DECLINE.
If the selected module doesn't care about the request body (ie. never
reads it), switch_protocol_filter() would do the right thing (discard
the body) before switching.
Otherwise, the module can do whatever it wants with the (full) HTTP/1
request, and even DECLINE if it finally wants to fall through HTTP/1
(after resetting r->need_upgrade probably).

>
> 3. Common protocol switch hook behavior:
>
>         apr_status_t process_body(apr_bucket_brigade *bb, void *ctx)
>         {
>                 // brigade contains unchunked body data of request
>                 // may be called several times. EOS bucket in brigade
>                 // indicates end of body.
>                 // If request has no body, will be called once with EOS bucket.

A handler could use ap_get_brigade(r->input_filters) and friends more
easily, or ap_discard_request_body() explicitely, and/or
ap_pass_brigade(FLUSH) to force the 101 response, ..., and/or go to :

>
>                 // c->output_filters can be written
>                 // c->input_filters can be read [...]
>                 process protocol;

Followed by:
                  r->keepalive = AP_CONN_CLOSE;
                  return OK;
>         }

Wouldn't that work for every case we discussed?



Re: Upgrade Summary

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Dec 10, 2015 at 11:46 AM, Stefan Eissing
<st...@greenbytes.de> wrote:
> Given all the input on this thread, I arrive at the following pseudo code:

Thanks for _compiling_ this thread, quite exhaustive :)

I wonder if we could let each Protocols module hook wherever
appropriate in the current hooking set.

TLS handshake is already at the right place IMO, it possibly needs a
simple fix like,

Index: modules/ssl/ssl_engine_kernel.c
===================================================================
--- modules/ssl/ssl_engine_kernel.c    (revision 1718341)
+++ modules/ssl/ssl_engine_kernel.c    (working copy)
@@ -233,7 +233,8 @@ int ssl_hook_ReadReq(request_rec *r)
      * has sent a suitable Upgrade header. */
     if (sc->enabled == SSL_ENABLED_OPTIONAL && !myConnConfig(r->connection)
         && (upgrade = apr_table_get(r->headers_in, "Upgrade")) != NULL
-        && ap_find_token(r->pool, upgrade, "TLS/1.0")) {
+        && ap_find_token(r->pool, upgrade, "TLS")
+        && !ap_has_request_body(r)) {
         if (upgrade_connection(r)) {
             return AP_FILTER_ERROR;
         }
--
so that we don't Upgrade when a body is given (looks like it's RFC
compliant since Upgrade is not mandatory).

Or maybe,
-        && ap_find_token(r->pool, upgrade, "TLS/1.0")) {
+        && ap_find_token(r->pool, upgrade, "TLS")) {
+        if (ap_has_request_body(r)) {
+            return HTTP_REQUEST_ENTITY_TOO_LARGE;
+        }
         if (upgrade_connection(r)) {
             return AP_FILTER_ERROR;
         }
--
because a body in clear text while requiring TLS makes few sense, and
clients that send TLS body directly (no header) after the Upgrade
would notice (now), that looks safer.
But stricly speaking this looks not very RFC7230 compliant too, I
could not find an "Upgrade: TLS" exception there (the whole HTTP/1
request must be read otherwise before upgrade)...

Possibly we could also,
+            ap_discard_request_body(r);
but I'd feel safer with the previous patch, WDYT?


Regarding WebSocket, we already have proxy_wstunnel that handshakes
successfully as a (proxy_)handler.
I don't know if the upcoming body is to be HTTP/1 or not, but should
this be enforced we could use ap_get_brigade() to forward it until
complete (while still detecting HTTP/1 "errors"), and then use the
current TCP forwarding until EOF on either side.


>
> 1. Post Read Request Hook:
>
>         if (Upgrade: request header present) {
>                 collect protocol proposals;
>                 ps = protocol with highest preference from proposals;
>                 if (ps && ps != current) {

Here I would use ap_add_output_filter(switch_protocol_filter, r); with
switch_protocol_filter() which would flush out the 101 response (or
not) based on r->need_upgrade and r->current_protocol, before any
Protocols data.
Maybe this filter could also call ap_discard_request_body(r) when
needed/asked to (eg. r->discard_body), that'd need CONN_SENSE handling
with MPM event possibly.
For the headers in the 101 response, another hook called from
switch_protocol_filter() could be used.

If I'm talking about new r-> field, that could also be in
core_request_config, reachable with
ap_get_core_module_config(r->request_config), for possibly a better
backportabily (but that's an implementation detail).

So now I think we can let the request fall through.
If a Protocols' module needs to bypass/handle auth[nz] itself, it
would hook after this one (post_read_request still, and return DONE).
Otherwise, as a handler, each Protocols' module would check
r->current_protocol against its own one, and either handle it or
DECLINE.
If the selected module doesn't care about the request body (ie. never
reads it), switch_protocol_filter() would do the right thing (discard
the body) before switching.
Otherwise, the module can do whatever it wants with the (full) HTTP/1
request, and even DECLINE if it finally wants to fall through HTTP/1
(after resetting r->need_upgrade probably).

>
> 3. Common protocol switch hook behavior:
>
>         apr_status_t process_body(apr_bucket_brigade *bb, void *ctx)
>         {
>                 // brigade contains unchunked body data of request
>                 // may be called several times. EOS bucket in brigade
>                 // indicates end of body.
>                 // If request has no body, will be called once with EOS bucket.

A handler could use ap_get_brigade(r->input_filters) and friends more
easily, or ap_discard_request_body() explicitely, and/or
ap_pass_brigade(FLUSH) to force the 101 response, ..., and/or go to :

>
>                 // c->output_filters can be written
>                 // c->input_filters can be read [...]
>                 process protocol;

Followed by:
                  r->keepalive = AP_CONN_CLOSE;
                  return OK;
>         }

Wouldn't that work for every case we discussed?

Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 10.12.2015 um 14:57 schrieb Mike Rumph <mi...@oracle.com>:
> 
> Question below:
> 
> On 12/10/2015 2:46 AM, Stefan Eissing wrote:
>> 	int xxx_switch(conn_rec *c, request_rec *r, server_rec *s,
>> 		const char *protocol, int phase)
>> 	{
>> 		apr_status_t stats;
>> 
>> 		if (strcmp(my_protocol, protocol)) {
> Do you mean "if my_protocol is protocol"?

If the protocol to switch to is not mine, DECLINE.

Should have written that in prose...

>> 			return DECLINED;
>> 		}
>> 		
>> 		if (need_handler && phase != handler) {
>> 			return APR_EAGAIN;
>> 		}
>> 
>> 		status = ap_upgrade_request(r, protocol, phase, my_additional_header, process_body);
>> 		if (status != APR_SUCCESS) {
>> 			// any type of error, could also fail if NULL is passed instead of
>> 			// &process_body and request has body
>> 			return status;
>> 		}
>> 
>> 		// c->output_filters can be written
>> 		// c->input_filters can be read, will pass data to process_body() until body is done,
>> 		// then return data up the filter chain.
>> 		process protocol;
>> 
>> 		return APR_EOF;
>> 	}
>> 
>> 
> 


Re: Upgrade Summary

Posted by Mike Rumph <mi...@oracle.com>.
Question below:

On 12/10/2015 2:46 AM, Stefan Eissing wrote:
> 	int xxx_switch(conn_rec *c, request_rec *r, server_rec *s,
> 		const char *protocol, int phase)
> 	{
> 		apr_status_t stats;
>
> 		if (strcmp(my_protocol, protocol)) {
Do you mean "if my_protocol is protocol"?
> 			return DECLINED;
> 		}
> 		
> 		if (need_handler && phase != handler) {
> 			return APR_EAGAIN;
> 		}
>
> 		status = ap_upgrade_request(r, protocol, phase, my_additional_header, process_body);
> 		if (status != APR_SUCCESS) {
> 			// any type of error, could also fail if NULL is passed instead of
> 			// &process_body and request has body
> 			return status;
> 		}
>
> 		// c->output_filters can be written
> 		// c->input_filters can be read, will pass data to process_body() until body is done,
> 		// then return data up the filter chain.
> 		process protocol;
>
> 		return APR_EOF;
> 	}
>
>


RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> Sent: donderdag 10 december 2015 11:47
> To: dev@httpd.apache.org
> Subject: Re: Upgrade Summary
> 
> Given all the input on this thread, I arrive at the following pseudo code:
> 
> 1. Post Read Request Hook:
> 
> 	if (Upgrade: request header present) {
> 		collect protocol proposals;
> 		ps = protocol with highest preference from proposals;
> 	        if (ps && ps != current) {
> 			status = switch protocol(phase => post_read);
> 			if (status == APR_EOF) {
> 				close connection;
> 			}
> 			else if (status == APR_EAGAIN) {
> 				// protocol switch wants to be called later
> before handler
> 				if (request is "OPTIONS *") {
> 					// TODO: invoke again with (phase =>
> handler)?

Why handle 'OPTIONS *' different here?

Isn't that 'just another simple HTTP/1.1 request'?


>From what I read about that request, it is a recommended easy request if you
don't have anything to send yourself in this stage. (With "HEAD /" as
alternative).

If not required I would recommend not hardcoding specific behavior on this
request. There is not much that makes it that different from "GET /" or
"HEAD /", or every other request that doesn't have a request body.

	Bert


Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
Given all the input on this thread, I arrive at the following pseudo code:

1. Post Read Request Hook:
   
	if (Upgrade: request header present) {
		collect protocol proposals;
		ps = protocol with highest preference from proposals;
	        if (ps && ps != current) {
			status = switch protocol(phase => post_read);
			if (status == APR_EOF) {
				close connection;
			}
			else if (status == APR_EAGAIN) {
				// protocol switch wants to be called later before handler
				if (request is "OPTIONS *") {
					// TODO: invoke again with (phase => handler)?
				}
				request->upgrade_protocol = ps;
			}
			else {
				// continue, HTTP/1 processing. May happen on TLS Upgrade
				// or if the protocol managed to switch back to HTTP/1
				// process other stati than APR_SUCCESS as usual
			}
		}
        	else if ("http/1.1" not in Protocols) {
			answer "505 Not Supported";
		}
	}
	else if ("http/1.1" not in Protocols) {
		answer "426 Upgrade Required";
	}

2. Handler Hook (First or just before):

	if (r->upgrade_protocol) {
		status = switch protocol(phase => handler);
		if (status == APR_EOF) {
			close connection;
		}
		else {
			// continue, HTTP/1 processing. May happen on TLS Upgrade
			// or if the protocol managed to switch back to HTTP/1
			// process other stati than APR_SUCCESS as usual
		}
	}

3. Common protocol switch hook behavior:

	apr_status_t process_body(apr_bucket_brigade *bb, void *ctx)
	{
		// brigade contains unchunked body data of request
		// may be called several times. EOS bucket in brigade
		// indicates end of body.
		// If request has no body, will be called once with EOS bucket.
	}

	int xxx_switch(conn_rec *c, request_rec *r, server_rec *s, 
		const char *protocol, int phase)
	{
		apr_status_t stats;

		if (strcmp(my_protocol, protocol)) {
			return DECLINED;
		}
		
		if (need_handler && phase != handler) {
			return APR_EAGAIN;
		}

		status = ap_upgrade_request(r, protocol, phase, my_additional_header, process_body);
		if (status != APR_SUCCESS) {
			// any type of error, could also fail if NULL is passed instead of
			// &process_body and request has body
			return status;
		}

		// c->output_filters can be written
		// c->input_filters can be read, will pass data to process_body() until body is done,
		// then return data up the filter chain.
		process protocol;

		return APR_EOF;
	}


Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
Jacob, thanks for the code! I will let this and my pseudo code stew over the weekend and then probably next week start the changes to upgrade handling.

//Stefan

> Am 10.12.2015 um 23:18 schrieb Jacob Champion <ch...@gmail.com>:
> 
> Okay, I finally have actual code to share. This is the original experimental pre_protocol_switch hook that Stefan and I were talking about a while ago [1], rebased onto 2.4.18. The two patches are available at
> 
>    https://github.com/jchampio/httpd/commits/dev/websocket-protocols
> 
> It is *not* an implementation of Stefan's most recent pseudocode (a primary difference is that his proposal does the filter + upgrade in one hook instead of two), and it doesn't solve many of the use cases that you've already agreed must be handled:
> 
> - 100-continue is not handled correctly
> - the incoming request body is not correctly dealt with
> - there is only one switch point, at the handler stage
> - etc.
> 
> *But* it is a working implementation from mod_websocket's point of view, so I offer it up primarily for informational purposes. The module making use of the new hook is available at
> 
> https://github.com/jchampio/apache-websocket/blob/dev/hook/mod_websocket.c
> 
> You might be most interested in the mod_websocket_pre_protocol_switch() implementation (line 1627), which contains the pre-upgrade checks I need to do to remain compliant with RFC 6455.
> 
> == Asides ==
> 
> 1) The current Protocols API does not correctly ignore upgrades from HTTP/1.0 requests; the latest commit to my experimental branch patched this.
> 
> 2) As Stefan and I discussed in [1], Upgrade header values are case-sensitive (correct?), but the RFC 6455 declares that servers should respond to any protocol that's a case-insensitive match for "websocket". To make matters worse, the official IANA upgrade token is "WebSocket", but the RFC uses "websocket" throughout. I have seen both used in the wild (Autobahn uses WebSocket; Firefox uses websocket), and I'm not sure what the right way to resolve this is.
> 
> Hope this helps. Again, I don't expect this to actually be part of the final solution, but sometimes it's nice to see code.
> 
> --Jacob
> 
> [1] https://mail-archives.apache.org/mod_mbox/httpd-dev/201509.mbox/%3C55FF85F0.8040106@gmail.com%3E


Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
Okay, I finally have actual code to share. This is the original 
experimental pre_protocol_switch hook that Stefan and I were talking 
about a while ago [1], rebased onto 2.4.18. The two patches are available at

     https://github.com/jchampio/httpd/commits/dev/websocket-protocols

It is *not* an implementation of Stefan's most recent pseudocode (a 
primary difference is that his proposal does the filter + upgrade in one 
hook instead of two), and it doesn't solve many of the use cases that 
you've already agreed must be handled:

- 100-continue is not handled correctly
- the incoming request body is not correctly dealt with
- there is only one switch point, at the handler stage
- etc.

*But* it is a working implementation from mod_websocket's point of view, 
so I offer it up primarily for informational purposes. The module making 
use of the new hook is available at

 
https://github.com/jchampio/apache-websocket/blob/dev/hook/mod_websocket.c

You might be most interested in the mod_websocket_pre_protocol_switch() 
implementation (line 1627), which contains the pre-upgrade checks I need 
to do to remain compliant with RFC 6455.

== Asides ==

1) The current Protocols API does not correctly ignore upgrades from 
HTTP/1.0 requests; the latest commit to my experimental branch patched this.

2) As Stefan and I discussed in [1], Upgrade header values are 
case-sensitive (correct?), but the RFC 6455 declares that servers should 
respond to any protocol that's a case-insensitive match for "websocket". 
To make matters worse, the official IANA upgrade token is "WebSocket", 
but the RFC uses "websocket" throughout. I have seen both used in the 
wild (Autobahn uses WebSocket; Firefox uses websocket), and I'm not sure 
what the right way to resolve this is.

Hope this helps. Again, I don't expect this to actually be part of the 
final solution, but sometimes it's nice to see code.

--Jacob

[1] 
https://mail-archives.apache.org/mod_mbox/httpd-dev/201509.mbox/%3C55FF85F0.8040106@gmail.com%3E

Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
On 12/10/2015 02:45 AM, Bert Huijben wrote:
> Websockets are different… and in some ways not really an upgrade of the
> connection… more like a hostile takeover with a final operation to a target

Ha! Yes -- although from the point of view of HTTP/1.1, all successful 
upgrades are hostile takeovers, no? But a failure to upgrade is more of 
a problem for a WebSocket client than a pure-HTTP client.

> More like a CONNECT request to a proxy…

Heh, interesting that you mention that. Proxy and cache behavior is, 
IIRC, the motivation for many of the strange-looking design decisions in 
the WebSocket protocol...

--Jacob

RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.
Great to see where this discussion is headed:

+1 on the last design ideas.

 

Going with one ‘as early as possible’ upgrade and one ‘upgrade last’ should handle all these cases just fine.

 

I don’t think the h2c and TLS cases really have to be that different as suggested in the earlier parts of the discussion. Both want to upgrade as soon as possible, which is +- after the initial request is read, and before the response is generated.

 

TLS then really needs to exchange information both ways, while for h2c allowing exchange both ways allows doing some things a bit earlier.

 

 

Websockets are different… and in some ways not really an upgrade of the connection… more like a hostile takeover with a final operation to a targetJ

 

More like a CONNECT request to a proxy… or perhaps a PRI request to a HTTP/1.1 server that handles h2direct that way… one final request until the connection closes.

 

                Bert

 

 

From: William A Rowe Jr [mailto:wrowe@rowe-clan.net] 
Sent: donderdag 10 december 2015 07:40
To: httpd <de...@httpd.apache.org>
Subject: Re: Upgrade Summary

 

On Wed, Dec 9, 2015 at 9:22 PM, Jacob Champion <champion.p@gmail.com <ma...@gmail.com> > wrote:

On 12/09/2015 05:19 PM, William A Rowe Jr wrote:

 

    _If_ all the other protocols worked like WebSocket and required
    authnz before an upgrade could succeed, it wouldn't make sense for
    each upgrade handler to have to do the authnz check themselves. But
    in this case, WebSocket is different enough that I think it will
    probably have to.

No, because we will simply give you two chances to accept and trigger
the upgrade, once in the post_read_request, another in ap_invoke_handler
phase before filters are inserted.


Here you make it sound as if I won't have to duplicate the authnz checks in the handler phase, but in a previous email you said

websocket can ignore the Upgrade: websocket offer after inspecting authnz


Did I misunderstand? I feel like I'm missing something crucial to your point.

 

I propose two chances to catch the upgrade, early and late.  You need auth,

therefore you must inspect the late catch, which follows authnz but precedes 

the handler invocation.

 

If a websocket protocol module determines that authnz denied websocket

but not the resource, then it can proceed to the normal http/1 handler without

responding with a 101-switching protocols.  But if the request meets the

requirements to be handled by the websocket protocol, you reply to the

Protocol API with an 'upgrade accepted' and take ownership of the request

and the connection.

 

Sounds reasonable.

I'd certainly like to see a websocket prototype
or outline before we declare the protocol baked for the second time :)


Working on it. :) My original experimental hook for 2.4.17 rebased nicely onto 2.4.18, but mod_websocket has been refactored significantly since I wrote that and I couldn't reuse my work. Sorry I've been slow in actual code-writing; I've been under the weather this week.

 

No worries, 2.4.18 is an incremental set of improvements, and 2.4.19

will be moreso, but not in the next week :) 

 


Re: Upgrade Summary

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Dec 9, 2015 at 9:22 PM, Jacob Champion <ch...@gmail.com> wrote:

> On 12/09/2015 05:19 PM, William A Rowe Jr wrote:
>
>>
>>     _If_ all the other protocols worked like WebSocket and required
>>     authnz before an upgrade could succeed, it wouldn't make sense for
>>     each upgrade handler to have to do the authnz check themselves. But
>>     in this case, WebSocket is different enough that I think it will
>>     probably have to.
>>
>> No, because we will simply give you two chances to accept and trigger
>> the upgrade, once in the post_read_request, another in ap_invoke_handler
>> phase before filters are inserted.
>>
>
> Here you make it sound as if I won't have to duplicate the authnz checks
> in the handler phase, but in a previous email you said
>
> websocket can ignore the Upgrade: websocket offer after inspecting authnz
>>
>
> Did I misunderstand? I feel like I'm missing something crucial to your
> point.


I propose two chances to catch the upgrade, early and late.  You need auth,
therefore you must inspect the late catch, which follows authnz but
precedes
the handler invocation.

If a websocket protocol module determines that authnz denied websocket
but not the resource, then it can proceed to the normal http/1 handler
without
responding with a 101-switching protocols.  But if the request meets the
requirements to be handled by the websocket protocol, you reply to the
Protocol API with an 'upgrade accepted' and take ownership of the request
and the connection.


> Sounds reasonable.
>
> I'd certainly like to see a websocket prototype
>> or outline before we declare the protocol baked for the second time :)
>>
>
> Working on it. :) My original experimental hook for 2.4.17 rebased nicely
> onto 2.4.18, but mod_websocket has been refactored significantly since I
> wrote that and I couldn't reuse my work. Sorry I've been slow in actual
> code-writing; I've been under the weather this week.


No worries, 2.4.18 is an incremental set of improvements, and 2.4.19
will be moreso, but not in the next week :)

Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
On 12/09/2015 05:19 PM, William A Rowe Jr wrote:
>     The "handler phase" is simply the establishment of a bidirectional
>     WebSocket connection. If the client did something pathological like
>     sending a request body with their GET request, I think that needs to
>     go into the bit bucket before the upgrade completes.
>
> If that is permissible under the websocket API, then fine.

There's no mention of it either way. I think most people forget that GET 
requests can have (useless?) bodies, anyway.

>     _If_ all the other protocols worked like WebSocket and required
>     authnz before an upgrade could succeed, it wouldn't make sense for
>     each upgrade handler to have to do the authnz check themselves. But
>     in this case, WebSocket is different enough that I think it will
>     probably have to.
>
> No, because we will simply give you two chances to accept and trigger
> the upgrade, once in the post_read_request, another in ap_invoke_handler
> phase before filters are inserted.

Here you make it sound as if I won't have to duplicate the authnz checks 
in the handler phase, but in a previous email you said

> websocket can ignore the Upgrade: websocket offer after inspecting authnz

Did I misunderstand? I feel like I'm missing something crucial to your 
point.

> But you are painting all the other protocols into a corner.  TLS will resume
> the http/1 request on the same thread.

Cool, that answers my question then ("Are there any cases where, after a 
successful upgrade, we would not want to close the connection upon 
returning from the protocol handler?"). If TLS is able to switch the 
filters around and have the stack continue as normal, then that sounds 
like a good reason not to do what I was suggesting. ;D

> the h2c or websocket protocol marks
> the connection: close (or already closed/eof, depending) which prevents
> the original http/1 handler from treating the connection as a keep-alive
> connection.  In the websocket case, I expect you can do that as step #1
> in the protocol handler to address any paranoia about falling back into the
> request processing cycle.

Sounds reasonable.

> I'd certainly like to see a websocket prototype
> or outline before we declare the protocol baked for the second time :)

Working on it. :) My original experimental hook for 2.4.17 rebased 
nicely onto 2.4.18, but mod_websocket has been refactored significantly 
since I wrote that and I couldn't reuse my work. Sorry I've been slow in 
actual code-writing; I've been under the weather this week.

--Jacob


Re: Upgrade Summary

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Dec 9, 2015 at 6:50 PM, Jacob Champion <ch...@gmail.com> wrote:

> On 12/09/2015 03:19 PM, William A Rowe Jr wrote:
>
>> Because the request body is inbound already at some state of completion
>> or incomplete transmission, it is competing with the TLS handshake, which
>> is a bidirectional engagement with the same socket between the user agent
>> and server.  Unlike h2c and websocket, where Roy suggests we leave the
>> http/1 input filter in place and inject the http/2 output filter for the
>> duration
>> of the initial Upgrade: request, we must pass TLS traffic in both
>> directions
>> at once during Upgrade: TLS.
>>
>> Also, the results of the TLS handshake are interesting to authnz
>> configuration
>> and need to be available throughout those request phases.  This means that
>> for any Upgrade: TLS request must be satisfied (101 switching protocols)
>> prior to those auth phases.
>>
>> In the case of h2c and websocket, the http/1.1 request headers are
>> sufficient
>> to begin processing the request. At the end of the first request, the
>> input
>> filter is also toggled into the new protocol to resume processing of all
>> subsequent requests.  So this Upgrade: handling must occur before the
>> request handler is invoked, even a simple handler like 'OPTIONS *',
>>
>
> Great, thanks for the clarification.
>
> Please remember that a request handler is based on a method and location,
>> while Upgrade is not the request itself, but a proposal to switch
>> protocols.
>>
>
> I... think I agree with you, but I'm not clear on the point you're trying
> to make. It is true that an Upgrade is a proposal to switch protocols. It
> is also true that an Upgrade is associated with a specific HTTP/1.1
> request, with a specific request-target.
>
> The latter point is (more?) important for protocols like WebSocket. When
> you do a [GET /blah + Upgrade: h2c], the resulting protocol (HTTP/2) has
> access to the full global scope of the server after the upgrade. Even if
> /blah doesn't exist(!), the upgrade can proceed, and I can use the
> resulting connection to perform other HTTP/2 requests.
>
> In contrast, a WebSocket upgrade limits the scope of the connection to
> whatever resource is at the request-target. If I have two WebSocket
> endpoints at /ws1 and /ws2, and I do a [GET /ws1 + Upgrade: websocket], I
> can't use the resulting connection to later switch to the resource at /ws2.
> I'm stuck with whatever is at /ws1 until I start a completely new
> connection.


That was my understanding, thanks for clarifying.


> In the case of TLS and h2c, that request is satisfied over the
>> corresponding
>> HTTP/1 or HTTP/2 output filter, but I'm not clear whether websocket has
>> any equivalence in terms of a content handler phase.
>>
>
> The "handler phase" is simply the establishment of a bidirectional
> WebSocket connection. If the client did something pathological like sending
> a request body with their GET request, I think that needs to go into the
> bit bucket before the upgrade completes.


If that is permissible under the websocket API, then fine.  If the
websocket
protocol disallows request bodies altogether, that's also fine.  You simply
ignore the Upgrade: request and we continue to process the request as
http/1.  If there is no http/1 representation, the appropriate error should
be
emitted in or before the handler phase.


>     Sounds reasonable. Unfortunate in the sense that it's possible for a
>>     module implementing the Upgrade API to screw up and accidentally
>>     ignore authnz, but since most protocols we're talking about can
>>     safely upgrade first and respond with 401/3 on the new protocol, I
>>     don't see much of an alternative.
>>
>> Howso?  Any module can have any bug, I don't see your objection.
>>
>
> Some APIs are easier to write bugs with than others. All else being equal,
> I have a personal preference for strong/defensive API boundaries over
> weaker/looser ones, that's all. Especially for security-related code.
>
> _If_ all the other protocols worked like WebSocket and required authnz
> before an upgrade could succeed, it wouldn't make sense for each upgrade
> handler to have to do the authnz check themselves. But in this case,
> WebSocket is different enough that I think it will probably have to.
>

No, because we will simply give you two chances to accept and trigger
the upgrade, once in the post_read_request, another in ap_invoke_handler
phase before filters are inserted.  A websocket protocol handler will likely
choose the later, obviously.  TLS and quite possibly h2c should choose
the former.  Other Upgrade: handers to be added later would make their
respective choice based on what they need to accomplish.


>     Why leave it up to the protocol handler (to potentially get wrong)?
>>     Are there any cases where, after a successful upgrade, we would not
>>     want to close the connection upon returning from the protocol handler?
>>
>> Why are you focused on such bugs?  It isn't making any sense.
>>
>
> Same reason as before. Reducing the number of places an upgrade handler
> can get it wrong by strengthening the API's guarantees is a Good Thing,
> IMHO, as long as the cost of doing that is reasonable and we aren't
> painting ourselves into a corner.


But you are painting all the other protocols into a corner.  TLS will resume
the http/1 request on the same thread.  h2c will resume the http/1 request
either in the same thread (problematic) or in an h2 worker thread.  That
request_rec will indicate completion, the h2c or websocket protocol marks
the connection: close (or already closed/eof, depending) which prevents
the original http/1 handler from treating the connection as a keep-alive
connection.  In the websocket case, I expect you can do that as step #1
in the protocol handler to address any paranoia about falling back into the
request processing cycle.

Good coding style should prevent the scenarios you describe, that's why
we leverage peer review, much like the recent review of the protocol api by
so many of the httpd devs.  I'd certainly like to see a websocket prototype
or outline before we declare the protocol baked for the second time :)

Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
On 12/09/2015 03:19 PM, William A Rowe Jr wrote:
> Because the request body is inbound already at some state of completion
> or incomplete transmission, it is competing with the TLS handshake, which
> is a bidirectional engagement with the same socket between the user agent
> and server.  Unlike h2c and websocket, where Roy suggests we leave the
> http/1 input filter in place and inject the http/2 output filter for the
> duration
> of the initial Upgrade: request, we must pass TLS traffic in both
> directions
> at once during Upgrade: TLS.
>
> Also, the results of the TLS handshake are interesting to authnz
> configuration
> and need to be available throughout those request phases.  This means that
> for any Upgrade: TLS request must be satisfied (101 switching protocols)
> prior to those auth phases.
>
> In the case of h2c and websocket, the http/1.1 request headers are
> sufficient
> to begin processing the request. At the end of the first request, the input
> filter is also toggled into the new protocol to resume processing of all
> subsequent requests.  So this Upgrade: handling must occur before the
> request handler is invoked, even a simple handler like 'OPTIONS *',

Great, thanks for the clarification.

> Please remember that a request handler is based on a method and location,
> while Upgrade is not the request itself, but a proposal to switch protocols.

I... think I agree with you, but I'm not clear on the point you're 
trying to make. It is true that an Upgrade is a proposal to switch 
protocols. It is also true that an Upgrade is associated with a specific 
HTTP/1.1 request, with a specific request-target.

The latter point is (more?) important for protocols like WebSocket. When 
you do a [GET /blah + Upgrade: h2c], the resulting protocol (HTTP/2) has 
access to the full global scope of the server after the upgrade. Even if 
/blah doesn't exist(!), the upgrade can proceed, and I can use the 
resulting connection to perform other HTTP/2 requests.

In contrast, a WebSocket upgrade limits the scope of the connection to 
whatever resource is at the request-target. If I have two WebSocket 
endpoints at /ws1 and /ws2, and I do a [GET /ws1 + Upgrade: websocket], 
I can't use the resulting connection to later switch to the resource at 
/ws2. I'm stuck with whatever is at /ws1 until I start a completely new 
connection.

> In the case of TLS and h2c, that request is satisfied over the
> corresponding
> HTTP/1 or HTTP/2 output filter, but I'm not clear whether websocket has
> any equivalence in terms of a content handler phase.

The "handler phase" is simply the establishment of a bidirectional 
WebSocket connection. If the client did something pathological like 
sending a request body with their GET request, I think that needs to go 
into the bit bucket before the upgrade completes.

>     Sounds reasonable. Unfortunate in the sense that it's possible for a
>     module implementing the Upgrade API to screw up and accidentally
>     ignore authnz, but since most protocols we're talking about can
>     safely upgrade first and respond with 401/3 on the new protocol, I
>     don't see much of an alternative.
>
> Howso?  Any module can have any bug, I don't see your objection.

Some APIs are easier to write bugs with than others. All else being 
equal, I have a personal preference for strong/defensive API boundaries 
over weaker/looser ones, that's all. Especially for security-related code.

_If_ all the other protocols worked like WebSocket and required authnz 
before an upgrade could succeed, it wouldn't make sense for each upgrade 
handler to have to do the authnz check themselves. But in this case, 
WebSocket is different enough that I think it will probably have to.

>     Why leave it up to the protocol handler (to potentially get wrong)?
>     Are there any cases where, after a successful upgrade, we would not
>     want to close the connection upon returning from the protocol handler?
>
> Why are you focused on such bugs?  It isn't making any sense.

Same reason as before. Reducing the number of places an upgrade handler 
can get it wrong by strengthening the API's guarantees is a Good Thing, 
IMHO, as long as the cost of doing that is reasonable and we aren't 
painting ourselves into a corner.

--Jacob

Re: Upgrade Summary

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Wed, Dec 9, 2015 at 1:57 PM, Jacob Champion <ch...@gmail.com> wrote:

> On 12/09/2015 09:17 AM, William A Rowe Jr wrote:
>
>> On Tue, Dec 8, 2015 at 9:10 PM, Roy T. Fielding <fielding@gbiv.com
>> <ma...@gbiv.com>> wrote:
>>
>>     This should be easily handled by adding a filter that translates the
>>     HTTP/1
>>     incoming body (if any) to a single channel of the new protocol.
>>     Just fake it.
>>     There is no need to wait or set aside the bytes, unless that is
>>     desired for
>>     other reasons (e.g., denial of denial of service attacks).
>>
>> However, we must read the request body complete before the input handler
>> is toggled to the h2c state, or before we switch to any bidirectional
>> protocol, e.g. TLS or otherwise.
>>
>
> For my own clarification: this is a protocol-specific limitation, right?
> As in, you can't begin a TLS handshake without first draining the incoming
> request body off the connection.


Because the request body is inbound already at some state of completion
or incomplete transmission, it is competing with the TLS handshake, which
is a bidirectional engagement with the same socket between the user agent
and server.  Unlike h2c and websocket, where Roy suggests we leave the
http/1 input filter in place and inject the http/2 output filter for the
duration
of the initial Upgrade: request, we must pass TLS traffic in both
directions
at once during Upgrade: TLS.

Also, the results of the TLS handshake are interesting to authnz
configuration
and need to be available throughout those request phases.  This means that
for any Upgrade: TLS request must be satisfied (101 switching protocols)
prior to those auth phases.

In the case of h2c and websocket, the http/1.1 request headers are
sufficient
to begin processing the request. At the end of the first request, the input
filter is also toggled into the new protocol to resume processing of all
subsequent requests.  So this Upgrade: handling must occur before the
request handler is invoked, even a simple handler like 'OPTIONS *',

Please remember that a request handler is based on a method and location,
while Upgrade is not the request itself, but a proposal to switch protocols.
In the case of TLS and h2c, that request is satisfied over the
corresponding
HTTP/1 or HTTP/2 output filter, but I'm not clear whether websocket has
any equivalence in terms of a content handler phase.

    > 3. When to do the upgrade dance:
>>     >  a post_read_request: upgrade precedes authentication
>>     >  b handler: upgrade only honored on authenticated and otherwise ok
>> requests
>>     >  c both: introduce separate hooks? have an additional parameter?
>> more complexity
>>
>>     (a).  We do want to upgrade non-ok responses.  If the "new" protocol
>>     wants to
>>     send a canned HTTP/1.1 error, it can do so without our help.
>>
>> +1 in the case of h2c, TLS.  I understand that websocket upgrades are
>> going to be conditional on authentication. As long as we give websocket
>> the chance to upgrade in the fixups or similar phase, after auth, then
>> websocket can ignore the Upgrade: websocket offer after inspecting authnz.
>>
>
> Sounds reasonable. Unfortunate in the sense that it's possible for a
> module implementing the Upgrade API to screw up and accidentally ignore
> authnz, but since most protocols we're talking about can safely upgrade
> first and respond with 401/3 on the new protocol, I don't see much of an
> alternative.
>

Howso?  Any module can have any bug, I don't see your objection.

I think we agree that TLS upgrades happen early, h2c/websocket upgrades
happen late (before the content handler is invoked), and if either the
protocol
module or an auth module has a bug, we fix the bugs as with any other code
in the server.  Right?

    > 4. status code of protocol switch handler: if we move the task of 101
>> sending, the switch handler might not do it and keep the connection on the
>> "old" protocol. Then a connection close is not necessary. So, we would do
>> the close only when the switch handler returns APR_EOF.
>>
>>     Eh?
>>
>> I'm wondering the same, the new protocol loop can do all of the
>> connection handling, no need for the protocol switch to do anything but
>> continue, if the connection is ended in the protocol handler, it's gone.
>>
>
> Why leave it up to the protocol handler (to potentially get wrong)? Are
> there any cases where, after a successful upgrade, we would not want to
> close the connection upon returning from the protocol handler?


Why are you focused on such bugs?  It isn't making any sense.  If the
module sends a response or indicates the connection is closed, there is
nothing more that happens in the request processing loop anyways.  This
isn't for the core Protocol API to solve, IMHO.

Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
On 12/09/2015 09:17 AM, William A Rowe Jr wrote:
> On Tue, Dec 8, 2015 at 9:10 PM, Roy T. Fielding <fielding@gbiv.com
> <ma...@gbiv.com>> wrote:
>
>     > On Dec 8, 2015, at 2:07 AM, Stefan Eissing <stefan.eissing@greenbytes.de <ma...@greenbytes.de>>
>     wrote:
>     >
>     > Open:
>     > 1. Protocols like Websocket need to take over the 101 sending themselves in the "switch protocol" phase. (correct, Jacob?). Should we delegate the sending of the 101 to the protocol switch handler?
>
>     That seems unlikely.
>
>
> Agreed, anywhere we like we can send the 101 with headers, all that the
> websocket module needed to do was to set up output_headers or perhaps we
> add intermediate_output_headers in the request structure.  Easily fixed
> so this code is still common...

+1.

>     This should be easily handled by adding a filter that translates the
>     HTTP/1
>     incoming body (if any) to a single channel of the new protocol.
>     Just fake it.
>     There is no need to wait or set aside the bytes, unless that is
>     desired for
>     other reasons (e.g., denial of denial of service attacks).
>
> However, we must read the request body complete before the input handler
> is toggled to the h2c state, or before we switch to any bidirectional
> protocol, e.g. TLS or otherwise.

For my own clarification: this is a protocol-specific limitation, right? 
As in, you can't begin a TLS handshake without first draining the 
incoming request body off the connection.

>
>     > 3. When to do the upgrade dance:
>     >  a post_read_request: upgrade precedes authentication
>     >  b handler: upgrade only honored on authenticated and otherwise ok requests
>     >  c both: introduce separate hooks? have an additional parameter? more complexity
>
>     (a).  We do want to upgrade non-ok responses.  If the "new" protocol
>     wants to
>     send a canned HTTP/1.1 error, it can do so without our help.
>
>
> +1 in the case of h2c, TLS.  I understand that websocket upgrades are
> going to be conditional on authentication. As long as we give websocket
> the chance to upgrade in the fixups or similar phase, after auth, then
> websocket can ignore the Upgrade: websocket offer after inspecting authnz.

Sounds reasonable. Unfortunate in the sense that it's possible for a 
module implementing the Upgrade API to screw up and accidentally ignore 
authnz, but since most protocols we're talking about can safely upgrade 
first and respond with 401/3 on the new protocol, I don't see much of an 
alternative.

>     > 4. status code of protocol switch handler: if we move the task of 101 sending, the switch handler might not do it and keep the connection on the "old" protocol. Then a connection close is not necessary. So, we would do the close only when the switch handler returns APR_EOF.
>
>     Eh?
>
> I'm wondering the same, the new protocol loop can do all of the
> connection handling, no need for the protocol switch to do anything but
> continue, if the connection is ended in the protocol handler, it's gone.

Why leave it up to the protocol handler (to potentially get wrong)? Are 
there any cases where, after a successful upgrade, we would not want to 
close the connection upon returning from the protocol handler?

--Jacob


Re: Upgrade Summary

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, Dec 8, 2015 at 9:10 PM, Roy T. Fielding <fi...@gbiv.com> wrote:

> > On Dec 8, 2015, at 2:07 AM, Stefan Eissing <st...@greenbytes.de>
> wrote:
> >
> > Open:
> > 1. Protocols like Websocket need to take over the 101 sending themselves
> in the "switch protocol" phase. (correct, Jacob?). Should we delegate the
> sending of the 101 to the protocol switch handler?
>
> That seems unlikely.


Agreed, anywhere we like we can send the 101 with headers, all that the
websocket module needed to do was to set up output_headers or perhaps we
add intermediate_output_headers in the request structure.  Easily fixed so
this code is still common...


> > 2. General handling of request bodies. Options:
> >  a setaside in core of up to nnn bytes before switch invocation
> >  b do nothing, let protocol switch handler care about it
>
> I think folks are confusing implementation with protocol.  There is no need
> for the protocol being read on a connection to be the same protocol that is
> being written in response.  In other words, the incoming connection remains
> reading HTTP/1 bits until the message is finished, regardless of the
> decision
> to upgrade the response stream -- the other protocol engine doesn't care.
>

+1, between HTTP/1 and HTTP/2, or between HTTP/1 and websocket.  (And we
already worked out that websocket upgrade passes no request body anyways,
right?)


> This should be easily handled by adding a filter that translates the HTTP/1
> incoming body (if any) to a single channel of the new protocol.  Just fake
> it.
> There is no need to wait or set aside the bytes, unless that is desired for
> other reasons (e.g., denial of denial of service attacks).
>

However, we must read the request body complete before the input handler is
toggled to the h2c state, or before we switch to any bidirectional
protocol, e.g. TLS or otherwise.

> 3. When to do the upgrade dance:
> >  a post_read_request: upgrade precedes authentication
> >  b handler: upgrade only honored on authenticated and otherwise ok
> requests
> >  c both: introduce separate hooks? have an additional parameter? more
> complexity
>
> (a).  We do want to upgrade non-ok responses.  If the "new" protocol wants
> to
> send a canned HTTP/1.1 error, it can do so without our help.
>

+1 in the case of h2c, TLS.  I understand that websocket upgrades are going
to be conditional on authentication. As long as we give websocket the
chance to upgrade in the fixups or similar phase, after auth, then
websocket can ignore the Upgrade: websocket offer after inspecting authnz.


> > 4. status code of protocol switch handler: if we move the task of 101
> sending, the switch handler might not do it and keep the connection on the
> "old" protocol. Then a connection close is not necessary. So, we would do
> the close only when the switch handler returns APR_EOF.
>
> Eh?


I'm wondering the same, the new protocol loop can do all of the connection
handling, no need for the protocol switch to do anything but continue, if
the connection is ended in the protocol handler, it's gone.


> > 5. Will it be possible to migrate the current TLS upgrade handling to
> this revised scheme?
>
> TLS upgrade is never done with a body (and is broken code anyway).  Just
> fix it.
>

Fixing (leveraging set-aside code that already exists in mod_ssl for TLS
protocol renegotiation)... I see nothing in either RFC that suggests that
upgrade to TLS with a request body is not permitted.  You may be right
about existing CUPS clients.

Note that the upgrade token is "TLS" -- it does not have to be "TLS/1.0".
>

Funny, just working on this w.r.t. TLS vs TLS/1 vs TLS/1.x  - we need to
accept any of the above.  We should reject TLS/1.3, and TLS/2 however,
whenever mod_ssl/openssl is not configured handshake those protocols.

I interpret the token version as the minimum acceptable handshake, e.g. any
client that will handshake only TLS/1.1 or TLS/1.2 would send Upgrade: TLS
or Upgrade: TLS/1 or Upgrade: TLS/1.1. The TLS handshake itself will
present the best possible protocol level, while successfully negotiating
this earlier protocol.  We also must advertise TLS/1.1 (or TLS/1.2) if
mod_ssl/openssl is not configured to accept TLS/1.0 (or TLS/1.1)
handshakes. am I misinterpreting this?

Re: Upgrade Summary

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
> On Dec 8, 2015, at 2:07 AM, Stefan Eissing <st...@greenbytes.de> wrote:
> 
> Trying to summarize the status of the discussion and where the issues are with the current Upgrade implementation.
> 
> Clarified:
> A. any 100 must be sent out *before* a 101 response
> B. request bodies are to be read in the original protocol, input filters like chunk can be used, indeed are necessary, as if the request is being processed normally

Yes.

> C. if a protocol supports upgrade on request bodies is up to the protocol implementation and needs to be checked in the "propose" phase

In some respects, yes, but Upgrade is defined by HTTP/1 and no other
protocol applies until after it is done.  That means ignoring any
idiotic handshake requirements of the "new" protocol that aren't
consistent with HTTP/1.  It also means h2's requirements on Upgrade
are irrelevant except for when it requires not upgrading to h2.

The client's assumption must be that the Upgrade will fail and any
attempt to use Expect will timeout, so the entire message will be
sent eventually regardless of anyone's stupid hacks to try and game
the protocol.  Hence, the server has no choice but to receive the
entire message as HTTP/1.1 even if it thinks it could have responded
fast enough to interrupt the client in mid-send.

> 
> Open:
> 1. Protocols like Websocket need to take over the 101 sending themselves in the "switch protocol" phase. (correct, Jacob?). Should we delegate the sending of the 101 to the protocol switch handler?

That seems unlikely.

> 2. General handling of request bodies. Options:
>  a setaside in core of up to nnn bytes before switch invocation
>  b do nothing, let protocol switch handler care about it

I think folks are confusing implementation with protocol.  There is no need
for the protocol being read on a connection to be the same protocol that is
being written in response.  In other words, the incoming connection remains
reading HTTP/1 bits until the message is finished, regardless of the decision
to upgrade the response stream -- the other protocol engine doesn't care.

This should be easily handled by adding a filter that translates the HTTP/1
incoming body (if any) to a single channel of the new protocol.  Just fake it.
There is no need to wait or set aside the bytes, unless that is desired for
other reasons (e.g., denial of denial of service attacks).

> 3. When to do the upgrade dance:
>  a post_read_request: upgrade precedes authentication
>  b handler: upgrade only honored on authenticated and otherwise ok requests
>  c both: introduce separate hooks? have an additional parameter? more complexity

(a).  We do want to upgrade non-ok responses.  If the "new" protocol wants to
send a canned HTTP/1.1 error, it can do so without our help.

> 4. status code of protocol switch handler: if we move the task of 101 sending, the switch handler might not do it and keep the connection on the "old" protocol. Then a connection close is not necessary. So, we would do the close only when the switch handler returns APR_EOF.

Eh?

> 5. Will it be possible to migrate the current TLS upgrade handling to this revised scheme?

TLS upgrade is never done with a body (and is broken code anyway).  Just fix it.
Note that the upgrade token is "TLS" -- it does not have to be "TLS/1.0".

....Roy


Re: Upgrade Summary

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Tue, Dec 8, 2015 at 3:43 PM, Jacob Champion <ch...@gmail.com> wrote:

> On 12/08/2015 01:03 PM, Jacob Champion wrote:
>
>> - The module that won the proposal is given one last chance to check the
>> incoming request and fail the upgrade with an immediate HTTP response.
>>
>
> And to add to this, for completeness: mod_websocket also needs to add any
> required headers (e.g. Sec-WebSocket-Accept/Protocol/Extensions) to the 101
> response during this same conceptual "check" phase.


It seems prudent in the API design to allow the target of the switch to also
specify any exceptional 100-continue interim response headers as well as
these 101-switching protocols interim response headers.

Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
On 12/08/2015 01:03 PM, Jacob Champion wrote:
> - The module that won the proposal is given one last chance to check the
> incoming request and fail the upgrade with an immediate HTTP response.

And to add to this, for completeness: mod_websocket also needs to add 
any required headers (e.g. Sec-WebSocket-Accept/Protocol/Extensions) to 
the 101 response during this same conceptual "check" phase.

--Jacob

Re: Upgrade Summary

Posted by Jacob Champion <ch...@gmail.com>.
On 12/08/2015 02:07 AM, Stefan Eissing wrote:
> C. if a protocol supports upgrade on request bodies is up to the protocol implementation and needs to be checked in the "propose" phase

Only if the module's intent is to simply ignore upgrade requests if they 
have bodies. Other modules might choose to propose the protocol anyway, 
and explicitly fail the request with a 4xx if the upgrade comes to them.

> 1. Protocols like Websocket need to take over the 101 sending themselves in the "switch protocol" phase. (correct, Jacob?). Should we delegate the sending of the 101 to the protocol switch handler?

It's more that I need to be able to interrupt the upgrade if necessary 
-- whether that's handled by a separate hook, or by delegating the 
upgrade entirely to me, is up for debate. I worry about the duplication 
of the Upgrade code if you delegate that to the modules though... seems 
like core should understand how to upgrade, and handle it for the modules.

In my head the ideal handshake API goes like this:

- Request comes in with a proposed set of protocols to Upgrade to.
- Core uses the propose hook to get a list of protocols that are valid 
upgrades for the current request target. A protocol is chosen.
- The module that won the proposal is given one last chance to check the 
incoming request and fail the upgrade with an immediate HTTP response.
- Otherwise, core sends the 101.
- The switch handler takes over the connection.
- The connection is closed after the switch handler returns.

I am still not sure where the authnz calls fit in, though. h2c and 
tls+http/1.1 can send a 401 after the protocol switch if they want 
(right?), but WebSocket needs to send it before the protocol switch, and 
having to replicate the authnz checks in my switch handler would be a 
pain...

> 4. status code of protocol switch handler: if we move the task of 101 sending, the switch handler might not do it and keep the connection on the "old" protocol. Then a connection close is not necessary. So, we would do the close only when the switch handler returns APR_EOF.

That's an interesting idea. Seems like it would work.

--Jacob

Re: Upgrade Summary

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 8, 2015 at 12:18 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Dec 8, 2015 at 11:54 AM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>>
>> Am 08.12.2015 um 11:44 schrieb Yann Ylavic <yl...@gmail.com>:
>>>
>>> On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
>>>>
>>>> PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output filter, return and have it being processed normally. The output filter would then send the 101 and do the TLS handshake. Would that work?
>>>
>>> The issue is that this output filter would have to both read and write
>>> during the TLS handshake, not sure this is suitable.
>>
>> Ok, maybe from pre_read_request hook then?
>
> Hmm, pre_read of the second request, interesting.

Actually no, the previous response has to be Upgraded, so for the TLS
case we'd have needed the handshake already.
Also, pre_read has no server_rec selected, so on which configuration
would we base the upgrade on...

> But maybe output filter could work too after all, we'd need to set the
> correct CONN_SENSE for event to do right thing?

Looks like the most suitable option if that can work.

Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 08.12.2015 um 12:18 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Tue, Dec 8, 2015 at 11:54 AM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>> 
>> Am 08.12.2015 um 11:44 schrieb Yann Ylavic <yl...@gmail.com>:
>>> 
>>> On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
>>>> 
>>>> PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output filter, return and have it being processed normally. The output filter would then send the 101 and do the TLS handshake. Would that work?
>>> 
>>> The issue is that this output filter would have to both read and write
>>> during the TLS handshake, not sure this is suitable.
>> 
>> Ok, maybe from pre_read_request hook then?
> 
> Hmm, pre_read of the second request, interesting.
> But maybe output filter could work too after all, we'd need to set the
> correct CONN_SENSE for event to do right thing?
> pre_read may be simpler though, based on a flag in the conn_rec?

mod_ssl could have that flag in its SSLConnRec, i would assume...

Re: Upgrade Summary

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 8, 2015 at 11:54 AM, Stefan Eissing
<st...@greenbytes.de> wrote:
>
> Am 08.12.2015 um 11:44 schrieb Yann Ylavic <yl...@gmail.com>:
>>
>> On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
>>>
>>> PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output filter, return and have it being processed normally. The output filter would then send the 101 and do the TLS handshake. Would that work?
>>
>> The issue is that this output filter would have to both read and write
>> during the TLS handshake, not sure this is suitable.
>
> Ok, maybe from pre_read_request hook then?

Hmm, pre_read of the second request, interesting.
But maybe output filter could work too after all, we'd need to set the
correct CONN_SENSE for event to do right thing?
pre_read may be simpler though, based on a flag in the conn_rec?

Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 08.12.2015 um 14:08 schrieb Bert Huijben <be...@qqmail.nl>:
> [...]
> (I'm pretty sure we find new interesting proxy server scenarios :-( )

Not only that. Someone tested h2c against his server from various Tor exit nodes and got a failure rate > 10% because of network middle boxes...

Fun times.




RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> Sent: dinsdag 8 december 2015 13:54
> To: dev@httpd.apache.org
> Subject: Re: Upgrade Summary
> 
> I see. Delta-V goodies.
> 
> My proposal therefore is:
> - keep the upgrade/protocol switch mechanism as is for the 2.4.18 release
> - make the agreed upon changes, including TLS upgrade and small content
> h2c upgrades for the next release
> 
> Since the mechanism is already out with 2.4.17, I see no sense in delaying
> 2.4.18 at this point and rather give us time to change it later, so it
works for
> everyone.

Sure... my upgrade requests for Subversion are for 2.4.x future... Certainly
no rush here.

I don't expect this upgrade support to reach Subversion far before 1.10. 

Perhaps we will backport some of this to 1.9, but I don't think we can just
enable H2 support in a minor release without testing on a bigger group
first.
(I'm pretty sure we find new interesting proxy server scenarios :-( )

	Bert


Re: Upgrade Summary

Posted by Yann Ylavic <yl...@gmail.com>.
If so, for 2.4.18 we should probably backport Bill's proposal in
STATUS (r1717816), so that OPTIONS works as in 2.4.16...

On Tue, Dec 8, 2015 at 1:54 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
> I see. Delta-V goodies.
>
> My proposal therefore is:
> - keep the upgrade/protocol switch mechanism as is for the 2.4.18 release
> - make the agreed upon changes, including TLS upgrade and small content h2c upgrades for the next release
>
> Since the mechanism is already out with 2.4.17, I see no sense in delaying 2.4.18 at this point and rather give us time to change it later, so it works for everyone.
>
> //Stefan
>
>> Am 08.12.2015 um 13:46 schrieb Bert Huijben <be...@qqmail.nl>:
>>
>>
>>
>>> -----Original Message-----
>>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>>> Sent: dinsdag 8 december 2015 13:22
>>> To: dev@httpd.apache.org
>>> Subject: Re: Upgrade Summary
>>>
>>>
>>>> Am 08.12.2015 um 12:40 schrieb Bert Huijben <be...@qqmail.nl>:
>>>>> -----Original Message-----
>>>>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>>>>> Sent: dinsdag 8 december 2015 11:55
>>>>> To: dev@httpd.apache.org
>>>>> Subject: Re: Upgrade Summary
>>>>>>> [...]3. When to do the upgrade dance:
>>>>>>> a post_read_request: upgrade precedes authentication
>>>>>>
>>>>>> Looks like it can't be RFC compliant, is it?
>>>>>
>>>>> I think it will not be, right.
>>>>
>>>> I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
>>>> authentication *after* switching should work just like when not
>> switching.
>>>
>>> It does. We are only talking about upgrade.
>>>
>>>> As client I would like to switch as soon as possible, as at that point I
>> can
>>>> start multiple requests.. potentially with their own auth, using the
>> same or
>>>> different realms; or even different auth schemes.
>>>
>>> Again, your easiest choice is a direct h2c connection. The second easiest
>> is
>>> an initial OPTIONS * request without *request* body. The response may
>>> have
>>> any body length it wants.
>>>
>>> The options * will, as I understand it, not trigger any authentication. In
>>> another mail, you describe that you already send such a request as 1st
>> thing.
>>> But you said it has a body. What does that contain, I wonder?
>>>
>>> If you can live without that request body, we are all fine and have a
>> simple
>>> implementation. If we need to implement upgrades to h2c on some length
>>> request bodies, I personally do not have the time to do that right away
>>> among
>>> all other changes that are being discussed. At least not this week.
>>>
>>> So, what is so relevant about the OPTIONS request body, may I ask?
>>
>> I can't live without that request body. That would just add another request
>> to my handshake... the thing I'm trying to avoid.
>>
>> I'm guessing the body of that initial body is something like
>> [[
>> <D:options xmlns:D="DAV:">
>>   <D:activity-collection-set />
>> </D:options>
>> ]]
>> (content-type text/xml of course)
>>
>> And this request is mostly handled by mod_dav, and further annotated with
>> additional headers by mod_dav_svn.
>>
>> I can't just redesign DAV to optimize for http/2. If we had a timemachine,
>> we would have made other decisions.
>>
>> Would have been nice if we knew DELTA/V was never really implemented outside
>> Subversion. But now we have compatibility guarantees with older Subversion
>> clients/servers and other implementations that may use some of the features.
>>
>>       Bert
>

Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
I see. Delta-V goodies.

My proposal therefore is:
- keep the upgrade/protocol switch mechanism as is for the 2.4.18 release
- make the agreed upon changes, including TLS upgrade and small content h2c upgrades for the next release

Since the mechanism is already out with 2.4.17, I see no sense in delaying 2.4.18 at this point and rather give us time to change it later, so it works for everyone.

//Stefan

> Am 08.12.2015 um 13:46 schrieb Bert Huijben <be...@qqmail.nl>:
> 
> 
> 
>> -----Original Message-----
>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>> Sent: dinsdag 8 december 2015 13:22
>> To: dev@httpd.apache.org
>> Subject: Re: Upgrade Summary
>> 
>> 
>>> Am 08.12.2015 um 12:40 schrieb Bert Huijben <be...@qqmail.nl>:
>>>> -----Original Message-----
>>>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>>>> Sent: dinsdag 8 december 2015 11:55
>>>> To: dev@httpd.apache.org
>>>> Subject: Re: Upgrade Summary
>>>>>> [...]3. When to do the upgrade dance:
>>>>>> a post_read_request: upgrade precedes authentication
>>>>> 
>>>>> Looks like it can't be RFC compliant, is it?
>>>> 
>>>> I think it will not be, right.
>>> 
>>> I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
>>> authentication *after* switching should work just like when not
> switching.
>> 
>> It does. We are only talking about upgrade.
>> 
>>> As client I would like to switch as soon as possible, as at that point I
> can
>>> start multiple requests.. potentially with their own auth, using the
> same or
>>> different realms; or even different auth schemes.
>> 
>> Again, your easiest choice is a direct h2c connection. The second easiest
> is
>> an initial OPTIONS * request without *request* body. The response may
>> have
>> any body length it wants.
>> 
>> The options * will, as I understand it, not trigger any authentication. In
>> another mail, you describe that you already send such a request as 1st
> thing.
>> But you said it has a body. What does that contain, I wonder?
>> 
>> If you can live without that request body, we are all fine and have a
> simple
>> implementation. If we need to implement upgrades to h2c on some length
>> request bodies, I personally do not have the time to do that right away
>> among
>> all other changes that are being discussed. At least not this week.
>> 
>> So, what is so relevant about the OPTIONS request body, may I ask?
> 
> I can't live without that request body. That would just add another request
> to my handshake... the thing I'm trying to avoid.
> 
> I'm guessing the body of that initial body is something like
> [[
> <D:options xmlns:D="DAV:">
>   <D:activity-collection-set />
> </D:options>
> ]]
> (content-type text/xml of course)
> 
> And this request is mostly handled by mod_dav, and further annotated with
> additional headers by mod_dav_svn.
> 
> I can't just redesign DAV to optimize for http/2. If we had a timemachine,
> we would have made other decisions.
> 
> Would have been nice if we knew DELTA/V was never really implemented outside
> Subversion. But now we have compatibility guarantees with older Subversion
> clients/servers and other implementations that may use some of the features.
> 
> 	Bert


RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> Sent: dinsdag 8 december 2015 13:22
> To: dev@httpd.apache.org
> Subject: Re: Upgrade Summary
> 
> 
> > Am 08.12.2015 um 12:40 schrieb Bert Huijben <be...@qqmail.nl>:
> >> -----Original Message-----
> >> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> >> Sent: dinsdag 8 december 2015 11:55
> >> To: dev@httpd.apache.org
> >> Subject: Re: Upgrade Summary
> >>>> [...]3. When to do the upgrade dance:
> >>>> a post_read_request: upgrade precedes authentication
> >>>
> >>> Looks like it can't be RFC compliant, is it?
> >>
> >> I think it will not be, right.
> >
> > I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
> > authentication *after* switching should work just like when not
switching.
> 
> It does. We are only talking about upgrade.
> 
> > As client I would like to switch as soon as possible, as at that point I
can
> > start multiple requests.. potentially with their own auth, using the
same or
> > different realms; or even different auth schemes.
> 
> Again, your easiest choice is a direct h2c connection. The second easiest
is
> an initial OPTIONS * request without *request* body. The response may
> have
> any body length it wants.
> 
> The options * will, as I understand it, not trigger any authentication. In
> another mail, you describe that you already send such a request as 1st
thing.
> But you said it has a body. What does that contain, I wonder?
> 
> If you can live without that request body, we are all fine and have a
simple
> implementation. If we need to implement upgrades to h2c on some length
> request bodies, I personally do not have the time to do that right away
> among
> all other changes that are being discussed. At least not this week.
> 
> So, what is so relevant about the OPTIONS request body, may I ask?

I can't live without that request body. That would just add another request
to my handshake... the thing I'm trying to avoid.

I'm guessing the body of that initial body is something like
[[
<D:options xmlns:D="DAV:">
   <D:activity-collection-set />
</D:options>
]]
(content-type text/xml of course)

And this request is mostly handled by mod_dav, and further annotated with
additional headers by mod_dav_svn.

I can't just redesign DAV to optimize for http/2. If we had a timemachine,
we would have made other decisions.

Would have been nice if we knew DELTA/V was never really implemented outside
Subversion. But now we have compatibility guarantees with older Subversion
clients/servers and other implementations that may use some of the features.

	Bert


Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 08.12.2015 um 12:40 schrieb Bert Huijben <be...@qqmail.nl>:
>> -----Original Message-----
>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>> Sent: dinsdag 8 december 2015 11:55
>> To: dev@httpd.apache.org
>> Subject: Re: Upgrade Summary
>>>> [...]3. When to do the upgrade dance:
>>>> a post_read_request: upgrade precedes authentication
>>> 
>>> Looks like it can't be RFC compliant, is it?
>> 
>> I think it will not be, right.
> 
> I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
> authentication *after* switching should work just like when not switching.

It does. We are only talking about upgrade.

> As client I would like to switch as soon as possible, as at that point I can
> start multiple requests.. potentially with their own auth, using the same or
> different realms; or even different auth schemes.

Again, your easiest choice is a direct h2c connection. The second easiest is 
an initial OPTIONS * request without *request* body. The response may have 
any body length it wants. 

The options * will, as I understand it, not trigger any authentication. In
another mail, you describe that you already send such a request as 1st thing.
But you said it has a body. What does that contain, I wonder?

If you can live without that request body, we are all fine and have a simple
implementation. If we need to implement upgrades to h2c on some length
request bodies, I personally do not have the time to do that right away among
all other changes that are being discussed. At least not this week.

So, what is so relevant about the OPTIONS request body, may I ask?

//Stefan

RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: dinsdag 8 december 2015 12:41
> To: dev@httpd.apache.org
> Subject: RE: Upgrade Summary
> 


> I don't think we should require all auth to happen on HTTP/1.1.

If you want to go for equivalent implementations for things like the TLS
upgrade... then you really don't want to have AUTH completed before upgrade.

My preferred implementation would issue the 101 on HTTP/1.1 and then produce
the 401 on H2.
(I would be happy with an EOS on the response headers... I'm not interested
in the body :-))

I can handle '100 continue' before the 101 on HTTP/1.1, but also after
switching to H2 and before the actual result on H2... But I'm happy that we
don't have to support all possible scenarios there.

	Bert


RE: Upgrade Summary

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> Sent: dinsdag 8 december 2015 11:55
> To: dev@httpd.apache.org
> Subject: Re: Upgrade Summary
> 
> 
> > Am 08.12.2015 um 11:44 schrieb Yann Ylavic <yl...@gmail.com>:
> >
> > On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
> > <st...@greenbytes.de> wrote:
> >> [...]
> >> Open:
> >> 1. Protocols like Websocket need to take over the 101 sending
> themselves in the "switch protocol" phase. (correct, Jacob?). Should we
> delegate the sending of the 101 to the protocol switch handler?
> >> 2. General handling of request bodies. Options:
> >>  a setaside in core of up to nnn bytes before switch invocation
> >>  b do nothing, let protocol switch handler care about it
> >
> > a. is tempting, where nnn would be the limit above which we don't honor
> Upgrade.
> > But that could be a default behaviour, i.e. when "Protocols ... http1"
> > is finally elected.
> > Possibly specific modules would bypass that?
> >
> >> 3. When to do the upgrade dance:
> >>  a post_read_request: upgrade precedes authentication
> >
> > Looks like it can't be RFC compliant, is it?
> 
> I think it will not be, right.

I read the spec as H2c and HTTP/1.1 are equivalent protocols. Handling
authentication *after* switching should work just like when not switching.

As client I would like to switch as soon as possible, as at that point I can
start multiple requests.. potentially with their own auth, using the same or
different realms; or even different auth schemes.


I don't think we should require all auth to happen on HTTP/1.1.

With equivalent protocols we should switch as soon as possible... I don't
think servers should be configured as H2c only for authenticated users.
Especially if they also allow H2direct.

	Bert



Re: Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 08.12.2015 um 11:44 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
> <st...@greenbytes.de> wrote:
>> [...]
>> Open:
>> 1. Protocols like Websocket need to take over the 101 sending themselves in the "switch protocol" phase. (correct, Jacob?). Should we delegate the sending of the 101 to the protocol switch handler?
>> 2. General handling of request bodies. Options:
>>  a setaside in core of up to nnn bytes before switch invocation
>>  b do nothing, let protocol switch handler care about it
> 
> a. is tempting, where nnn would be the limit above which we don't honor Upgrade.
> But that could be a default behaviour, i.e. when "Protocols ... http1"
> is finally elected.
> Possibly specific modules would bypass that?
> 
>> 3. When to do the upgrade dance:
>>  a post_read_request: upgrade precedes authentication
> 
> Looks like it can't be RFC compliant, is it?

I think it will not be, right.

>>  b handler: upgrade only honored on authenticated and otherwise ok requests
> 
> This is probably the right thing to do, would OPTIONS be special is this case?

It currently is insofar as core installs a storage handler to invoke the, otherwise
same, upgrade handling code. It's a special case, but nothing the protocol handlers
need to care about...

>>  c both: introduce separate hooks? have an additional parameter? more complexity
> 
> Depends on whether OPTIONS (others?) need special treatment.

The current design works with it while exposing in the API only one set of hooks. If
those get called from one or the other does not matter. For each request, the hooks
are invoked at most once.

>> 4. status code of protocol switch handler: if we move the task of 101 sending, the switch handler might not do it and keep the connection on the "old" protocol. Then a connection close is not necessary. So, we would do the close only when the switch handler returns APR_EOF.
>> 5. Will it be possible to migrate the current TLS upgrade handling to this revised scheme?
> 
> We must do that IMHO, the scheme should be suite for all the cases.

Agreed.

>> PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output filter, return and have it being processed normally. The output filter would then send the 101 and do the TLS handshake. Would that work?
> 
> The issue is that this output filter would have to both read and write
> during the TLS handshake, not sure this is suitable.

Ok, maybe from pre_read_request hook then?


Re: Upgrade Summary

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 8, 2015 at 11:07 AM, Stefan Eissing
<st...@greenbytes.de> wrote:
> Trying to summarize the status of the discussion and where the issues are with the current Upgrade implementation.
>
> Clarified:
> A. any 100 must be sent out *before* a 101 response
> B. request bodies are to be read in the original protocol, input filters like chunk can be used, indeed are necessary, as if the request is being processed normally
> C. if a protocol supports upgrade on request bodies is up to the protocol implementation and needs to be checked in the "propose" phase

Agreed.

>
> Open:
> 1. Protocols like Websocket need to take over the 101 sending themselves in the "switch protocol" phase. (correct, Jacob?). Should we delegate the sending of the 101 to the protocol switch handler?
> 2. General handling of request bodies. Options:
>   a setaside in core of up to nnn bytes before switch invocation
>   b do nothing, let protocol switch handler care about it

a. is tempting, where nnn would be the limit above which we don't honor Upgrade.
But that could be a default behaviour, i.e. when "Protocols ... http1"
is finally elected.
Possibly specific modules would bypass that?

> 3. When to do the upgrade dance:
>   a post_read_request: upgrade precedes authentication

Looks like it can't be RFC compliant, is it?

>   b handler: upgrade only honored on authenticated and otherwise ok requests

This is probably the right thing to do, would OPTIONS be special is this case?

>   c both: introduce separate hooks? have an additional parameter? more complexity

Depends on whether OPTIONS (others?) need special treatment.

> 4. status code of protocol switch handler: if we move the task of 101 sending, the switch handler might not do it and keep the connection on the "old" protocol. Then a connection close is not necessary. So, we would do the close only when the switch handler returns APR_EOF.
> 5. Will it be possible to migrate the current TLS upgrade handling to this revised scheme?

We must do that IMHO, the scheme should be suite for all the cases.

>
> PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output filter, return and have it being processed normally. The output filter would then send the 101 and do the TLS handshake. Would that work?

The issue is that this output filter would have to both read and write
during the TLS handshake, not sure this is suitable.

Upgrade Summary

Posted by Stefan Eissing <st...@greenbytes.de>.
Trying to summarize the status of the discussion and where the issues are with the current Upgrade implementation.

Clarified:
A. any 100 must be sent out *before* a 101 response
B. request bodies are to be read in the original protocol, input filters like chunk can be used, indeed are necessary, as if the request is being processed normally
C. if a protocol supports upgrade on request bodies is up to the protocol implementation and needs to be checked in the "propose" phase

Open:
1. Protocols like Websocket need to take over the 101 sending themselves in the "switch protocol" phase. (correct, Jacob?). Should we delegate the sending of the 101 to the protocol switch handler?
2. General handling of request bodies. Options:
  a setaside in core of up to nnn bytes before switch invocation
  b do nothing, let protocol switch handler care about it
3. When to do the upgrade dance:
  a post_read_request: upgrade precedes authentication
  b handler: upgrade only honored on authenticated and otherwise ok requests
  c both: introduce separate hooks? have an additional parameter? more complexity
4. status code of protocol switch handler: if we move the task of 101 sending, the switch handler might not do it and keep the connection on the "old" protocol. Then a connection close is not necessary. So, we would do the close only when the switch handler returns APR_EOF.
5. Will it be possible to migrate the current TLS upgrade handling to this revised scheme?

Anything I missed?

PS. Re 5: with change 1+4, a TLS upgrade switcher could install an output filter, return and have it being processed normally. The output filter would then send the 101 and do the TLS handshake. Would that work?

> Am 08.12.2015 um 10:42 schrieb Stefan Eissing <st...@greenbytes.de>:
> 
> 
>> Am 07.12.2015 um 23:42 schrieb Jacob Champion <ch...@gmail.com>:
>> 
>> On 12/07/2015 02:30 PM, Bert Huijben wrote:
>>> Is this a h2 limitation or a mod_h2 limitation?
>>> 
>>> If I would like h2c upgrade from Subversion without an additional
>>> request I would have to send the upgrade request with a very short
>>> OPTIONS request that has a body.
>> 
>> The HTTP/2 spec (RFC 7540, sec 3.2) appears to allow the sending of a request body in an h2c upgrade (h2 upgrades are prohibited). So it would appear to be a mod_http2 limitation for now.
>> 
>> Hopefully Stefan can confirm tomorrow (it's pretty late in Germany).
> 
> It is possible, but difficult to do unless you can read the body and set it aside somewhere. Which again imposes an arbitrary (for the client) limit. Experience shows that this leads to design of clients that work will in developments, but then encounter requests usage/sites/servers that have other limits and break.
> 
> If Apache accepts body lengths of up to 64KB (or something configurable) and some other server implements 8K and some third 1MB, how will that help design of a client that, for some reason, *wants* an upgrade to succeed? 
> 
> //Stefan
> 


Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

Posted by Jacob Champion <ch...@gmail.com>.
On 12/08/2015 01:42 AM, Stefan Eissing wrote:
> If Apache accepts body lengths of up to 64KB (or something
> configurable) and some other server implements 8K and some third 1MB,
> how will that help design of a client that, for some reason, *wants*
> an upgrade to succeed?

I don't disagree with your premise, and I think that the zero-length 
body requirement is the correct thing to do for now. I will theorycraft 
in another email to the list.

--Jacob

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 8, 2015 at 11:36 AM, Stefan Eissing
<st...@greenbytes.de> wrote:
>
> I do not understand. How do you want to make pipelining requests and protocol upgrades work together? I assume you talk about http pipelining where you send a second request before you receive the response for the first one...

I suppose pipelining would start from the second request...

RE: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> Sent: dinsdag 8 december 2015 11:36
> To: dev@httpd.apache.org
> Subject: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl
> Upgradetls
> 
> Bert,
> 
> I do not understand. How do you want to make pipelining requests and
> protocol upgrades work together? I assume you talk about http pipelining
> where you send a second request before you receive the response for the
> first one...

I can't... but you were explaining that I could just switch to the new
protocol after my request... which I can't.

I have to perform a full stop... which is a performance killer in a client
that is optimized for pipelining. (Serf and libsvn_ra_serf are 100%
optimized for pipelining)

That is why I want to upgrade on the first request.


At the start of any session we currently perform 2 requests with full stop
behavior, to support user scnearios:
* one to detect if we have HTTP/1.1 or HTTP/1.0.

This is an OPTIONS request with a small Content-Length body. This will
already hand over the capabilities of the Subversion server and some
interesting URLS. If we find an old server here, the full handshake will
have more requests.

* A second request with a small chunked body to see if the server supports
chunked encoding (assuming we found 1.1).
If users use an nginx frontend (which is not uncommon) this fails and we
explicitly cache all requests to have explicit Content-Length headers in
this case.

After this we have many different scenarios, of which many want to pipeline,
or open multiple connections as soon as possible.

For Subversion I would like to switch to H2c in one of these two requests.
The first one looks like the simplest one. (Combining upgrading with chunked
makes things even harder).



I can't store whether a server supports H2 and just use it. Subversion is
used on laptops that are used on multiple networks.


Receiving Alt-Svc in the first response and just pipelining the upgrade
between further requests if there is a match, is the only other way I could
upgrade without a performance penalty on all servers that don't support H2c.

	Bert


Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

Posted by Stefan Eissing <st...@greenbytes.de>.
Bert,

I do not understand. How do you want to make pipelining requests and protocol upgrades work together? I assume you talk about http pipelining where you send a second request before you receive the response for the first one...

//Stefan

> Am 08.12.2015 um 11:27 schrieb Bert Huijben <be...@qqmail.nl>:
> 
> 
> 
>> -----Original Message-----
>> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
>> Sent: dinsdag 8 december 2015 10:43
>> To: dev@httpd.apache.org
>> Subject: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl
>> Upgradetls
> 
> 
>> If Apache accepts body lengths of up to 64KB (or something configurable)
>> and some other server implements 8K and some third 1MB, how will that
>> help design of a client that, for some reason, *wants* an upgrade to
>> succeed?
> 
> For Subversion this would make a 100% difference.
> 
> Currently all true Subversion servers are implemented via httpd.
> 
> 
> Upgrade to h2c is currently not interesting for webbrowsers as all of them
> have decided not to implement it... But it makes a huge difference for web
> applications that talk to specific servers.
> 
> Which is very similar to that TLS upgrade that is interested for
> webprinting... These usecases don't use combinations of random clients, with
> random servers... They use specific combinations.
> 
> 	Bert
> 


RE: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> Sent: dinsdag 8 december 2015 10:43
> To: dev@httpd.apache.org
> Subject: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl
> Upgradetls


> If Apache accepts body lengths of up to 64KB (or something configurable)
> and some other server implements 8K and some third 1MB, how will that
> help design of a client that, for some reason, *wants* an upgrade to
> succeed?

For Subversion this would make a 100% difference.

Currently all true Subversion servers are implemented via httpd.


Upgrade to h2c is currently not interesting for webbrowsers as all of them
have decided not to implement it... But it makes a huge difference for web
applications that talk to specific servers.

Which is very similar to that TLS upgrade that is interested for
webprinting... These usecases don't use combinations of random clients, with
random servers... They use specific combinations.

	Bert


Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 07.12.2015 um 23:42 schrieb Jacob Champion <ch...@gmail.com>:
> 
> On 12/07/2015 02:30 PM, Bert Huijben wrote:
>> Is this a h2 limitation or a mod_h2 limitation?
>> 
>> If I would like h2c upgrade from Subversion without an additional
>> request I would have to send the upgrade request with a very short
>> OPTIONS request that has a body.
> 
> The HTTP/2 spec (RFC 7540, sec 3.2) appears to allow the sending of a request body in an h2c upgrade (h2 upgrades are prohibited). So it would appear to be a mod_http2 limitation for now.
> 
> Hopefully Stefan can confirm tomorrow (it's pretty late in Germany).

It is possible, but difficult to do unless you can read the body and set it aside somewhere. Which again imposes an arbitrary (for the client) limit. Experience shows that this leads to design of clients that work will in developments, but then encounter requests usage/sites/servers that have other limits and break.

If Apache accepts body lengths of up to 64KB (or something configurable) and some other server implements 8K and some third 1MB, how will that help design of a client that, for some reason, *wants* an upgrade to succeed? 

//Stefan


Re: AW: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

Posted by Jacob Champion <ch...@gmail.com>.
On 12/07/2015 02:30 PM, Bert Huijben wrote:
> Is this a h2 limitation or a mod_h2 limitation?
>
> If I would like h2c upgrade from Subversion without an additional
> request I would have to send the upgrade request with a very short
> OPTIONS request that has a body.

The HTTP/2 spec (RFC 7540, sec 3.2) appears to allow the sending of a 
request body in an h2c upgrade (h2 upgrades are prohibited). So it would 
appear to be a mod_http2 limitation for now.

Hopefully Stefan can confirm tomorrow (it's pretty late in Germany).

--Jacob

RE: AW: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls

Posted by Bert Huijben <be...@qqmail.nl>.
Is this a h2 limitation or a mod_h2 limitation?

If I would like h2c upgrade from Subversion without an additional request I would have to send the upgrade request with a very short OPTIONS request that has a body.

The way I read the spec that should be possible if both sides go through all those rough edge cases to support a request in one protocol and the response in the other. For serf I intended to implement that, but I’m not sure if it is worth the trouble if httpd doesn’t implement it.

Bert

Sent from Mail for Windows 10



From: Stefan Eissing
Sent: maandag 7 december 2015 20:11
To: dev@httpd.apache.org
Subject: Re: AW: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgradetls


h2 does not propose a switch if the request has a body. 

As clarified on the http wg lists by the gurus there,  when i asked, the upgraded connection is in a mixed state after a 101. Any byte sent by the server MUST be from the switched protocol, while the client needs to send the body in HTTP/1 format and can only talk the new proto afterwards. 

For an Expect 100 that would mean that the 100 intermediate comes before the 101. However that is untested with h2 as we do not propose a switch when a body is there.

Am 07.12.2015 um 19:50 schrieb Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>:
 
 
Von: William A Rowe Jr [mailto:wrowe@rowe-clan.net] 
Gesendet: Montag, 7. Dezember 2015 17:39
An: httpd <de...@httpd.apache.org>
Betreff: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls
 
https://tools.ietf.org/html/rfc7230#section-6.7 makes things more interesting, it calls out that 101-continue and the request body read precedes the 101-switching protocols.  Not sure who decided that would be a good idea, sigh... but mod_ssl TLS upgrade has these reversed for several good reasons including the intent to encrypt the request body if present and simple economics of processing.
I think that handling upgrade advertisement and alerting must be in post read req, bypassing all request hooks until the 101-continue is presented, any small request body read and set aside for the http input brigade, and 101-switching protocols is presented.  This allows the request to still be processed for tls-style upgrades, or discarded for relevant protocols.
How do we handle this today if the client just sends a request body and not an Expect header? Do we set it already aside before answering with a 101?
Regards
Rüdiger



Re: AW: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Stefan Eissing <st...@greenbytes.de>.
h2 does not propose a switch if the request has a body. 

As clarified on the http wg lists by the gurus there,  when i asked, the upgraded connection is in a mixed state after a 101. Any byte sent by the server MUST be from the switched protocol, while the client needs to send the body in HTTP/1 format and can only talk the new proto afterwards. 

For an Expect 100 that would mean that the 100 intermediate comes before the 101. However that is untested with h2 as we do not propose a switch when a body is there.

> Am 07.12.2015 um 19:50 schrieb Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>:
> 
>  
>  
> Von: William A Rowe Jr [mailto:wrowe@rowe-clan.net] 
> Gesendet: Montag, 7. Dezember 2015 17:39
> An: httpd <de...@httpd.apache.org>
> Betreff: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls
>  
> https://tools.ietf.org/html/rfc7230#section-6.7 makes things more interesting, it calls out that 101-continue and the request body read precedes the 101-switching protocols.  Not sure who decided that would be a good idea, sigh... but mod_ssl TLS upgrade has these reversed for several good reasons including the intent to encrypt the request body if present and simple economics of processing.
> 
> I think that handling upgrade advertisement and alerting must be in post read req, bypassing all request hooks until the 101-continue is presented, any small request body read and set aside for the http input brigade, and 101-switching protocols is presented.  This allows the request to still be processed for tls-style upgrades, or discarded for relevant protocols.
> 
> How do we handle this today if the client just sends a request body and not an Expect header? Do we set it already aside before answering with a 101?
> 
> Regards
> 
> Rüdiger

AW: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

Von: William A Rowe Jr [mailto:wrowe@rowe-clan.net]
Gesendet: Montag, 7. Dezember 2015 17:39
An: httpd <de...@httpd.apache.org>
Betreff: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls


https://tools.ietf.org/html/rfc7230#section-6.7 makes things more interesting, it calls out that 101-continue and the request body read precedes the 101-switching protocols.  Not sure who decided that would be a good idea, sigh... but mod_ssl TLS upgrade has these reversed for several good reasons including the intent to encrypt the request body if present and simple economics of processing.

I think that handling upgrade advertisement and alerting must be in post read req, bypassing all request hooks until the 101-continue is presented, any small request body read and set aside for the http input brigade, and 101-switching protocols is presented.  This allows the request to still be processed for tls-style upgrades, or discarded for relevant protocols.

How do we handle this today if the client just sends a request body and not an Expect header? Do we set it already aside before answering with a 101?

Regards

Rüdiger

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 8, 2015 at 1:13 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Mon, Dec 7, 2015 at 6:07 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> The 100-continue case isn't a particular one IMHO, any Upgrade header
>> would be informative, not authoritative (not a 101 status).
>
> Not sure what you mean.  If an Upgrade: request is recieved and declined,
> you are right, there is no 101-switching protocols, just the typical
> http/1.1
> response following any 100-continue negotiated request body.

I mean if httpd honors a 100-continue, it has to be done before the
101 Switching Protocols, either in the http_filter (like currently),
or if the request is proxyed and we implement end-to-end 100-continue
(I'm working on this), by forwarding the backend's 100-continue.
Then we can upgrade with 101 for the real response.

It's not different than a normal request with a body and no
100-continue expection IMHO.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 6:07 PM, Yann Ylavic <yl...@gmail.com> wrote:

> On Mon, Dec 7, 2015 at 5:38 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> >
> > https://tools.ietf.org/html/rfc7230#section-6.7 makes things more
> > interesting, it calls out that 101-continue and the request body read
> > precedes the 101-switching protocols.  Not sure who decided that would
> be a
> > good idea, sigh... but mod_ssl TLS upgrade has these reversed for several
> > good reasons including the intent to encrypt the request body if present
> and
> > simple economics of processing.
>
> As I read rfc7230#section-6.7 :
>
>    A client cannot begin using an upgraded protocol on the connection
>    until it has completely sent the request message (i.e., the client
>    can't change the protocol it is sending in the middle of a message).
>
> the server is supposed to respond with a 101 and than the Upgrade'd
> response (using the Upgrade protocol, eg. TLS/1.x), but *after* the
> request body is read (in plain text!).
> That means, for the TLS/1.x case, that the client handshake is
> supposed to arrive after a request body (if any), and hence mod_ssl
> can't assume the first request body is TLS'd, and the body ought to be
> set aside for any (relevant) TLS response (which needs the
> handshake...).
>
> I don't see this is working like this in mod_ssl, am I missing something ?
>

Yes you are missing the relevant design spec, RFC2817, now superseded
in part, in conjunction with RFC2616.

The behavior has to change, and my generalized proposal is in the message
immediately preceding this reply.


> Still rfc7230#section-6.7:
>
>    If a server receives both an Upgrade and an Expect header field with
>    the "100-continue" expectation (Section 5.1.1 of [RFC7231]), the
>    server MUST send a 100 (Continue) response before sending a 101
>    (Switching Protocols) response.
>
> The 100-continue case isn't a particular one IMHO, any Upgrade header
> would be informative, not authoritative (not a 101 status).
>

Not sure what you mean.  If an Upgrade: request is recieved and declined,
you are right, there is no 101-switching protocols, just the typical
http/1.1
response following any 100-continue negotiated request body.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 08.12.2015 um 10:48 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Tue, Dec 8, 2015 at 3:07 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> [...]
> That's why I'd rather go with *not* honoring the Upgrade until a
> request comes with no body.

++1

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 8, 2015 at 3:07 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>>
>> The handler handling the first request has to read the body before the
>> switch, eg. mod_proxy could forward the request without the client
>> having switched to any other protocol.
>
> Whoa... First the handler can't process the request until the switch, and
> more importantly the protocol upgrade is hop by hop, review s6.7 again
> please.

Hop by hop does not mean that Upgrade has to be done as soon as
possible, but that it must not be forwarded (this is client <=> httpd
stuff only, any the backend/origin wouldn't notice).

The handler has to process an HTTP/1 body (that is, plain text when
http: is used), because this is what the client will send (s6.7 states
that the first request is HTTP/1, including the body, IOW the client
can't change that between the header and the body).
So if we were to set aside the first request body on Upgrade, do the
switch, and then run the handler, the latter should not expect
anything else than HTTP/1 for the body (this is how the body is, be it
aside or not).

Moreover, if we choose to honor the Upgrade after the body, we must
produce a response which is Upgraded, that means for the TLS/1.x case
that we must first enable mod_ssl, read/write/do the TLS handshake so
that the response is really TLS/1.x upgraded.
We can indeed do that by reading/setaside the body, send the 101
switch to the client, do the handshake, play the setaside HTTP/1 body
through the input filter, and finally be HTTP/1+TLS on the (first)
response and further requests.
But I find this a bit complicated, not to talk about resources usage /
possible DOS...

>
> The mechanism to address your use case is HTTP/1.1 CONNECT.

My use case is a normal HTTP/1 request with a body, w/ or w/o
100-continue, through mod_proxy_http, not CONNECT.
And I don't want Upgrade to break or make this overly complicated.

>
>> That does not mean it has to (or prereqs a) setaside body, this could
>> be streamed (and is even desirable in the mod_proxy case).
>
> Not possible to read plaintext and write TLS, for example.  The body must be
> consumed first.

The HTTP/1 Upgrade described in the RFC is actually "read plaintext
and write TLS", AIUI.

>
>> So couldn't we handle the switch (101) in an output filter instead
>> (before the first bytes are sent)?
>
> No, for TLS, absolutely not.
>
> You presume the handler consumed the body, and you have no assurance of
> that.  Which means the output filter now needs to attend to the input filter
> state, and we are back where we started with the no 101 before the upgrade
> can be satisfied.

That's why I'd rather go with *not* honoring the Upgrade until a
request comes with no body.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Dec 7, 2015 19:29, "Yann Ylavic" <yl...@gmail.com> wrote:
>
> On Tue, Dec 8, 2015 at 1:58 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:
> > On Mon, Dec 7, 2015 at 6:35 PM, Yann Ylavic <yl...@gmail.com>
wrote:
> >>
> >> The body of the first request is never Upgraded, so why would we read
> >> it using the Upgraded protocol?
> >
> >
> > How do you mean?  The RFC states we must read the request body
> > (following a 100-continue) prior to a 101-switching protocols.
>
> Right.
>
> > AFTER
> > the protocol is switched, we are ready to invoke the handler, which
> > will read the request body (which I suggest we have set aside within
> > the http input filter) and it will write to the output filter stack,
but to
> > a different stack of protocol and network filters.
>
> The handler handling the first request has to read the body before the
> switch, eg. mod_proxy could forward the request without the client
> having switched to any other protocol.

Whoa... First the handler can't process the request until the switch, and
more importantly the protocol upgrade is hop by hop, review s6.7 again
please.

The mechanism to address your use case is HTTP/1.1 CONNECT.

In terms of h2/h2c/HTTP/https the connection between user agent and gateway
has no bearing on the connection to the back end in the reverse proxy
situation.

> That does not mean it has to (or prereqs a) setaside body, this could
> be streamed (and is even desirable in the mod_proxy case).

Not possible to read plaintext and write TLS, for example.  The body must
be consumed first.

> So couldn't we handle the switch (101) in an output filter instead
> (before the first bytes are sent)?

No, for TLS, absolutely not.

You presume the handler consumed the body, and you have no assurance of
that.  Which means the output filter now needs to attend to the input
filter state, and we are back where we started with the no 101 before the
upgrade can be satisfied.

If the upgrade fails (tls handshake failure or whatnot) we have no
intention of starting a stateful transaction that will be broken.  No
successful upgrade, then no handler invocation.

And if the handler wants to make an election based on the protocol, it
cannot do this before reading all the input.  Sub-optimal if not simply
broken.

And as I've called out already the entire pre-handler cycle needs to be
able to inspect the tls session for authn/authz.

> Or simply don't honor Upgrade on requests with bodies when it appears
> that we'd need to setaside (that would be the case too with
> client-to-backend 100-continue and some response available before all
> the request body is read, not to speak about the default_handler where
> the body is just discarded)?
> I think we should try hard to stream things in HTTP/1 too, not
> necessarily honor optionals.

What's the performance impact of a 16k or 64k set aside?  None, those
buckets would have been called up anyways without the protocol switch.  We
are streaming, some baseline buckets must always be filled in all cases.
The extent of that buffer can be administor controlled.

The call is out there from svn, and many other web services will eventually
rely on the same concept, this is a must have to avoid the very round trips
that h2c sought to avoid.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 8, 2015 at 1:58 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Mon, Dec 7, 2015 at 6:35 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> The body of the first request is never Upgraded, so why would we read
>> it using the Upgraded protocol?
>
>
> How do you mean?  The RFC states we must read the request body
> (following a 100-continue) prior to a 101-switching protocols.

Right.

> AFTER
> the protocol is switched, we are ready to invoke the handler, which
> will read the request body (which I suggest we have set aside within
> the http input filter) and it will write to the output filter stack, but to
> a different stack of protocol and network filters.

The handler handling the first request has to read the body before the
switch, eg. mod_proxy could forward the request without the client
having switched to any other protocol.
That does not mean it has to (or prereqs a) setaside body, this could
be streamed (and is even desirable in the mod_proxy case).
So couldn't we handle the switch (101) in an output filter instead
(before the first bytes are sent)?
Or simply don't honor Upgrade on requests with bodies when it appears
that we'd need to setaside (that would be the case too with
client-to-backend 100-continue and some response available before all
the request body is read, not to speak about the default_handler where
the body is just discarded)?
I think we should try hard to stream things in HTTP/1 too, not
necessarily honor optionals.

RE: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Stefan Eissing [mailto:stefan.eissing@greenbytes.de]
> Sent: dinsdag 8 december 2015 10:25
> To: dev@httpd.apache.org
> Subject: Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl
> Upgrade tls
> 
> 
> > Am 08.12.2015 um 01:58 schrieb William A Rowe Jr <wrowe@rowe-
> clan.net>:
> >
> > On Mon, Dec 7, 2015 at 6:35 PM, Yann Ylavic <yl...@gmail.com>
> wrote:
> > On Tue, Dec 8, 2015 at 1:27 AM, William A Rowe Jr <wrowe@rowe-
> clan.net> wrote:
> > > On Mon, Dec 7, 2015 at 6:15 PM, Yann Ylavic <yl...@gmail.com>
> wrote:
> > >>
> > >> On Tue, Dec 8, 2015 at 1:07 AM, Yann Ylavic <yl...@gmail.com>
> wrote:
> > >> >
> > >> > the body ought to be
> > >> > set aside for any (relevant) TLS response (which needs the
> > >> > handshake...).
> > >>
> > >> Hmm, no need to set aside, *unless* with must produce a response
> > >> before the entire body (and the handshake) is read.
> > >> But we'd better not Upgrade in this case...
> > >
> > >
> > > Yes, there is a set aside, because the handler will read from the
filter
> > > stack... the handler phase has not yet occurred, and other content
> > > input filters may be inserted to transform the request body.
> > >
> > > The upgrade switch would occur before the request content handler
> > > is invoked, in all cases (post_read_request, or later during fixups
> > > or the very beginning of invoke_handler).
> >
> > But this isn't what the RFC says, right?
> > The body of the first request is never Upgraded, so why would we read
> > it using the Upgraded protocol?
> >
> > How do you mean?  The RFC states we must read the request body
> > (following a 100-continue) prior to a 101-switching protocols.  AFTER
> > the protocol is switched, we are ready to invoke the handler, which
> > will read the request body (which I suggest we have set aside within
> > the http input filter) and it will write to the output filter stack, but
to
> > a different stack of protocol and network filters.
> 
> No, that is not what the RFC says. HTTP/1 switching protocols does not
need
> to happen simultaneously on upstream and downstream. Instead, the server
> switches directly after the 101 response, the client switches after the
last
> byte of the request.

The client can only switch once it has confirmation from the server, that it
is really going to perform the upgrade. If the server doesn't accept the
upgrade it must continue using 1.1.

So it has to stop producing data after that first request and first check if
there is a 101 response before it can pipeline the next request.
 
In pipelining clients such as serf this makes a huge difference.

At some points during update/checkout we may have tens of requests pipelined
on a single connection... but that process starts a bit after the initial
handshake. I'm trying to be in H2(c) at that point, to avoid opening the
additional connections at that point, which we currently have.


Once the client has the 101 it can switch protocol for both input and output
and start sending the next request. Waiting for the 101 is a full stall.

	Bert


Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 08.12.2015 um 01:58 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
> 
> On Mon, Dec 7, 2015 at 6:35 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Dec 8, 2015 at 1:27 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> > On Mon, Dec 7, 2015 at 6:15 PM, Yann Ylavic <yl...@gmail.com> wrote:
> >>
> >> On Tue, Dec 8, 2015 at 1:07 AM, Yann Ylavic <yl...@gmail.com> wrote:
> >> >
> >> > the body ought to be
> >> > set aside for any (relevant) TLS response (which needs the
> >> > handshake...).
> >>
> >> Hmm, no need to set aside, *unless* with must produce a response
> >> before the entire body (and the handshake) is read.
> >> But we'd better not Upgrade in this case...
> >
> >
> > Yes, there is a set aside, because the handler will read from the filter
> > stack... the handler phase has not yet occurred, and other content
> > input filters may be inserted to transform the request body.
> >
> > The upgrade switch would occur before the request content handler
> > is invoked, in all cases (post_read_request, or later during fixups
> > or the very beginning of invoke_handler).
> 
> But this isn't what the RFC says, right?
> The body of the first request is never Upgraded, so why would we read
> it using the Upgraded protocol?
> 
> How do you mean?  The RFC states we must read the request body
> (following a 100-continue) prior to a 101-switching protocols.  AFTER
> the protocol is switched, we are ready to invoke the handler, which
> will read the request body (which I suggest we have set aside within 
> the http input filter) and it will write to the output filter stack, but to 
> a different stack of protocol and network filters.

No, that is not what the RFC says. HTTP/1 switching protocols does not need
to happen simultaneously on upstream and downstream. Instead, the server
switches directly after the 101 response, the client switches after the last
byte of the request.

This is problematic for protocols like HTTP/2 who may require to receive
data in the new protocol format *before* they are able to read an arbitrarily
long request body. So, every HTTP/2 implementation will have a request
body maximum length that it can upgrade with. For interop reasons, me and
other implementors choose to have 0 as max length, since every clients
will run into that limit immediately and learn to cope with that.

Please remember that such a request will be answered correctly and that 
the client can upgrade to h2c on the next one, or anytime it likes. The
upgrade possibilities are announced by Apache on the first response, so
it should be easy to make a decision.

//Stefan



Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 6:35 PM, Yann Ylavic <yl...@gmail.com> wrote:

> On Tue, Dec 8, 2015 at 1:27 AM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> > On Mon, Dec 7, 2015 at 6:15 PM, Yann Ylavic <yl...@gmail.com>
> wrote:
> >>
> >> On Tue, Dec 8, 2015 at 1:07 AM, Yann Ylavic <yl...@gmail.com>
> wrote:
> >> >
> >> > the body ought to be
> >> > set aside for any (relevant) TLS response (which needs the
> >> > handshake...).
> >>
> >> Hmm, no need to set aside, *unless* with must produce a response
> >> before the entire body (and the handshake) is read.
> >> But we'd better not Upgrade in this case...
> >
> >
> > Yes, there is a set aside, because the handler will read from the filter
> > stack... the handler phase has not yet occurred, and other content
> > input filters may be inserted to transform the request body.
> >
> > The upgrade switch would occur before the request content handler
> > is invoked, in all cases (post_read_request, or later during fixups
> > or the very beginning of invoke_handler).
>
> But this isn't what the RFC says, right?
> The body of the first request is never Upgraded, so why would we read
> it using the Upgraded protocol?
>

How do you mean?  The RFC states we must read the request body
(following a 100-continue) prior to a 101-switching protocols.  AFTER
the protocol is switched, we are ready to invoke the handler, which
will read the request body (which I suggest we have set aside within
the http input filter) and it will write to the output filter stack, but to
a different stack of protocol and network filters.

Depending on the protocol, it may have to perform some magic to
move the (smaller) request body over to a new protocol if the http/1.1
filter will no longer be in the stack.

If the request body can't be set aside while still speaking http/1.1
(buffer too small, etc) then in that case we cannot honor the upgrade:
request or switch protocols at all.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 8, 2015 at 1:27 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Mon, Dec 7, 2015 at 6:15 PM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> On Tue, Dec 8, 2015 at 1:07 AM, Yann Ylavic <yl...@gmail.com> wrote:
>> >
>> > the body ought to be
>> > set aside for any (relevant) TLS response (which needs the
>> > handshake...).
>>
>> Hmm, no need to set aside, *unless* with must produce a response
>> before the entire body (and the handshake) is read.
>> But we'd better not Upgrade in this case...
>
>
> Yes, there is a set aside, because the handler will read from the filter
> stack... the handler phase has not yet occurred, and other content
> input filters may be inserted to transform the request body.
>
> The upgrade switch would occur before the request content handler
> is invoked, in all cases (post_read_request, or later during fixups
> or the very beginning of invoke_handler).

But this isn't what the RFC says, right?
The body of the first request is never Upgraded, so why would we read
it using the Upgraded protocol?

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 6:15 PM, Yann Ylavic <yl...@gmail.com> wrote:

> On Tue, Dec 8, 2015 at 1:07 AM, Yann Ylavic <yl...@gmail.com> wrote:
> >
> > the body ought to be
> > set aside for any (relevant) TLS response (which needs the
> > handshake...).
>
> Hmm, no need to set aside, *unless* with must produce a response
> before the entire body (and the handshake) is read.
> But we'd better not Upgrade in this case...
>

Yes, there is a set aside, because the handler will read from the filter
stack... the handler phase has not yet occurred, and other content
input filters may be inserted to transform the request body.

The upgrade switch would occur before the request content handler
is invoked, in all cases (post_read_request, or later during fixups
or the very beginning of invoke_handler).

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Dec 8, 2015 at 1:07 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> the body ought to be
> set aside for any (relevant) TLS response (which needs the
> handshake...).

Hmm, no need to set aside, *unless* with must produce a response
before the entire body (and the handshake) is read.
But we'd better not Upgrade in this case...

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Dec 7, 2015 at 5:38 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> https://tools.ietf.org/html/rfc7230#section-6.7 makes things more
> interesting, it calls out that 101-continue and the request body read
> precedes the 101-switching protocols.  Not sure who decided that would be a
> good idea, sigh... but mod_ssl TLS upgrade has these reversed for several
> good reasons including the intent to encrypt the request body if present and
> simple economics of processing.

As I read rfc7230#section-6.7 :

   A client cannot begin using an upgraded protocol on the connection
   until it has completely sent the request message (i.e., the client
   can't change the protocol it is sending in the middle of a message).

the server is supposed to respond with a 101 and than the Upgrade'd
response (using the Upgrade protocol, eg. TLS/1.x), but *after* the
request body is read (in plain text!).
That means, for the TLS/1.x case, that the client handshake is
supposed to arrive after a request body (if any), and hence mod_ssl
can't assume the first request body is TLS'd, and the body ought to be
set aside for any (relevant) TLS response (which needs the
handshake...).

I don't see this is working like this in mod_ssl, am I missing something ?


Still rfc7230#section-6.7:

   If a server receives both an Upgrade and an Expect header field with
   the "100-continue" expectation (Section 5.1.1 of [RFC7231]), the
   server MUST send a 100 (Continue) response before sending a 101
   (Switching Protocols) response.

The 100-continue case isn't a particular one IMHO, any Upgrade header
would be informative, not authoritative (not a 101 status).

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Stefan Eissing <st...@greenbytes.de>.
Wow, you have been busy in my sleep...

I will respond to individual messages later in my office. From what I got overall:

- I completely agree with Yann, request bodies, 100 continue or not, are sent using the original, pre-upgrade protocol, e.g. http/1.1
- until the body is read, 101 may be sent back, but afterwards the downstream is talking the new protocol, while the upstream can only start doing so after the last request byte
- this makes upgrades on requests with bodies awkward for both parties. the approach of setaside nnn bytes is bad for interop as there is no standard what nnn is supposed to be, leading to failed requests varying by implementation or even size specific configuration. For h2 therfore all server implementors on the HTTP WG list at that time agreed to body-less upgrades only. 
- In case a client wants to talk to a known server implementation and is concerned about efficiency, I'd recommend http2 direct mode. 24 initial bytes and you have http2.
- as Jacob pointed out, some protocols need deeper inspections/other responses before sending out 101. So, I think making that part of the protocol switch handler is better.

//Stefan

> Am 08.12.2015 um 03:50 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
> 
> Sorry it took so long to respond to this earlier post, some of this might have already been covered...
> 
>> On Mon, Dec 7, 2015 at 1:03 PM, Stefan Eissing <st...@greenbytes.de> wrote:
>>  
>> Think about CORS restrictions and other stuff.
>> - I think its the protocol handlers job to deal with any request body.
> 
> Strongly disagree, because 1. the protocol switch mechansim isn't the content handler of the originating request, the request body is a request-level entity, and 2. TLS gave you a perfect illustration of altering the connection stack to continue using the core protocol stack.
> 
> In the case of h2c, or even a mod_echo Upgrade: echo mechanism, the entire request/response is going to be handed off to a new protocol stack, I understand that.  But that is one of several use cases here, and that new protocol stack is going to consume the network pipe and close it, I trust?!?  And return the appropriate status that the connection is gone even if it leaves the old code to mop up, e.g. lingering_close.
> 
>> For h2 upgrade, for example, the switch is only offered for requests without body.
> 
> Well, h2c is advertised on the first request with or without a body, and then never again. Other modules must advertise, e.g. in conjunction with the 426 Upgrade Required.
> 
> In fact, how do you propose to handle an authentication case with h2c living in near the handler phase, when it will alert during the authnz phase that http1.1 has been disallowed/h2c is required for a particular resource, and that upgrade (which could have occurred in the post-read-request phase) has not yet occurred during auth?
> 
>> - moving things to post read sounds tempting, however I'm not sure if we want to upgrade on non-authed request or not, for example.
> 
> This is transport layer, yes, we always address the protocol without consideration as to authnz; the same as we decode gzip irrespective of client auth.  The actual requests over h2c (or http/1.1) are *each* still subject to auth per-request.
> 
> I can't see a justification for excluding a client other than the user agent identifier (known inoperable agent), otherwise h2c benefits the client-server performance and there is no obvious reason to prevent it?
>  
>> I am not sure what else we do in post read, maybe someone else has an opinion about that. It certainly looks nicer in the OPTIONS * case. 
> 
> The most critical thing for all of us to pay attention to is the setenvif logic and mod_headers injection, things that are there to deliberately affect the initial read and processing of.  The admin may want to trick httpd into *not* honoring upgrades from specific user agent signatures that are known-wonky, or when other request headers are present.  So I suggested HOOK_LAST, although a slightly different order could work.  We carefully inject mod_ssl (not upgrade, but the core read request hook) to fall after mod_setenvif, but otherwise at HOOK_MIDDLE.
> 
> 
> 
> 

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Sorry it took so long to respond to this earlier post, some of this might
have already been covered...

On Mon, Dec 7, 2015 at 1:03 PM, Stefan Eissing <stefan.eissing@greenbytes.de
> wrote:

>
>
Think about CORS restrictions and other stuff.
> - I think its the protocol handlers job to deal with any request body.
>

Strongly disagree, because 1. the protocol switch mechansim isn't the
content handler of the originating request, the request body is a
request-level entity, and 2. TLS gave you a perfect illustration of
altering the connection stack to continue using the core protocol stack.

In the case of h2c, or even a mod_echo Upgrade: echo mechanism, the entire
request/response is going to be handed off to a new protocol stack, I
understand that.  But that is one of several use cases here, and that new
protocol stack is going to consume the network pipe and close it, I
trust?!?  And return the appropriate status that the connection is gone
even if it leaves the old code to mop up, e.g. lingering_close.

For h2 upgrade, for example, the switch is only offered for requests
> without body.
>

Well, h2c is advertised on the first request with or without a body, and
then never again. Other modules must advertise, e.g. in conjunction with
the 426 Upgrade Required.

In fact, how do you propose to handle an authentication case with h2c
living in near the handler phase, when it will alert during the authnz
phase that http1.1 has been disallowed/h2c is required for a particular
resource, and that upgrade (which could have occurred in the
post-read-request phase) has not yet occurred during auth?

- moving things to post read sounds tempting, however I'm not sure if we
> want to upgrade on non-authed request or not, for example.
>

This is transport layer, yes, we always address the protocol without
consideration as to authnz; the same as we decode gzip irrespective of
client auth.  The actual requests over h2c (or http/1.1) are *each* still
subject to auth per-request.

I can't see a justification for excluding a client other than the user
agent identifier (known inoperable agent), otherwise h2c benefits the
client-server performance and there is no obvious reason to prevent it?


> I am not sure what else we do in post read, maybe someone else has an
> opinion about that. It certainly looks nicer in the OPTIONS * case.
>

The most critical thing for all of us to pay attention to is the setenvif
logic and mod_headers injection, things that are there to deliberately
affect the initial read and processing of.  The admin may want to trick
httpd into *not* honoring upgrades from specific user agent signatures that
are known-wonky, or when other request headers are present.  So I suggested
HOOK_LAST, although a slightly different order could work.  We carefully
inject mod_ssl (not upgrade, but the core read request hook) to fall after
mod_setenvif, but otherwise at HOOK_MIDDLE.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 7:25 PM, Jacob Champion <ch...@gmail.com> wrote:

> On 12/07/2015 03:49 PM, William A Rowe Jr wrote:
>
>>     Just to confirm, the purpose of splitting this up into two separate
>>     calls to the same function is solely to deal with "OPTIONS *", which
>>     doesn't follow the same logical path in httpd? Or is there another
>>     reason?
>>
>> No, it is because the TLS handshake from the Upgrade: TLS  handling
>> is used in the auth path, everything from SSLRequireSSL to using the
>> CN of a client certificate for authentication.
>>
>
> Oh, I see. Thanks for the clarification.
>
>     Put another way, I'm going to propose WebSocket for the paths that
>>     the server admin wants, whether the incoming request is
>>     valid/authorized or not, and I still need some way to interrupt the
>>     upgrade if we're chosen and the client hasn't sent a proper request.
>>     Does that makes sense?
>>
>>
>> So how would a second invocation during the ap_invoke_handler cause
>> an interruption in this?
>>
>
> I don't think it would; that comment was unrelated to the "two calls"
> clarification. I'm just trying to work through whether or not the current
> architecture is sufficient for my use case.


I think it may be...


>
> Upgrade: TLS would occur in the early processing, Upgrade: WebSocket
>> would occur in the later processing phase, but still prior to invoking a
>> handler
>> for the HTTP/1.1 request.  Remember that there is no 'error' for a
>> malformed
>> upgrade request, only a failure to proceed with 101-switching protocols.
>>
>
> From the HTTP/1.1 point of view, yes. I could choose to simply ignore bad
> upgrades, eventually fall back to the file handler, and be perfectly
> HTTP-compliant.
>
> From the WebSocket point of view, though, server implementations have to
> reject improper upgrades with a 4xx. Letting a bad upgrade fall through to
> some other handler (which could possibly return 200 and some document) is
> non-compliant behavior. It would be nice (TM) if I had a hook to "accept
> the upgrade" (from httpd's point of view, not the client's), but perform my
> checks and send back an error code before the 101 proceeded. The only other
> option I see is to inject myself as a handler in an error case and hope I'm
> not preempted by some other module.
>

You can ap_die anywhere during the pre-handler phases, between or before
the second protocol switch offer.  What you are describing seems like
something well handled within the auth stack or in final fixups, no?  That
would include any upgrade-required status (as tls is going to do for
SSLRequireSSL when SSL optional is configured).


> Again, I need to write code so I have some solid information for you all
> instead of just speculation based on reading the source.


:)  I would have dove deeper already but this conversation had to happen
first, thanks for all the details of this specific use case.


> Right now the core Protocol API will only send an Upgrade: offer on the
>> first request in a chain of keepalive requests.  If the user agent asks
>> for
>> OPTIONS * and the WebSocket module declines to offer itself, and the
>> user agent continues with a GET /webapp/... request that the WebSocket
>> module is willing to upgrade, today the core Protocol API will not
>> advertise
>> that to the user agent on this kept-alive request for you.
>>
>
> Ah. That is unfortunate...
>

... but fixable.

Perhaps the advertising hook determines if it will 're-advertise', or not?

Of course there is a fail-case, and that would be Upgrade Required, but
>> the protocol module needs to set this sometime before handler invocation.
>> Right now you can't do that within core_upgrade_request hooks, because
>> without an Upgrade: request from the client, the core_upgrade_request
>> implementation calls none of the protocol hooks on a kept-alive 2nd or
>> later request, right now.
>>
>> Stupid question, is there an Upgrade: of protocols within HTTP/2?  An
>> obvious case would be the websocket connection embedded within the
>> h2c connection, but I expect that is not allowed.  TLS upgrade would be
>> nonsense on an h2c connection.  But is there a provision for it in spec?
>>
>
> No. Upgrade semantics are explicitly removed from HTTP/2 (section 8.1.1).
> I assume the rationale is that other protocols can be negotiated directly
> via either ALPN or HTTP/1.1 Upgrades, just like HTTP/2 is.
>

Thanks for clarifying.

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Jacob Champion <ch...@gmail.com>.
On 12/07/2015 03:49 PM, William A Rowe Jr wrote:
>     Just to confirm, the purpose of splitting this up into two separate
>     calls to the same function is solely to deal with "OPTIONS *", which
>     doesn't follow the same logical path in httpd? Or is there another
>     reason?
>
> No, it is because the TLS handshake from the Upgrade: TLS  handling
> is used in the auth path, everything from SSLRequireSSL to using the
> CN of a client certificate for authentication.

Oh, I see. Thanks for the clarification.

>     Put another way, I'm going to propose WebSocket for the paths that
>     the server admin wants, whether the incoming request is
>     valid/authorized or not, and I still need some way to interrupt the
>     upgrade if we're chosen and the client hasn't sent a proper request.
>     Does that makes sense?
>
>
> So how would a second invocation during the ap_invoke_handler cause
> an interruption in this?

I don't think it would; that comment was unrelated to the "two calls" 
clarification. I'm just trying to work through whether or not the 
current architecture is sufficient for my use case.

> Upgrade: TLS would occur in the early processing, Upgrade: WebSocket
> would occur in the later processing phase, but still prior to invoking a
> handler
> for the HTTP/1.1 request.  Remember that there is no 'error' for a malformed
> upgrade request, only a failure to proceed with 101-switching protocols.

 From the HTTP/1.1 point of view, yes. I could choose to simply ignore 
bad upgrades, eventually fall back to the file handler, and be perfectly 
HTTP-compliant.

 From the WebSocket point of view, though, server implementations have 
to reject improper upgrades with a 4xx. Letting a bad upgrade fall 
through to some other handler (which could possibly return 200 and some 
document) is non-compliant behavior. It would be nice (TM) if I had a 
hook to "accept the upgrade" (from httpd's point of view, not the 
client's), but perform my checks and send back an error code before the 
101 proceeded. The only other option I see is to inject myself as a 
handler in an error case and hope I'm not preempted by some other module.

Again, I need to write code so I have some solid information for you all 
instead of just speculation based on reading the source.

> Right now the core Protocol API will only send an Upgrade: offer on the
> first request in a chain of keepalive requests.  If the user agent asks for
> OPTIONS * and the WebSocket module declines to offer itself, and the
> user agent continues with a GET /webapp/... request that the WebSocket
> module is willing to upgrade, today the core Protocol API will not advertise
> that to the user agent on this kept-alive request for you.

Ah. That is unfortunate...

> Of course there is a fail-case, and that would be Upgrade Required, but
> the protocol module needs to set this sometime before handler invocation.
> Right now you can't do that within core_upgrade_request hooks, because
> without an Upgrade: request from the client, the core_upgrade_request
> implementation calls none of the protocol hooks on a kept-alive 2nd or
> later request, right now.
>
> Stupid question, is there an Upgrade: of protocols within HTTP/2?  An
> obvious case would be the websocket connection embedded within the
> h2c connection, but I expect that is not allowed.  TLS upgrade would be
> nonsense on an h2c connection.  But is there a provision for it in spec?

No. Upgrade semantics are explicitly removed from HTTP/2 (section 
8.1.1). I assume the rationale is that other protocols can be negotiated 
directly via either ALPN or HTTP/1.1 Upgrades, just like HTTP/2 is.

--Jacob

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 5:21 PM, Jacob Champion <ch...@gmail.com> wrote:

> On 12/07/2015 02:40 PM, William A Rowe Jr wrote:
>
>> Not "noise" at all... I'm imagining a mod_echo protocol example that
>> looks much like your use case...
>>
>> 1st call to core_upgrade_request in post_read_req hook, after setenvif
>> and mod_headers processing... tls is ready, echo (or websocket) would
>> not be ready.  Those ready indicate via ap_select_protocol hook response.
>>
>
> Just to confirm, the purpose of splitting this up into two separate calls
> to the same function is solely to deal with "OPTIONS *", which doesn't
> follow the same logical path in httpd? Or is there another reason?
>

No, it is because the TLS handshake from the Upgrade: TLS  handling
is used in the auth path, everything from SSLRequireSSL to using the
CN of a client certificate for authentication.

TLS Upgrade must occur somewhere early in the post_read_request
phase of operation.  For that matter, so should h2c, I'd imagine, but
those phases may already be invoked a second time in mod_http2,
someone else could verify or disclaim that.

... normal pre-handler request preparation and auth ...
>>
>> 2nd call to core_upgrade_request in ap_invoke_handler hook, after all
>> of these other preparations but excluding filter configuration (filters
>> may
>> differ for the new protocol), tls was handled earlier, echo (or websocket)
>> are now ready, based on auth results.  Those ready again indicate
>> via ap_select_protocol hook response.
>>
>> WDYT?
>>
>
> I have a suspicion that this isn't quite enough, at least not with the
> code that's currently there. I really need to write code to confirm, but:
> if WebSocket is enabled for the request-target path, but the upgrade
> request is malformed, I still want my module to propose a WebSocket upgrade
> to core, so that I have the ability to fail said upgrade with a 4xx _if_
> we're chosen as the eventual protocol. (If a different protocol is chosen,
> the client lucks out.)
>
> Put another way, I'm going to propose WebSocket for the paths that the
> server admin wants, whether the incoming request is valid/authorized or
> not, and I still need some way to interrupt the upgrade if we're chosen and
> the client hasn't sent a proper request. Does that makes sense?


So how would a second invocation during the ap_invoke_handler cause
an interruption in this?

Upgrade: TLS would occur in the early processing, Upgrade: WebSocket
would occur in the later processing phase, but still prior to invoking a
handler
for the HTTP/1.1 request.  Remember that there is no 'error' for a malformed
upgrade request, only a failure to proceed with 101-switching protocols.

Right now the core Protocol API will only send an Upgrade: offer on the
first request in a chain of keepalive requests.  If the user agent asks for
OPTIONS * and the WebSocket module declines to offer itself, and the
user agent continues with a GET /webapp/... request that the WebSocket
module is willing to upgrade, today the core Protocol API will not advertise
that to the user agent on this kept-alive request for you.

Of course there is a fail-case, and that would be Upgrade Required, but
the protocol module needs to set this sometime before handler invocation.
Right now you can't do that within core_upgrade_request hooks, because
without an Upgrade: request from the client, the core_upgrade_request
implementation calls none of the protocol hooks on a kept-alive 2nd or
later request, right now.

Stupid question, is there an Upgrade: of protocols within HTTP/2?  An
obvious case would be the websocket connection embedded within the
h2c connection, but I expect that is not allowed.  TLS upgrade would be
nonsense on an h2c connection.  But is there a provision for it in spec?

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Jacob Champion <ch...@gmail.com>.
On 12/07/2015 02:40 PM, William A Rowe Jr wrote:
> Not "noise" at all... I'm imagining a mod_echo protocol example that
> looks much like your use case...
>
> 1st call to core_upgrade_request in post_read_req hook, after setenvif
> and mod_headers processing... tls is ready, echo (or websocket) would
> not be ready.  Those ready indicate via ap_select_protocol hook response.

Just to confirm, the purpose of splitting this up into two separate 
calls to the same function is solely to deal with "OPTIONS *", which 
doesn't follow the same logical path in httpd? Or is there another reason?

> ... normal pre-handler request preparation and auth ...
>
> 2nd call to core_upgrade_request in ap_invoke_handler hook, after all
> of these other preparations but excluding filter configuration (filters may
> differ for the new protocol), tls was handled earlier, echo (or websocket)
> are now ready, based on auth results.  Those ready again indicate
> via ap_select_protocol hook response.
>
> WDYT?

I have a suspicion that this isn't quite enough, at least not with the 
code that's currently there. I really need to write code to confirm, 
but: if WebSocket is enabled for the request-target path, but the 
upgrade request is malformed, I still want my module to propose a 
WebSocket upgrade to core, so that I have the ability to fail said 
upgrade with a 4xx _if_ we're chosen as the eventual protocol. (If a 
different protocol is chosen, the client lucks out.)

Put another way, I'm going to propose WebSocket for the paths that the 
server admin wants, whether the incoming request is valid/authorized or 
not, and I still need some way to interrupt the upgrade if we're chosen 
and the client hasn't sent a proper request. Does that makes sense?

--Jacob

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Dec 7, 2015 at 4:10 PM, Jacob Champion <ch...@gmail.com> wrote:

> On 12/07/2015 11:55 AM, Jacob Champion wrote:
>
>>  > - moving things to post read sounds tempting, however I'm not sure if
>> we want to upgrade on non-authed request or not, for example. I am not
>> sure what else we do in post read, maybe someone else has an opinion
>> about that. It certainly looks nicer in the OPTIONS * case.
>>
>> WebSocket upgrades rely on authn headers and cookies; there is no (good)
>> way after a connection has been established to say "well, I upgraded you
>> but now I'm closing the connection because you weren't authorized." The
>> check needs to be done before sending 101 (and otherwise a 401/403/4xx
>> needs to be sent instead of the upgrade).
>>
>
> D'oh. My WebSocket rambling has nothing to do with anything you said -- I
> only just realized you were both talking about mod_ssl's hook
> implementations, not the general order of the hooks in the new Upgrade
> architecture...
>
> Sorry for the noise.


Not "noise" at all... I'm imagining a mod_echo protocol example that
looks much like your use case...

1st call to core_upgrade_request in post_read_req hook, after setenvif
and mod_headers processing... tls is ready, echo (or websocket) would
not be ready.  Those ready indicate via ap_select_protocol hook response.

... normal pre-handler request preparation and auth ...

2nd call to core_upgrade_request in ap_invoke_handler hook, after all
of these other preparations but excluding filter configuration (filters may
differ for the new protocol), tls was handled earlier, echo (or websocket)
are now ready, based on auth results.  Those ready again indicate
via ap_select_protocol hook response.

WDYT?

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Jacob Champion <ch...@gmail.com>.
On 12/07/2015 11:55 AM, Jacob Champion wrote:
>  > - moving things to post read sounds tempting, however I'm not sure if
> we want to upgrade on non-authed request or not, for example. I am not
> sure what else we do in post read, maybe someone else has an opinion
> about that. It certainly looks nicer in the OPTIONS * case.
>
> WebSocket upgrades rely on authn headers and cookies; there is no (good)
> way after a connection has been established to say "well, I upgraded you
> but now I'm closing the connection because you weren't authorized." The
> check needs to be done before sending 101 (and otherwise a 401/403/4xx
> needs to be sent instead of the upgrade).

D'oh. My WebSocket rambling has nothing to do with anything you said -- 
I only just realized you were both talking about mod_ssl's hook 
implementations, not the general order of the hooks in the new Upgrade 
architecture...

Sorry for the noise.

--Jacob

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Jacob Champion <ch...@gmail.com>.
Ugh, looks like I'm too slow on my work with the upgrade proposal hooks;
sorry I haven't been able to give in-depth feedback yet.

On Dec 7, 2015 11:09 AM, "Stefan Eissing" <st...@greenbytes.de>
wrote:
>
> Not at my machine to really check the runtime behaviour. Can do that
tomorrow. My thoughts so far:
>
> - the return code was not checked intentionally. Once the 101
intermediate response is sent, there is nothing core can do on the
connection. It has been switched to a protocol unknown to core. Failure in
the protocol handler won't make it HTTP/1 again. If we do not close the
connection afterwards, but try to read the next h1 request, we just expose
ourselves to someone inserting payload that tricks us. Think about CORS
restrictions and other stuff.

+1. httpd et al need to ensure an upgrade is possible before sending 101,
not after.

> - I think its the protocol handlers job to deal with any request body.
core does not need to do anything here. For h2 upgrade, for example, the
switch is only offered for requests without body. That exposes more
predictable behaviour to clients than ignoring the upgrade on some
arbitrary number of bytes. Other protocols might be fine with bodies, h2 is
not. This was a consensus on the http wg mailing list some months ago.

By "core doesn't need to do anything" do you mean that it doesn't need to
set up the incoming filter chain? Couldn't the initial request body have
been sent with transfer encodings and other transformations?

> - moving things to post read sounds tempting, however I'm not sure if we
want to upgrade on non-authed request or not, for example. I am not sure
what else we do in post read, maybe someone else has an opinion about that.
It certainly looks nicer in the OPTIONS * case.

WebSocket upgrades rely on authn headers and cookies; there is no (good)
way after a connection has been established to say "well, I upgraded you
but now I'm closing the connection because you weren't authorized." The
check needs to be done before sending 101 (and otherwise a 401/403/4xx
needs to be sent instead of the upgrade).

--Jacob

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by Stefan Eissing <st...@greenbytes.de>.
Not at my machine to really check the runtime behaviour. Can do that tomorrow. My thoughts so far:

- the return code was not checked intentionally. Once the 101 intermediate response is sent, there is nothing core can do on the connection. It has been switched to a protocol unknown to core. Failure in the protocol handler won't make it HTTP/1 again. If we do not close the connection afterwards, but try to read the next h1 request, we just expose ourselves to someone inserting payload that tricks us. Think about CORS restrictions and other stuff.
- I think its the protocol handlers job to deal with any request body. core does not need to do anything here. For h2 upgrade, for example, the switch is only offered for requests without body. That exposes more predictable behaviour to clients than ignoring the upgrade on some arbitrary number of bytes. Other protocols might be fine with bodies, h2 is not. This was a consensus on the http wg mailing list some months ago. 
- moving things to post read sounds tempting, however I'm not sure if we want to upgrade on non-authed request or not, for example. I am not sure what else we do in post read, maybe someone else has an opinion about that. It certainly looks nicer in the OPTIONS * case. 

//stefan

> Am 07.12.2015 um 19:02 schrieb William A Rowe Jr <wr...@rowe-clan.net>:
> 
> Here's my thoughts to address this rather quickly for 2.4.18 to keep
> ourselves on track; simply move the core protocol handling phase and 
> ap_switch_protocol action into the appropriate request processing phase, 
> address the fail-cases of the ap_switch_protocol exceptions that have 
> been overlooked so that users can start leveraging the API, and just fix
> the mod_ssl OPTIONS * advertisement as presented on trunk (I am
> updating STATUS in a moment for that patch).
> 
> The actual fix to incorporate mod_ssl TLS upgrade to work within the
> Protocol API can wait, so we don't further disrupt the overdue fixes
> for the mod_http2 feature for our users.  Although I would -like- them 
> to coexist nicely, I don't believe that must happen until 2.4.19.  If the
> SSLOptions optional works within one vhost, and Protocol h2c works
> on another vhost, I think we are in good shape for the experimental
> new feature release.
> 
> WDTY?
> 
> 
>> On Mon, Dec 7, 2015 at 10:38 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>> Hi folks, sorry for the late interruption after we have already shipped 2.4.16, but there seems to be an issue that merits revisiting before the 2.4.16 API schema is widely adopted.
>> 
>> We seem to have misplaced the upgrade handler in the wrong hook.  This is easily shown by the fact that we had two different upgrade paths between mod_ssl TLS upgrade, and h2c upgrade paths, and a crufty patch to solve the OPTIONS * request exception which isn't an exception at all per RFC7230.
>> 
>> In mod_ssl connection upgrade we once emitted the TLS upgrade header in fixups.  Fixups are bypassed by OPTIONS *.  My trunk patch moved this mod_ssl TLS upgrade handling to post read req, leaving OPTIONS * with its simple behavior.
>> 
>> In h2c this exists in an upgrade handler, and OPTIONS * was hacked to invoke this function.  Problem, it is a transport (hop by hop) detail and should not be presented in the request, but rather in the connection hooks.  Moreover, none of the other *request* processing phases appear valid prior to conducting the h2c upgrade, and some appear that they can be harmful to the admin attempting to configure Protocol h2c.
>> 
>> The relevant specs are clear, it is a hop-by-hop connection transport offer carried over the request headers.  It is not generally subject to auth processing, certainly not resource processing.  The underlying request must still be satisfied.  The current Protocols logic doesn't appear to do this because it was based off of tangential specs such as h2c that don't override 2616bis.  If we consider that a "method" has a "handler", and transfer "headers" are "filtered" it becomes more obvious.  This is not the UPGRADE method.
>> 
>> https://tools.ietf.org/html/rfc7230#section-6.7 makes things more interesting, it calls out that 101-continue and the request body read precedes the 101-switching protocols.  Not sure who decided that would be a good idea, sigh... but mod_ssl TLS upgrade has these reversed for several good reasons including the intent to encrypt the request body if present and simple economics of processing.
>> 
>> I think that handling upgrade advertisement and alerting must be in post read req, bypassing all request hooks until the 101-continue is presented, any small request body read and set aside for the http input brigade, and 101-switching protocols is presented.  This allows the request to still be processed for tls-style upgrades, or discarded for relevant protocols.
>> 
>> The beginning of a patch is attached, I note that the ap_switch_protocol return value was never properly interpreted and my patch doesn't begin to catch all of the exceptional cases.  Because 101-switching protocols has already been sent, if the call fails to perform an upgrade, it seems we need to respond with a generic 400 switch failed unless the registered protocol switch handler is going to ap_die with a different sort of specific error (e.g. "A server MUST NOT switch protocols unless the received message semantics can be honored by the new protocol; an OPTIONS request can be honored by any protocol.")
>> 
>> Stefan and other protocol gurus, please have a look at the proposed patch, thanks.
>> 
>> Bill
>> 
> 

Re: 2.4 pause - mod_http2 patchset Upgrade h2c vs mod_ssl Upgrade tls

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Here's my thoughts to address this rather quickly for 2.4.18 to keep
ourselves on track; simply move the core protocol handling phase and
ap_switch_protocol action into the appropriate request processing phase,
address the fail-cases of the ap_switch_protocol exceptions that have
been overlooked so that users can start leveraging the API, and just fix
the mod_ssl OPTIONS * advertisement as presented on trunk (I am
updating STATUS in a moment for that patch).

The actual fix to incorporate mod_ssl TLS upgrade to work within the
Protocol API can wait, so we don't further disrupt the overdue fixes
for the mod_http2 feature for our users.  Although I would -like- them
to coexist nicely, I don't believe that must happen until 2.4.19.  If the
SSLOptions optional works within one vhost, and Protocol h2c works
on another vhost, I think we are in good shape for the experimental
new feature release.

WDTY?


On Mon, Dec 7, 2015 at 10:38 AM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:

> Hi folks, sorry for the late interruption after we have already shipped
> 2.4.16, but there seems to be an issue that merits revisiting before the
> 2.4.16 API schema is widely adopted.
>
> We seem to have misplaced the upgrade handler in the wrong hook.  This is
> easily shown by the fact that we had two different upgrade paths between
> mod_ssl TLS upgrade, and h2c upgrade paths, and a crufty patch to solve the
> OPTIONS * request exception which isn't an exception at all per RFC7230.
>
> In mod_ssl connection upgrade we once emitted the TLS upgrade header in
> fixups.  Fixups are bypassed by OPTIONS *.  My trunk patch moved this
> mod_ssl TLS upgrade handling to post read req, leaving OPTIONS * with its
> simple behavior.
>
> In h2c this exists in an upgrade handler, and OPTIONS * was hacked to
> invoke this function.  Problem, it is a transport (hop by hop) detail and
> should not be presented in the request, but rather in the connection
> hooks.  Moreover, none of the other *request* processing phases appear
> valid prior to conducting the h2c upgrade, and some appear that they can be
> harmful to the admin attempting to configure Protocol h2c.
>
> The relevant specs are clear, it is a hop-by-hop connection transport
> offer carried over the request headers.  It is not generally subject to
> auth processing, certainly not resource processing.  The underlying request
> must still be satisfied.  The current Protocols logic doesn't appear to do
> this because it was based off of tangential specs such as h2c that don't
> override 2616bis.  If we consider that a "method" has a "handler", and
> transfer "headers" are "filtered" it becomes more obvious.  This is not the
> UPGRADE method.
>
> https://tools.ietf.org/html/rfc7230#section-6.7 makes things more
> interesting, it calls out that 101-continue and the request body read
> precedes the 101-switching protocols.  Not sure who decided that would be a
> good idea, sigh... but mod_ssl TLS upgrade has these reversed for several
> good reasons including the intent to encrypt the request body if present
> and simple economics of processing.
>
> I think that handling upgrade advertisement and alerting must be in post
> read req, bypassing all request hooks until the 101-continue is presented,
> any small request body read and set aside for the http input brigade, and
> 101-switching protocols is presented.  This allows the request to still be
> processed for tls-style upgrades, or discarded for relevant protocols.
>
> The beginning of a patch is attached, I note that the ap_switch_protocol
> return value was never properly interpreted and my patch doesn't begin to
> catch all of the exceptional cases.  Because 101-switching protocols has
> already been sent, if the call fails to perform an upgrade, it seems we
> need to respond with a generic 400 switch failed unless the registered
> protocol switch handler is going to ap_die with a different sort of
> specific error (e.g. "A server MUST NOT switch protocols unless the
> received message semantics can be honored by the new protocol; an OPTIONS
> request can be honored by any protocol.")
>
> Stefan and other protocol gurus, please have a look at the proposed patch,
> thanks.
>
> Bill
>