On Wed, Dec 9, 2015 at 6:50 PM, Jacob Champion <champio...@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 :)
