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 >
