From: Joe Orton Date: Tue, 6 Nov 2007 15:02:32 +0000 (+0000) Subject: mod_ssl: Fix forever-broken TLS upgrade support; perform the upgrade X-Git-Tag: 2.3.0~1287 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c15016e058f3081bfbce0f1c1531cd70ef57124f;p=thirdparty%2Fapache%2Fhttpd.git mod_ssl: Fix forever-broken TLS upgrade support; perform the upgrade in the post_read_request hook rather than in a filter, and fix the filter insertion issue: * modules/ssl/ssl_engine_kernel.c (upgrade_connection): New function, mostly moved from ssl_io_filter_Upgrade. (ssl_hook_ReadReq): Call upgrade_connection to upgrade to TLS if required. * modules/ssl/ssl_engine_io.c (ssl_io_filter_Upgrade): Remove function. (ssl_io_input_add_filter, ssl_io_filter_init): Take a request_rec pointer and pass to ap_add_*_filter to ensure the filter chain is modified correctly; remove it from the filter afterwards. (ssl_io_filter_register): Drop UPGRADE_FILTER registration. * modules/ssl/mod_ssl.c (ssl_init_ssl_connection): Take a request_rec pointer, pass to ssl_io_filter_init. (ssl_hook_pre_connection): Pass NULL request_rec pointer to above. (ssl_hook_Insert_Filter): Remove function. (ssl_register_hooks): Drop insert_filter hook. * modules/ssl/ssl_private.h: Update prototypes. PR: 41231 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@592446 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 33a7c635d73..4927df2ea89 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,8 @@ Changes with Apache 2.3.0 [ When backported to 2.2.x, remove entry from this file ] + *) mod_ssl: Fix TLS upgrade (RFC 2817) support. PR 41231. [Joe Orton] + *) scoreboard: Correctly declare ap_time_process_request. PR 43789 [Tom Donovan ] diff --git a/modules/ssl/mod_ssl.c b/modules/ssl/mod_ssl.c index 8511228374a..dbaaca3951c 100644 --- a/modules/ssl/mod_ssl.c +++ b/modules/ssl/mod_ssl.c @@ -318,7 +318,7 @@ int ssl_engine_disable(conn_rec *c) return 1; } -int ssl_init_ssl_connection(conn_rec *c) +int ssl_init_ssl_connection(conn_rec *c, request_rec *r) { SSLSrvConfigRec *sc = mySrvConfig(c->base_server); SSL *ssl; @@ -381,7 +381,7 @@ int ssl_init_ssl_connection(conn_rec *c) SSL_set_verify_result(ssl, X509_V_OK); - ssl_io_filter_init(c, ssl); + ssl_io_filter_init(c, r, ssl); return APR_SUCCESS; } @@ -442,19 +442,10 @@ static int ssl_hook_pre_connection(conn_rec *c, void *csd) "Connection to child %ld established " "(server %s)", c->id, sc->vhost_id); - return ssl_init_ssl_connection(c); + return ssl_init_ssl_connection(c, NULL); } -static void ssl_hook_Insert_Filter(request_rec *r) -{ - SSLSrvConfigRec *sc = mySrvConfig(r->server); - - if (sc->enabled == SSL_ENABLED_OPTIONAL) { - ap_add_output_filter("UPGRADE_FILTER", NULL, r, r->connection); - } -} - /* * the module registration phase */ @@ -479,8 +470,6 @@ static void ssl_register_hooks(apr_pool_t *p) ap_hook_access_checker(ssl_hook_Access, NULL,NULL, APR_HOOK_MIDDLE); ap_hook_auth_checker (ssl_hook_Auth, NULL,NULL, APR_HOOK_MIDDLE); ap_hook_post_read_request(ssl_hook_ReadReq, pre_prr,NULL, APR_HOOK_MIDDLE); - ap_hook_insert_filter (ssl_hook_Insert_Filter, NULL,NULL, APR_HOOK_MIDDLE); -/* ap_hook_handler (ssl_hook_Upgrade, NULL,NULL, APR_HOOK_MIDDLE); */ ssl_var_register(p); diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c index 147da228818..65f8e7b3f1c 100644 --- a/modules/ssl/ssl_engine_io.c +++ b/modules/ssl/ssl_engine_io.c @@ -1170,85 +1170,6 @@ static int ssl_io_filter_connect(ssl_filter_ctx_t *filter_ctx) return APR_SUCCESS; } -#define SWITCH_STATUS_LINE "HTTP/1.1 101 Switching Protocols" -#define UPGRADE_HEADER "Upgrade: TLS/1.0, HTTP/1.1" -#define CONNECTION_HEADER "Connection: Upgrade" - -static apr_status_t ssl_io_filter_Upgrade(ap_filter_t *f, - apr_bucket_brigade *bb) -{ - const char *upgrade; - apr_bucket_brigade *upgradebb; - request_rec *r = f->r; - SSLConnRec *sslconn; - apr_status_t rv; - apr_bucket *b; - SSL *ssl; - - /* Just remove the filter, if it doesn't work the first time, it won't - * work at all for this request. - */ - ap_remove_output_filter(f); - - /* No need to ensure that this is a server with optional SSL, the filter - * is only inserted if that is true. - */ - - 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); - } - - apr_table_unset(r->headers_out, "Upgrade"); - - /* Send the interim 101 response. */ - upgradebb = apr_brigade_create(r->pool, f->c->bucket_alloc); - - ap_fputstrs(f->next, upgradebb, SWITCH_STATUS_LINE, CRLF, - UPGRADE_HEADER, CRLF, CONNECTION_HEADER, CRLF, CRLF, NULL); - - b = apr_bucket_flush_create(f->c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(upgradebb, b); - - rv = ap_pass_brigade(f->next, upgradebb); - if (rv) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, - "could not send interim 101 Upgrade response"); - return AP_FILTER_ERROR; - } - - ssl_init_ssl_connection(f->c); - - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, - "Awaiting re-negotiation handshake"); - - sslconn = myConnConfig(f->c); - ssl = sslconn->ssl; - - /* XXX: Should replace SSL_set_state with SSL_renegotiate(ssl); - * However, this causes failures in perl-framework currently, - * perhaps pre-test if we have already negotiated? - */ - SSL_set_accept_state(ssl); - SSL_do_handshake(ssl); - - if (SSL_get_state(ssl) != SSL_ST_OK) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "TLS Upgrade handshake failed: " - "Not accepted by client!?"); - - return AP_FILTER_ERROR; - } - - /* Now that we have initialized the ssl connection which added the ssl_io_filter, - pass the brigade off to the connection based output filters so that the - request can complete encrypted */ - return ap_pass_brigade(f->c->output_filters, bb); - -} - static apr_status_t ssl_io_filter_input(ap_filter_t *f, apr_bucket_brigade *bb, ap_input_mode_t mode, @@ -1651,14 +1572,20 @@ static apr_status_t ssl_io_filter_buffer(ap_filter_t *f, return APR_SUCCESS; } +/* The request_rec pointer is passed in here only to ensure that the + * filter chain is modified correctly when doing a TLS upgrade. It + * must *not* be used otherwise. */ static void ssl_io_input_add_filter(ssl_filter_ctx_t *filter_ctx, conn_rec *c, - SSL *ssl) + request_rec *r, SSL *ssl) { bio_filter_in_ctx_t *inctx; inctx = apr_palloc(c->pool, sizeof(*inctx)); - filter_ctx->pInputFilter = ap_add_input_filter(ssl_io_filter, inctx, NULL, c); + filter_ctx->pInputFilter = ap_add_input_filter(ssl_io_filter, inctx, r, c); + /* Immediately forget the request_rec pointer stored in the + * filter; it will go out of scope. */ + filter_ctx->pInputFilter->r = NULL; filter_ctx->pbioRead = BIO_new(&bio_filter_in_method); filter_ctx->pbioRead->ptr = (void *)inctx; @@ -1675,7 +1602,10 @@ static void ssl_io_input_add_filter(ssl_filter_ctx_t *filter_ctx, conn_rec *c, inctx->filter_ctx = filter_ctx; } -void ssl_io_filter_init(conn_rec *c, SSL *ssl) +/* The request_rec pointer is passed in here only to ensure that the + * filter chain is modified correctly when doing a TLS upgrade. It + * must *not* be used otherwise. */ +void ssl_io_filter_init(conn_rec *c, request_rec *r, SSL *ssl) { ssl_filter_ctx_t *filter_ctx; server_rec *s = c->base_server; @@ -1685,7 +1615,10 @@ void ssl_io_filter_init(conn_rec *c, SSL *ssl) filter_ctx->nobuffer = 0; filter_ctx->pOutputFilter = ap_add_output_filter(ssl_io_filter, - filter_ctx, NULL, c); + filter_ctx, r, c); + /* Immediately forget the request_rec pointer stored in the + * filter; it will go out of scope. */ + filter_ctx->pOutputFilter->r = NULL; filter_ctx->pbioWrite = BIO_new(&bio_filter_out_method); filter_ctx->pbioWrite->ptr = (void *)bio_filter_out_ctx_new(filter_ctx, c); @@ -1693,7 +1626,7 @@ void ssl_io_filter_init(conn_rec *c, SSL *ssl) /* We insert a clogging input filter. Let the core know. */ c->clogging_input_filters = 1; - ssl_io_input_add_filter(filter_ctx, c, ssl); + ssl_io_input_add_filter(filter_ctx, c, r, ssl); SSL_set_bio(ssl, filter_ctx->pbioRead, filter_ctx->pbioWrite); filter_ctx->pssl = ssl; @@ -1712,11 +1645,6 @@ void ssl_io_filter_init(conn_rec *c, SSL *ssl) void ssl_io_filter_register(apr_pool_t *p) { - /* This filter MUST be after the HTTP_HEADER filter, but it also must be - * a resource-level filter so it has the request_rec. - */ - ap_register_output_filter ("UPGRADE_FILTER", ssl_io_filter_Upgrade, NULL, AP_FTYPE_PROTOCOL + 5); - ap_register_input_filter (ssl_io_filter, ssl_io_filter_input, NULL, AP_FTYPE_CONNECTION + 5); ap_register_output_filter (ssl_io_filter, ssl_io_filter_output, NULL, AP_FTYPE_CONNECTION + 5); diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c index 0006fa3dcb1..58757cc132a 100644 --- a/modules/ssl/ssl_engine_kernel.c +++ b/modules/ssl/ssl_engine_kernel.c @@ -32,14 +32,83 @@ static void ssl_configure_env(request_rec *r, SSLConnRec *sslconn); +#define SWITCH_STATUS_LINE "HTTP/1.1 101 Switching Protocols" +#define UPGRADE_HEADER "Upgrade: TLS/1.0, HTTP/1.1" +#define CONNECTION_HEADER "Connection: Upgrade" + +/* Perform an upgrade-to-TLS for the given request, per RFC 2817. */ +static apr_status_t upgrade_connection(request_rec *r) +{ + struct conn_rec *conn = r->connection; + apr_bucket_brigade *bb; + SSLConnRec *sslconn; + apr_status_t rv; + SSL *ssl; + + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, + "upgrading connection to TLS"); + + bb = apr_brigade_create(r->pool, conn->bucket_alloc); + + rv = ap_fputstrs(conn->output_filters, bb, SWITCH_STATUS_LINE, CRLF, + UPGRADE_HEADER, CRLF, CONNECTION_HEADER, CRLF, CRLF, NULL); + if (rv == APR_SUCCESS) { + APR_BRIGADE_INSERT_TAIL(bb, + apr_bucket_flush_create(conn->bucket_alloc)); + rv = ap_pass_brigade(conn->output_filters, bb); + } + + if (rv) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "failed to send 101 interim response for connection " + "upgrade"); + return rv; + } + + ssl_init_ssl_connection(conn, r); + + sslconn = myConnConfig(conn); + ssl = sslconn->ssl; + + /* XXX: Should replace SSL_set_state with SSL_renegotiate(ssl); + * However, this causes failures in perl-framework currently, + * perhaps pre-test if we have already negotiated? + */ + SSL_set_accept_state(ssl); + SSL_do_handshake(ssl); + + if (SSL_get_state(ssl) != SSL_ST_OK) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "TLS upgrade handshake failed: not accepted by client!?"); + + return APR_ECONNABORTED; + } + + return APR_SUCCESS; +} + /* * Post Read Request Handler */ int ssl_hook_ReadReq(request_rec *r) { - SSLConnRec *sslconn = myConnConfig(r->connection); + SSLSrvConfigRec *sc = mySrvConfig(r->server); + SSLConnRec *sslconn; + const char *upgrade; SSL *ssl; + + /* Perform TLS upgrade here if "SSLEngine optional" is configured, + * SSL is not already set up for this connection, and the client + * has sent a suitable Upgrade header. */ + if (sc->enabled == SSL_ENABLED_OPTIONAL && !myConnConfig(r->connection) + && (upgrade = apr_table_get(r->headers_in, "Upgrade")) != NULL + && strcmp(ap_getword(r->pool, &upgrade, ','), "TLS/1.0") == 0) { + if (upgrade_connection(r)) { + return HTTP_INTERNAL_SERVER_ERROR; + } + } + sslconn = myConnConfig(r->connection); if (!sslconn) { return DECLINED; } diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index a2592cd6784..4d30dafe4d8 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -620,7 +620,7 @@ int ssl_proxy_enable(conn_rec *c); int ssl_engine_disable(conn_rec *c); /** I/O */ -void ssl_io_filter_init(conn_rec *, SSL *); +void ssl_io_filter_init(conn_rec *, request_rec *r, SSL *); void ssl_io_filter_register(apr_pool_t *); long ssl_io_data_cb(BIO *, int, MODSSL_BIO_CB_ARG_TYPE *, int, long, long); @@ -642,7 +642,7 @@ BOOL ssl_util_path_check(ssl_pathcheck_t, const char *, apr_pool_t *); ssl_algo_t ssl_util_algotypeof(X509 *, EVP_PKEY *); char *ssl_util_algotypestr(ssl_algo_t); void ssl_util_thread_setup(apr_pool_t *); -int ssl_init_ssl_connection(conn_rec *c); +int ssl_init_ssl_connection(conn_rec *c, request_rec *r); /** Pass Phrase Support */ void ssl_pphrase_Handle(server_rec *, apr_pool_t *);