From: Stefan Eissing Date: Mon, 12 Oct 2015 13:13:45 +0000 (+0000) Subject: mod_ssl: performing protocol switch directly after ALPN selection, mod_http2: connect... X-Git-Tag: 2.5.0-alpha~2720 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ebb34c0b07ba8521ddf5fb0375152fb5ae2a8c7b;p=apache mod_ssl: performing protocol switch directly after ALPN selection, mod_http2: connection hook inits network filters to force TLS handshake, reads input only if H2Direct explicitly enabled, changes H2Direct default to off even for cleartext connections git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1708107 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/docs/manual/mod/mod_http2.xml b/docs/manual/mod/mod_http2.xml index 3b91c99cee..cc8fc28603 100644 --- a/docs/manual/mod/mod_http2.xml +++ b/docs/manual/mod/mod_http2.xml @@ -56,7 +56,7 @@ H2Direct H2 Direct Protocol Switch H2Direct on|off - H2Direct on (for non TLS) + H2Direct off server config virtual host @@ -64,16 +64,23 @@

- This directive toggles the usage of the HTTP/2 Direct Mode. This + This directive toggles the usage of the HTTP/2 Direct Mode. For https, this should be used inside a VirtualHost - section to enable direct HTTP/2 communication for that virtual host. + section to enable direct HTTP/2 communication for that virtual host. For + http, this directive only works in server configurations. +

+

Direct communication means that if the first bytes received by the server on a connection match the HTTP/2 preamble, the HTTP/2 protocol is switched to immediately without further negotiation. This mode falls outside the RFC 7540 but has become widely implemented - as it is very convenient for development and testing. - By default the direct HTTP/2 mode is enabled. + on cleartext ports as it is very convenient for development and testing. +

+

+ Since this detection implies that the client will send data on + new connection immediately, direct HTTP/2 mode is disabled by + default.

Example diff --git a/modules/http2/h2_config.c b/modules/http2/h2_config.c index 6db702eca8..fa4f3d6e0c 100644 --- a/modules/http2/h2_config.c +++ b/modules/http2/h2_config.c @@ -47,7 +47,7 @@ static h2_config defconf = { NULL, /* no alt-svcs */ -1, /* alt-svc max age */ 0, /* serialize headers */ - -1, /* h2 direct mode */ + 0, /* h2 direct mode */ -1, /* # session extra files */ }; diff --git a/modules/http2/h2_h2.c b/modules/http2/h2_h2.c index 221f5118df..3915a8e43d 100644 --- a/modules/http2/h2_h2.c +++ b/modules/http2/h2_h2.c @@ -135,7 +135,7 @@ int h2_h2_process_conn(conn_rec* c) { h2_ctx *ctx = h2_ctx_get(c); h2_config *cfg = h2_config_get(c); - apr_bucket_brigade* temp; + apr_bucket_brigade* temp = NULL; int is_tls = h2_h2_is_tls(c); ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, "h2_h2, process_conn"); @@ -150,11 +150,14 @@ int h2_h2_process_conn(conn_rec* c) */ if (!h2_ctx_protocol_get(c) && !strcmp(AP_PROTOCOL_HTTP1, ap_get_protocol(c))) { - apr_status_t status; + apr_status_t status = APR_SUCCESS; - temp = apr_brigade_create(c->pool, c->bucket_alloc); - status = ap_get_brigade(c->input_filters, temp, - AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24); + if (is_tls) { + /* trigger the TLS handshake */ + temp = apr_brigade_create(c->pool, c->bucket_alloc); + status = ap_get_brigade(c->input_filters, temp, + AP_MODE_INIT, APR_BLOCK_READ, 0); + } if (status == APR_SUCCESS) { if (h2_ctx_protocol_get(c) @@ -166,30 +169,43 @@ int h2_h2_process_conn(conn_rec* c) * http/1.1. Check the actual bytes read for the H2 Magic * Token, *if* H2Direct mode is enabled here. */ - int direct_mode = h2_config_geti(cfg, H2_CONF_DIRECT); - if (direct_mode > 0 || (direct_mode < 0 && !is_tls)) { + if (h2_config_geti(cfg, H2_CONF_DIRECT) > 0) { char *s = NULL; apr_size_t slen; - apr_brigade_pflatten(temp, &s, &slen, c->pool); - if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, - "h2_h2, direct mode detected"); - h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c"); + if (!temp) { + temp = apr_brigade_create(c->pool, c->bucket_alloc); + } + status = ap_get_brigade(c->input_filters, temp, + AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24); + if (status == APR_SUCCESS) { + apr_brigade_pflatten(temp, &s, &slen, c->pool); + if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, + "h2_h2, direct mode detected"); + h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c"); + } + else { + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, + "h2_h2, not detected in %d bytes: %s", + (int)slen, s); + } } else { - ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, - "h2_h2, not detected in %d bytes: %s", - (int)slen, s); + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c, + "h2_h2, error reading 24 bytes speculative"); } } } } else { ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c, - "h2_h2, error reading 24 bytes speculative"); + "h2_h2, failed to init connection"); + } + + if (temp) { + apr_brigade_destroy(temp); } - apr_brigade_destroy(temp); } /* If "h2" was selected as protocol (by whatever mechanism), take over diff --git a/modules/http2/h2_task_input.c b/modules/http2/h2_task_input.c index 487f7e6069..616be156bb 100644 --- a/modules/http2/h2_task_input.c +++ b/modules/http2/h2_task_input.c @@ -116,7 +116,7 @@ apr_status_t h2_task_input_read(h2_task_input *input, } if (mode == AP_MODE_INIT) { - return APR_SUCCESS; + return ap_get_brigade(f->c->input_filters, bb, mode, block, readbytes); } if (input->bb) { diff --git a/modules/ssl/ssl_engine_io.c b/modules/ssl/ssl_engine_io.c index 2fe84310a1..1248ba6fcb 100644 --- a/modules/ssl/ssl_engine_io.c +++ b/modules/ssl/ssl_engine_io.c @@ -336,9 +336,6 @@ typedef struct { apr_pool_t *pool; char buffer[AP_IOBUFSIZE]; ssl_filter_ctx_t *filter_ctx; -#ifdef HAVE_TLS_ALPN - int alpn_finished; /* 1 if ALPN has finished, 0 otherwise */ -#endif } bio_filter_in_ctx_t; /* @@ -1493,41 +1490,6 @@ static apr_status_t ssl_io_filter_input(ap_filter_t *f, APR_BRIGADE_INSERT_TAIL(bb, bucket); } -#ifdef HAVE_TLS_ALPN - /* By this point, Application-Layer Protocol Negotiation (ALPN) should be - * completed (if our version of OpenSSL supports it). If we haven't already, - * find out which protocol was decided upon and inform other modules - * by calling running protocol_switch hook. - */ - if (!inctx->alpn_finished) { - SSLConnRec *sslconn = myConnConfig(f->c); - const unsigned char *next_proto = NULL; - unsigned next_proto_len = 0; - const char *protocol; - - SSL_get0_alpn_selected(inctx->ssl, &next_proto, &next_proto_len); - if (next_proto && next_proto_len) { - protocol = apr_pstrmemdup(f->c->pool, (const char *)next_proto, - next_proto_len); - ap_log_cerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, f->c, - APLOGNO(02836) "ALPN selected protocol: '%s'", - protocol); - - if (strcmp(protocol, ap_get_protocol(f->c))) { - status = ap_switch_protocol(f->c, NULL, sslconn->server, - protocol); - if (status != APR_SUCCESS) { - ap_log_cerror(APLOG_MARK, APLOG_ERR, status, f->c, - APLOGNO(02908) "protocol switch to '%s' failed", - protocol); - return status; - } - } - } - inctx->alpn_finished = 1; - } -#endif - return APR_SUCCESS; } @@ -2008,9 +1970,6 @@ static void ssl_io_input_add_filter(ssl_filter_ctx_t *filter_ctx, conn_rec *c, inctx->block = APR_BLOCK_READ; inctx->pool = c->pool; inctx->filter_ctx = filter_ctx; -#ifdef HAVE_TLS_ALPN - inctx->alpn_finished = 0; -#endif } /* The request_rec pointer is passed in here only to ensure that the diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c index 242fa4089d..94716e56e6 100644 --- a/modules/ssl/ssl_engine_kernel.c +++ b/modules/ssl/ssl_engine_kernel.c @@ -2210,14 +2210,30 @@ int ssl_callback_alpn_select(SSL *ssl, init_vhost(c, ssl); proposed = ap_select_protocol(c, NULL, sslconn->server, client_protos); - *out = (const unsigned char *)(proposed? proposed : ap_get_protocol(c)); - len = strlen((const char*)*out); + if (!proposed) { + proposed = ap_get_protocol(c); + } + + len = strlen(proposed); if (len > 255) { ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(02840) "ALPN negotiated protocol name too long"); return SSL_TLSEXT_ERR_ALERT_FATAL; } + *out = (const unsigned char *)proposed; *outlen = (unsigned char)len; + + if (strcmp(proposed, ap_get_protocol(c))) { + apr_status_t status; + + status = ap_switch_protocol(c, NULL, sslconn->server, proposed); + if (status != APR_SUCCESS) { + ap_log_cerror(APLOG_MARK, APLOG_ERR, status, c, + APLOGNO(02908) "protocol switch to '%s' failed", + proposed); + return SSL_TLSEXT_ERR_ALERT_FATAL; + } + } return SSL_TLSEXT_ERR_OK; }