Hi httpd folks, It seems that Joe Orton introduced a bug while updating ssl_engine_io.c between version 109499 and version 111159. The same bug was introduced into NetWare's mod_nw_ssl.c version 111327. (Please forgive me if that's not the correct way to reference svn version numbers... I'm new to svn.) The code in question is part of the TLS Upgrade feature described in RFC 2817 and was originally written by Ryan Bloom and committed by Bill Rowe if I'm not mistaken.
Compare the new code: upgrade = apr_table_get(r->headers_in, "Upgrade"); if (upgrade == NULL || strcmp(ap_getword(r->pool, &upgrade, ','), "TLS/1.0")) { /* "Upgrade: TLS/1.0, ..." header not found, don't do Upgrade */ return ap_pass_brigade(f->next, bb); } with the (bug-fixed, slightly cleaned-up) old code: if (upgrade == NULL) { /* "Upgrade: " header not found, don't do Upgrade */ return ap_pass_brigade(f->next, bb); } token_string = apr_pstrdup(r->pool,upgrade); token = apr_strtok(token_string,", ",&token_state); while (token && strcasecmp(token,"TLS/1.0")) { token = apr_strtok(NULL,", ",&token_state); } if (token == NULL) { /* "TLS/1.0" token not in Upgrade header, * don't do Upgrade */ return ap_pass_brigade(f->next, bb); } While the new code is much shorter, it introduces a bug. Consider the header "Upgrade: SSL/3.0, TLS/1.0" or better yet, a derivation of the example Upgrade header in the RFC 2616 snippet below with TLS/1.0 somewhere besides the first spot. The new code fails the upgrade on any Upgrade header whose first token is not "TLS/1.0". Also, I should mention that version 109499 had a bug where if there was an Upgrade header that didn't have TLS/1.0 as the first token, it got into an infinite loop. That's why I said bug-fixed above. Maybe someone who is more familiar with the apr_ and ap_ APIs can generate some working code that is shorter and cleaner than the strtok-based code above. Any correct implemenation I could think of using ap_getword seemed like it would be messier than the apr_strtok() version, especially since the tokens are in this case "each separated by one or more commas (",") and OPTIONAL linear white space (LWS)" (RFC 2616 2.1). If there isn't an ap_ or apr_ function that seperates these tokens more easily, I'm satisfied with the strtok based code above. Any objections to me asking Brad Nicholes to commit it and revert that part of 111159? (The other changes in that commit were very good) Thanks, Joel ----SNIP---- Here's the applicable verbage from RFC 2817 for your convenience: 3. Client Requested Upgrade to HTTP over TLS When the client sends an HTTP/1.1 request with an Upgrade header field containing the token "TLS/1.0", it is requesting the server to complete the current HTTP/1.1 request after switching to TLS/1.0. and from RFC 2616: 14.42 Upgrade The Upgrade general-header allows the client to specify what additional communication protocols it supports and would like to use if the server finds it appropriate to switch protocols. The server MUST use the Upgrade header field within a 101 (Switching Protocols) response to indicate which protocol(s) are being switched. Upgrade = "Upgrade" ":" 1#product For example, Upgrade: HTTP/2.0, SHTTP/1.3, IRC/6.9, RTA/x11