mod_ssl: performing protocol switch directly after ALPN selection, mod_http2: connect...

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 3b91c99cee162a19b0e9a21e3270dd7dcd74277e..cc8fc286033f6f92ad5fbcab3cdbf5b4ab1becf7 100644 (file)
--- a/docs/manual/mod/mod_http2.xml
+++ b/docs/manual/mod/mod_http2.xml
@@ -56,7 +56,7 @@
         <name>H2Direct</name>
         <description>H2 Direct Protocol Switch</description>
         <syntax>H2Direct on|off</syntax>
-        <default>H2Direct on (for non TLS)</default>
+        <default>H2Direct off</default>
         <contextlist>
             <context>server config</context>
             <context>virtual host</context>
@@ -64,16 +64,23 @@
         
         <usage>
             <p>
-                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 
                 <directive module="core" type="section">VirtualHost</directive> 
-                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.
+            </p>
+            <p>
                 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. 
+            </p>
+            <p>
+                Since this detection implies that the client will send data on
+                new connection immediately, direct HTTP/2 mode is disabled by
+                default.
             </p>
             <example><title>Example</title>
                 <highlight language="config">

diff --git a/modules/http2/h2_config.c b/modules/http2/h2_config.c
index 6db702eca80345a997420b4ae62c6b5b78fbd721..fa4f3d6e0c6d5438fd2e211ae8249393e9784021 100644 (file)
--- 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 221f5118df9eba4844e05d3cf45e8669a60a4cbc..3915a8e43db4f4e99f2311728708cb618ea2a26d 100644 (file)
--- 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 487f7e606920c26fce185bbdf2b357f52f885c5d..616be156bb72910d4fd63bbb1618474e0671a635 100644 (file)
--- 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 2fe84310a1e9a27b5c830991fe69616599ec5ffb..1248ba6fcb8a9031942e66422075936207307915 100644 (file)
--- 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 242fa4089d28333bb4cd0ddccb93af0d8e5a91e2..94716e56e6b13e6e73165df3019159879badb30e 100644 (file)
--- 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;
 }