cloudflare / sslconfig

Cloudflare's Internet facing SSL configuration
BSD 3-Clause "New" or "Revised" License
1.3k stars 132 forks source link

nginx h2 patch for 1.14.0 and fix spdy post bug #97

Open favortel opened 6 years ago

favortel commented 6 years ago

Nginx SPDY POST request will cause nginx segment fault in 1.14.0 and nginx-1.13.12

src/http/ngx_http_spdy.c:ngx_http_spdy_state_read_data buf->last = ngx_cpymem(buf->last, pos, size);

src/http/ngx_http_spdy.c:ngx_http_spdy_read_request_body

+    if (!r->request_body && ngx_http_spdy_init_request_body(r) != NGX_OK) {
+        stream->skip_data = NGX_SPDY_DATA_INTERNAL_ERROR;
+        return NGX_HTTP_INTERNAL_SERVER_ERROR;
+    }
...

Cause problem in nginx-1.13.12 and 1.14.0, r->request_body has been inited BEFORE calling ngx_http_spdy_read_request_body in src/http/ngx_http_request_body.c. So ngx_http_spdy_init_request_body will not run, and r->request_body->buf will be 0

If the patch for src/http/ngx_http_request_body.c still AFTER these codes:

 #if (NGX_HTTP_V2)
     if (r->stream) {
         rc = ngx_http_v2_read_request_body(r);
         goto done;
     }
 #endif

The r->request_body has been inited beforce ngx_http_spdy_read_request_body called, ngx_http_spdy_init_request_body will not run, and r->request_body->buf will be 0 https://github.com/cloudflare/sslconfig/blob/master/patches/nginx__1.13.0_http2_spdy.patch

 src/http/ngx_http_request_body.c
@@ -84,6 +84,12 @@ ngx_http_read_client_request_body(ngx_http_request_t *r,
         goto done;
     }
 #endif
+#if (NGX_HTTP_SPDY)
+    if (r->spdy_stream) {
+        rc = ngx_http_spdy_read_request_body(r, post_handler);
+        goto done;
+    }
+#endif

http://lxr.nginx.org/source/src/http/ngx_http_request_body.c 0070-0073

0054     rb = ngx_pcalloc(r->pool, sizeof(ngx_http_request_body_t));
0055     if (rb == NULL) {
0056         rc = NGX_HTTP_INTERNAL_SERVER_ERROR;
0057         goto done;
0058     }
0059 
0060     /*
0061      * set by ngx_pcalloc():
0062      *
0063      *     rb->bufs = NULL;
0064      *     rb->buf = NULL;
0065      *     rb->free = NULL;
0066      *     rb->busy = NULL;
0067      *     rb->chunked = NULL;
0068      */
0069 
0070     rb->rest = -1;
0071     rb->post_handler = post_handler;
0072 
0073     r->request_body = rb;
0074 
0075     if (r->headers_in.content_length_n < 0 && !r->headers_in.chunked) {
0076         r->request_body_no_buffering = 0;
0077         post_handler(r);
0078         return NGX_OK;
0079     }
0080 
0081 #if (NGX_HTTP_V2)
0082     if (r->stream) {
0083         rc = ngx_http_v2_read_request_body(r);
0084         goto done;
0085     }
0086 #endif

Which is different with nginx 1.12.2 http://lxr.nginx.org/source/src/http/ngx_http_request_body.c?v=nginx-1.12.2

0029 ngx_int_t
0030 ngx_http_read_client_request_body(ngx_http_request_t *r,
0031     ngx_http_client_body_handler_pt post_handler)
0032 {
0033     size_t                     preread;
0034     ssize_t                    size;
0035     ngx_int_t                  rc;
0036     ngx_buf_t                 *b;
0037     ngx_chain_t                out;
0038     ngx_http_request_body_t   *rb;
0039     ngx_http_core_loc_conf_t  *clcf;
0040 
0041     r->main->count++;
0042 
0043     if (r != r->main || r->request_body || r->discard_body) {
0044         r->request_body_no_buffering = 0;
0045         post_handler(r);
0046         return NGX_OK;
0047     }
0048 
0049 #if (NGX_HTTP_V2)
0050     if (r->stream) {
0051         rc = ngx_http_v2_read_request_body(r, post_handler);
0052         goto done;
0053     }
0054 #endif
...
...
0061     rb = ngx_pcalloc(r->pool, sizeof(ngx_http_request_body_t));
0062     if (rb == NULL) {
0063         rc = NGX_HTTP_INTERNAL_SERVER_ERROR;
0064         goto done;
0065     }
0066 
0067     /*
0068      * set by ngx_pcalloc():
0069      *
0070      *     rb->bufs = NULL;
0071      *     rb->buf = NULL;
0072      *     rb->free = NULL;
0073      *     rb->busy = NULL;
0074      *     rb->chunked = NULL;
0075      */
0076 
0077     rb->rest = -1;
0078     rb->post_handler = post_handler;
0079 
0080     r->request_body = rb;

So, I use "r->request_body->buf" replace "r->request_body"

+    if (!r->request_body->buf && ngx_http_spdy_init_request_body(r) != NGX_OK) {
+        stream->skip_data = NGX_SPDY_DATA_INTERNAL_ERROR;
+        return NGX_HTTP_INTERNAL_SERVER_ERROR;
+    }

Or you can move the following code BEFORE r->request_body has been inited

+#if (NGX_HTTP_SPDY)
+    if (r->spdy_stream) {
+        rc = ngx_http_spdy_read_request_body(r, post_handler);
+        goto done;
+    }
+#endif