[PATCH 2 of 3] HTTP/2: "http2" directive

Roman Arutyunyan arut at nginx.com
Thu Feb 9 12:02:34 UTC 2023


Hi,

On Thu, Feb 09, 2023 at 03:28:02PM +0400, Sergey Kandaurov wrote:
> 
> > On 7 Feb 2023, at 18:50, Roman Arutyunyan <arut at nginx.com> wrote:
> > 
> > # HG changeset patch
> > # User Roman Arutyunyan <arut at nginx.com>
> > # Date 1675781276 -14400
> > #      Tue Feb 07 18:47:56 2023 +0400
> > # Branch quic
> > # Node ID 735f9e501922e4b0a1b20730d62bac35ea398336
> > # Parent  38eec3d9f2c0d2e6d041efe3ee6d9c1618d8f1e6
> > HTTP/2: "http2" directive.
> > 
> > The directive enables HTTP/2 in the current server.  The previous way to
> > enable HTTP/2 via "listen ... http2" is now deprecated.  The new approach
> > allows to share HTTP/2 and HTTP/0.9-1.1 on the same port.
> > 
> > For SSL connections, HTTP/2 is now selected by ALPN callback based on whether
> > the protocol is enabled in the virtual server chosen by SNI.  This however only
> > works since OpenSSL 1.0.2h, where ALPN callback is invoked after SNI callback.
> > For older versions of OpenSSL, HTTP/2 is enabled based on the default virtual
> > server configuration.
> > 
> > For plain TCP connections, HTTP/2 is now auto-detected by HTTP/2 preface, if
> > HTTP/2 is enabled in the default virtual server.  If preface is not matched,
> > HTTP/0.9-1.1 is assumed.
> > 
> > diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c
> > --- a/src/http/modules/ngx_http_ssl_module.c
> > +++ b/src/http/modules/ngx_http_ssl_module.c
> > @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t 
> > #if (NGX_HTTP_V2 || NGX_HTTP_V3)
> >     ngx_http_connection_t   *hc;
> > #endif
> > +#if (NGX_HTTP_V2)
> > +    ngx_http_v2_srv_conf_t  *h2scf;
> > +#endif
> > #if (NGX_HTTP_V3)
> >     ngx_http_v3_srv_conf_t  *h3scf;
> > #endif
> > @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t 
> >     hc = c->data;
> > #endif
> > 
> > -#if (NGX_HTTP_V2)
> > -    if (hc->addr_conf->http2) {
> > -        srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS;
> > -        srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1;
> > -    } else
> > -#endif
> > +    srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS;
> > +    srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1;
> > +
> > #if (NGX_HTTP_V3)
> >     if (hc->addr_conf->quic) {
> > 
> > @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t 
> > 
> >     } else
> > #endif
> > +#if (NGX_HTTP_V2)
> >     {
> > -        srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS;
> > -        srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1;
> > +        h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module);
> > +
> > +        if (h2scf->enable || hc->addr_conf->http2) {
> > +            srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS;
> > +            srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1;
> > +        }
> >     }
> > +#endif
> 
> With NGX_HTTP_V3 defined but NGX_HTTP_V2 not,
> the else part will go to the SSL_select_next_proto() call.
> So, to fix this, NGX_HTTP_ALPN_PROTOS still has to be the last resort,
> for simplicity (and also reduces diff).
> 
> My version:
> 
> diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c
> --- a/src/http/modules/ngx_http_ssl_module.c
> +++ b/src/http/modules/ngx_http_ssl_module.c
> @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t 
>  #if (NGX_HTTP_V2 || NGX_HTTP_V3)
>      ngx_http_connection_t   *hc;
>  #endif
> +#if (NGX_HTTP_V2)
> +    ngx_http_v2_srv_conf_t  *h2scf;
> +#endif
>  #if (NGX_HTTP_V3)
>      ngx_http_v3_srv_conf_t  *h3scf;
>  #endif
> @@ -449,7 +452,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t 
>  #endif
>  
>  #if (NGX_HTTP_V2)
> -    if (hc->addr_conf->http2) {
> +    h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module);
> +
> +    if (h2scf->enable || hc->addr_conf->http2) {
>          srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS;
>          srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1;
>      } else
> 
> > 
> >     if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen,
> >                               in, inlen)
> 

Thanks for noticing this.  If fixed this way, nginx will be able to negotiate
HTTP/2 over QUIC, which is probably not what we want.

Following a discussion with Sergey, here's an updated version of the patch.

--
Roman Arutyunyan
-------------- next part --------------
# HG changeset patch
# User Roman Arutyunyan <arut at nginx.com>
# Date 1675781276 -14400
#      Tue Feb 07 18:47:56 2023 +0400
# Branch quic
# Node ID 25c6c44600f6f0e2aa947e184790e04cedf4ff6d
# Parent  36550cf579bc032586b5a1695c41b43c1f7e34d3
HTTP/2: "http2" directive.

The directive enables HTTP/2 in the current server.  The previous way to
enable HTTP/2 via "listen ... http2" is now deprecated.  The new approach
allows to share HTTP/2 and HTTP/0.9-1.1 on the same port.

For SSL connections, HTTP/2 is now selected by ALPN callback based on whether
the protocol is enabled in the virtual server chosen by SNI.  This however only
works since OpenSSL 1.0.2h, where ALPN callback is invoked after SNI callback.
For older versions of OpenSSL, HTTP/2 is enabled based on the default virtual
server configuration.

For plain TCP connections, HTTP/2 is now auto-detected by HTTP/2 preface, if
HTTP/2 is enabled in the default virtual server.  If preface is not matched,
HTTP/0.9-1.1 is assumed.

diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.c
+++ b/src/http/modules/ngx_http_ssl_module.c
@@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t 
 #if (NGX_HTTP_V2 || NGX_HTTP_V3)
     ngx_http_connection_t   *hc;
 #endif
+#if (NGX_HTTP_V2)
+    ngx_http_v2_srv_conf_t  *h2scf;
+#endif
 #if (NGX_HTTP_V3)
     ngx_http_v3_srv_conf_t  *h3scf;
 #endif
@@ -448,12 +451,6 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t 
     hc = c->data;
 #endif
 
-#if (NGX_HTTP_V2)
-    if (hc->addr_conf->http2) {
-        srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS;
-        srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1;
-    } else
-#endif
 #if (NGX_HTTP_V3)
     if (hc->addr_conf->quic) {
 
@@ -480,8 +477,19 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t 
     } else
 #endif
     {
-        srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS;
-        srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1;
+#if (NGX_HTTP_V2)
+        h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module);
+
+        if (h2scf->enable || hc->addr_conf->http2) {
+            srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS;
+            srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1;
+
+        } else
+#endif
+        {
+            srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS;
+            srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1;
+        }
     }
 
     if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen,
diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -4179,6 +4179,11 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
 
         if (ngx_strcmp(value[n].data, "http2") == 0) {
 #if (NGX_HTTP_V2)
+            ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                               "the \"listen ... http2\" directive "
+                               "is deprecated, use "
+                               "the \"http2\" directive instead");
+
             lsopt.http2 = 1;
             continue;
 #else
diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_
     rev->handler = ngx_http_wait_request_handler;
     c->write->handler = ngx_http_empty_handler;
 
-#if (NGX_HTTP_V2)
-    if (hc->addr_conf->http2) {
-        rev->handler = ngx_http_v2_init;
-    }
-#endif
-
 #if (NGX_HTTP_V3)
     if (hc->addr_conf->quic) {
         ngx_http_v3_init_stream(c);
@@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_
     ngx_buf_t                 *b;
     ngx_connection_t          *c;
     ngx_http_connection_t     *hc;
+#if (NGX_HTTP_V2)
+    ngx_http_v2_srv_conf_t    *h2scf;
+#endif
     ngx_http_core_srv_conf_t  *cscf;
 
     c = rev->data;
@@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_
         b->end = b->last + size;
     }
 
+    size = b->end - b->last;
+
     n = c->recv(c, b->last, size);
 
     if (n == NGX_AGAIN) {
@@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_
             return;
         }
 
-        /*
-         * We are trying to not hold c->buffer's memory for an idle connection.
-         */
-
-        if (ngx_pfree(c->pool, b->start) == NGX_OK) {
-            b->start = NULL;
+        if (b->pos == b->last) {
+
+            /*
+             * We are trying to not hold c->buffer's memory for an
+             * idle connection.
+             */
+
+            if (ngx_pfree(c->pool, b->start) == NGX_OK) {
+                b->start = NULL;
+            }
         }
 
         return;
@@ -489,10 +492,34 @@ ngx_http_wait_request_handler(ngx_event_
         }
     }
 
+    ngx_reusable_connection(c, 0);
+
+#if (NGX_HTTP_V2)
+
+    h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module);
+
+    if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) {
+
+        size = ngx_min(sizeof(NGX_HTTP_V2_PREFACE) - 1,
+                       (size_t) (b->last - b->pos));
+
+        if (ngx_memcmp(b->pos, NGX_HTTP_V2_PREFACE, size) == 0) {
+
+            if (size == sizeof(NGX_HTTP_V2_PREFACE) - 1) {
+                ngx_http_v2_init(rev);
+                return;
+            }
+
+            c->log->action = "waiting for request";
+            ngx_post_event(rev, &ngx_posted_events);
+            return;
+        }
+    }
+
+#endif
+
     c->log->action = "reading client request line";
 
-    ngx_reusable_connection(c, 0);
-
     c->data = ngx_http_create_request(c);
     if (c->data == NULL) {
         ngx_http_close_connection(c);
@@ -808,13 +835,16 @@ ngx_http_ssl_handshake_handler(ngx_conne
 #if (NGX_HTTP_V2                                                              \
      && defined TLSEXT_TYPE_application_layer_protocol_negotiation)
         {
-        unsigned int            len;
-        const unsigned char    *data;
-        ngx_http_connection_t  *hc;
+        unsigned int             len;
+        const unsigned char     *data;
+        ngx_http_connection_t   *hc;
+        ngx_http_v2_srv_conf_t  *h2scf;
 
         hc = c->data;
 
-        if (hc->addr_conf->http2) {
+        h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module);
+
+        if (h2scf->enable || hc->addr_conf->http2) {
 
             SSL_get0_alpn_selected(c->ssl->connection, &data, &len);
 
diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -63,8 +63,6 @@ static void ngx_http_v2_handle_connectio
 static void ngx_http_v2_lingering_close(ngx_connection_t *c);
 static void ngx_http_v2_lingering_close_handler(ngx_event_t *rev);
 
-static u_char *ngx_http_v2_state_proxy_protocol(ngx_http_v2_connection_t *h2c,
-    u_char *pos, u_char *end);
 static u_char *ngx_http_v2_state_preface(ngx_http_v2_connection_t *h2c,
     u_char *pos, u_char *end);
 static u_char *ngx_http_v2_state_preface_end(ngx_http_v2_connection_t *h2c,
@@ -232,6 +230,7 @@ static ngx_http_v2_parse_header_t  ngx_h
 void
 ngx_http_v2_init(ngx_event_t *rev)
 {
+    u_char                    *p, *end;
     ngx_connection_t          *c;
     ngx_pool_cleanup_t        *cln;
     ngx_http_connection_t     *hc;
@@ -314,8 +313,7 @@ ngx_http_v2_init(ngx_event_t *rev)
         return;
     }
 
-    h2c->state.handler = hc->proxy_protocol ? ngx_http_v2_state_proxy_protocol
-                                            : ngx_http_v2_state_preface;
+    h2c->state.handler = ngx_http_v2_state_preface;
 
     ngx_queue_init(&h2c->waiting);
     ngx_queue_init(&h2c->dependencies);
@@ -333,7 +331,23 @@ ngx_http_v2_init(ngx_event_t *rev)
     }
 
     c->idle = 1;
-    ngx_reusable_connection(c, 0);
+
+    if (c->buffer) {
+        p = c->buffer->pos;
+        end = c->buffer->last;
+
+        do {
+            p = h2c->state.handler(h2c, p, end);
+
+            if (p == NULL) {
+                return;
+            }
+
+        } while (p != end);
+
+        h2c->total_bytes += p - c->buffer->pos;
+        c->buffer->pos = p;
+    }
 
     ngx_http_v2_read_handler(rev);
 }
@@ -847,31 +861,10 @@ ngx_http_v2_lingering_close_handler(ngx_
 
 
 static u_char *
-ngx_http_v2_state_proxy_protocol(ngx_http_v2_connection_t *h2c, u_char *pos,
-    u_char *end)
-{
-    ngx_log_t  *log;
-
-    log = h2c->connection->log;
-    log->action = "reading PROXY protocol";
-
-    pos = ngx_proxy_protocol_read(h2c->connection, pos, end);
-
-    log->action = "processing HTTP/2 connection";
-
-    if (pos == NULL) {
-        return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_PROTOCOL_ERROR);
-    }
-
-    return ngx_http_v2_state_preface(h2c, pos, end);
-}
-
-
-static u_char *
 ngx_http_v2_state_preface(ngx_http_v2_connection_t *h2c, u_char *pos,
     u_char *end)
 {
-    static const u_char preface[] = "PRI * HTTP/2.0\r\n";
+    static const u_char preface[] = NGX_HTTP_V2_PREFACE_START;
 
     if ((size_t) (end - pos) < sizeof(preface) - 1) {
         return ngx_http_v2_state_save(h2c, pos, end, ngx_http_v2_state_preface);
@@ -892,7 +885,7 @@ static u_char *
 ngx_http_v2_state_preface_end(ngx_http_v2_connection_t *h2c, u_char *pos,
     u_char *end)
 {
-    static const u_char preface[] = "\r\nSM\r\n\r\n";
+    static const u_char preface[] = NGX_HTTP_V2_PREFACE_END;
 
     if ((size_t) (end - pos) < sizeof(preface) - 1) {
         return ngx_http_v2_state_save(h2c, pos, end,
@@ -3946,10 +3939,22 @@ static void
 ngx_http_v2_run_request(ngx_http_request_t *r)
 {
     ngx_connection_t          *fc;
+    ngx_http_v2_srv_conf_t    *h2scf;
     ngx_http_v2_connection_t  *h2c;
 
     fc = r->connection;
 
+    h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module);
+
+    if (!h2scf->enable && !r->http_connection->addr_conf->http2) {
+        ngx_log_error(NGX_LOG_INFO, fc->log, 0,
+                      "client attempted to request the server name "
+                      "for which the negotiated protocol is disabled");
+
+        ngx_http_finalize_request(r, NGX_HTTP_MISDIRECTED_REQUEST);
+        goto failed;
+    }
+
     if (ngx_http_v2_construct_request_line(r) != NGX_OK) {
         goto failed;
     }
diff --git a/src/http/v2/ngx_http_v2.h b/src/http/v2/ngx_http_v2.h
--- a/src/http/v2/ngx_http_v2.h
+++ b/src/http/v2/ngx_http_v2.h
@@ -64,6 +64,16 @@ typedef u_char *(*ngx_http_v2_handler_pt
 
 
 typedef struct {
+    ngx_flag_t                       enable;
+    size_t                           pool_size;
+    ngx_uint_t                       concurrent_streams;
+    ngx_uint_t                       concurrent_pushes;
+    size_t                           preread_size;
+    ngx_uint_t                       streams_index_mask;
+} ngx_http_v2_srv_conf_t;
+
+
+typedef struct {
     ngx_str_t                        name;
     ngx_str_t                        value;
 } ngx_http_v2_header_t;
@@ -408,9 +418,17 @@ ngx_int_t ngx_http_v2_table_size(ngx_htt
 #define NGX_HTTP_V2_USER_AGENT_INDEX      58
 #define NGX_HTTP_V2_VARY_INDEX            59
 
+#define NGX_HTTP_V2_PREFACE_START         "PRI * HTTP/2.0\r\n"
+#define NGX_HTTP_V2_PREFACE_END           "\r\nSM\r\n\r\n"
+#define NGX_HTTP_V2_PREFACE               NGX_HTTP_V2_PREFACE_START           \
+                                          NGX_HTTP_V2_PREFACE_END
+
 
 u_char *ngx_http_v2_string_encode(u_char *dst, u_char *src, size_t len,
     u_char *tmp, ngx_uint_t lower);
 
 
+extern ngx_module_t  ngx_http_v2_module;
+
+
 #endif /* _NGX_HTTP_V2_H_INCLUDED_ */
diff --git a/src/http/v2/ngx_http_v2_module.c b/src/http/v2/ngx_http_v2_module.c
--- a/src/http/v2/ngx_http_v2_module.c
+++ b/src/http/v2/ngx_http_v2_module.c
@@ -75,6 +75,13 @@ static ngx_conf_post_t  ngx_http_v2_chun
 
 static ngx_command_t  ngx_http_v2_commands[] = {
 
+    { ngx_string("http2"),
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG,
+      ngx_conf_set_flag_slot,
+      NGX_HTTP_SRV_CONF_OFFSET,
+      offsetof(ngx_http_v2_srv_conf_t, enable),
+      NULL },
+
     { ngx_string("http2_recv_buffer_size"),
       NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1,
       ngx_conf_set_size_slot,
@@ -314,6 +321,8 @@ ngx_http_v2_create_srv_conf(ngx_conf_t *
         return NULL;
     }
 
+    h2scf->enable = NGX_CONF_UNSET;
+
     h2scf->pool_size = NGX_CONF_UNSET_SIZE;
 
     h2scf->concurrent_streams = NGX_CONF_UNSET_UINT;
@@ -333,6 +342,8 @@ ngx_http_v2_merge_srv_conf(ngx_conf_t *c
     ngx_http_v2_srv_conf_t *prev = parent;
     ngx_http_v2_srv_conf_t *conf = child;
 
+    ngx_conf_merge_value(conf->enable, prev->enable, 0);
+
     ngx_conf_merge_size_value(conf->pool_size, prev->pool_size, 4096);
 
     ngx_conf_merge_uint_value(conf->concurrent_streams,
diff --git a/src/http/v2/ngx_http_v2_module.h b/src/http/v2/ngx_http_v2_module.h
--- a/src/http/v2/ngx_http_v2_module.h
+++ b/src/http/v2/ngx_http_v2_module.h
@@ -21,15 +21,6 @@ typedef struct {
 
 
 typedef struct {
-    size_t                          pool_size;
-    ngx_uint_t                      concurrent_streams;
-    ngx_uint_t                      concurrent_pushes;
-    size_t                          preread_size;
-    ngx_uint_t                      streams_index_mask;
-} ngx_http_v2_srv_conf_t;
-
-
-typedef struct {
     size_t                          chunk_size;
 
     ngx_flag_t                      push_preload;
@@ -39,7 +30,4 @@ typedef struct {
 } ngx_http_v2_loc_conf_t;
 
 
-extern ngx_module_t  ngx_http_v2_module;
-
-
 #endif /* _NGX_HTTP_V2_MODULE_H_INCLUDED_ */


More information about the nginx-devel mailing list