Update NGINX SPDY patch for 1.9.15 #36

felixbuenemann · 2016-05-27T04:14:00Z

This updates the patch to work with NGINX 1.9.15 and also fixes a problem with the previous patch, that did not re-enable the spdy_* directives that were marked as deprecated by the http2 module as well as fixing a compile problem when the http2 module is enabled, but spdy is disabled.

See blog.cloudflare.com/open-sourcing-our-nginx-http-2-spdy-code

felixbuenemann · 2016-05-27T04:17:03Z

Btw. the patch also applies to 1.11.0 with some fuzz, but I have only tested it on 1.9.15.

felixbuenemann · 2016-05-27T04:18:33Z

@jgrahamc If you want to keep the 1.9.7 patch around, I could also rename the updated patch and send a separate bugfix for the spdy directives in the old patch.

jgrahamc · 2016-05-27T10:21:38Z

This is cool. Can you submit an updated patch instead so we can keep the old one around as well?

felixbuenemann · 2016-05-27T17:01:53Z

@jgrahamc I have separated the patch and fixed the non-working spdy_* directives in the 1.9.7 patch. I have also renamed the old patch to add the version number in order to avoid confusion.

felixbuenemann · 2016-05-27T18:57:21Z

Btw. I tested the 1.9.15 patch against 1.11.0 and it works fine. Current stable 1.10.0 is identical to 1.9.15 so it'll work too.

kn007 · 2016-05-29T13:53:59Z

Using with both --with-http_spdy_module and --with-http_v2_module, it's work. Thank you.

But when I test this patch in 1.9.15 and 1.11.0, it's return same error when without both --with-http_spdy_module and --with-http_v2_module :

src/http/modules/ngx_http_ssl_module.c: In function  ngx_http_ssl_npn_advertised :
src/http/modules/ngx_http_ssl_module.c:445:5: error: expected expression before  }  token
make[1]: *** [objs/src/http/modules/ngx_http_ssl_module.o] Error 1

src/http/modules/ngx_http_ssl_module.c :

 397 static int
 398 ngx_http_ssl_npn_advertised(ngx_ssl_conn_t *ssl_conn,
 399     const unsigned char **out, unsigned int *outlen, void *arg)
 400 {
 401 #if (NGX_HTTP_V2 || NGX_HTTP_SPDY || NGX_DEBUG)
 402     ngx_connection_t  *c;
 403
 404     c = ngx_ssl_get_connection(ssl_conn);
 405     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "SSL NPN advertised");
 406 #endif
 407
 408 #if (NGX_HTTP_V2 || NGX_HTTP_SPDY)
 409     {
 410     ngx_http_connection_t  *hc;
 411
 412     hc = c->data;
 413 #endif
 414
 415 #if (NGX_HTTP_V2 && NGX_HTTP_SPDY)
 416     if (hc->addr_conf->http2 && hc->addr_conf->spdy) {
 417         *out = (unsigned char *) NGX_HTTP_V2_NPN_ADVERTISE
 418                                  NGX_SPDY_NPN_ADVERTISE
 419                                  NGX_HTTP_NPN_ADVERTISE;
 420         *outlen = sizeof(NGX_HTTP_V2_NPN_ADVERTISE NGX_SPDY_NPN_ADVERTISE
 421                          NGX_HTTP_NPN_ADVERTISE) - 1;
 422
 423         return SSL_TLSEXT_ERR_OK;
 424     } else
 425 #endif
 426 #if (NGX_HTTP_V2)
 427     if (hc->addr_conf->http2) {
 428         *out =
 429             (unsigned char *) NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE;
 430         *outlen = sizeof(NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1;
 431
 432         return SSL_TLSEXT_ERR_OK;
 433     } else
 434 #endif
 435 #if (NGX_HTTP_SPDY)
 436     if (hc->addr_conf->spdy) {
 437         *out = (unsigned char *) NGX_SPDY_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE;
 438         *outlen = sizeof(NGX_SPDY_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1;
 439
 440         return SSL_TLSEXT_ERR_OK;
 441     }
 442 #endif
 443
 444 #if (NGX_HTTP_V2 || NGX_HTTP_SPDY)
 445     }
 446 #endif
 447
 448     *out = (unsigned char *) NGX_HTTP_NPN_ADVERTISE;
 449     *outlen = sizeof(NGX_HTTP_NPN_ADVERTISE) - 1;
 450
 451     return SSL_TLSEXT_ERR_OK;
 452 }
 453
 454 #endif

gcc (GCC) 4.9.1 20140922 (Red Hat 4.9.1-10)

del line 409 & 445, it's work.

frostieDE · 2016-05-29T14:48:58Z

Works fine against nginx 1.11.0 and OpenSSL 1.0.2h (with ChaCha20 patch). I am building nginx as an Ubuntu 16.04 package.

Thanks for your work @felixbuenemann

felixbuenemann · 2016-05-29T21:40:18Z

@kn007 I believe the extra block was added for C89 compatibility, where all variables have do be declared at the start of a block. I'm not sure why that leads to a compile error for you. One thing that looks problematic about the above code is that if only HTTP2 is compiled, there's an unmatched else branch. I think the block can be avoided by re-ordering the init and debug log code.

felixbuenemann · 2016-05-29T21:42:43Z

@kn007 Oh, I just noticed I misread your first comment, you were talking about the case where only --with-http_v2_module is enabled. I'll look into that.

The various spdy configuration directives were still marked as deprecated by the http v2 module in the existing patch. Rhis change removes the deprecation so that the directives defined by the spdy module can be used again.

Tis fixes a compile error caused by the 1.9.7 NGINX SPDY patch due to an unmatched else branch, if the http_v2 module was enabled, but the spdy module was disabled.

This should make it clearer which version this patch is targeting, now that there is another patch for 1.9.15.

This updates the 1.9.7 patch to work with NGINX 1.9.15 and also fixes two problem with the previous patch, that did not re-enable spdy directives that were marked as deprecated by the http2 module and failed to compile when the http2 module was enabled and spdy was disabled.

felixbuenemann · 2016-05-29T22:36:08Z

@kn007 I have updated the 1.9.7 and 1.9.15 patches to fix the compile problem --with-http_v2_module but without --with-http_spdy_module.

Please do not use your fix removing lines 409 and 445, it is wrong and will only work by accident.

Thanks for the report!

felixbuenemann · 2016-05-29T22:38:58Z

@jgrahamc I have kept the 1.9.7 patch fixes separate. I can combine them into a single commit and/or move them over to a separate PR if you prefer.

kn007 · 2016-05-29T22:41:47Z

@felixbuenemann Got it, thank you. That's very kind of you.

felixbuenemann · 2016-05-29T22:43:50Z

@jgrahamc It would be nice if the nginx spdy patch was mentioned in the README.md. I can't speak on behalf of CloudFlare, so it's best if you did that. A short paragraph linking to the blog post would probably be sufficient.

xetorixik · 2016-06-02T03:28:31Z

The least I can do is write a big thank you for your time, effort and eventually the SPDY patch for Nginx 1.10 @felixbuenemann

kn007 · 2016-06-02T12:04:04Z

nginx 1.11.1 test pass

angristan · 2016-06-10T13:37:51Z

Not working anymore for me :

src/http/modules/ngx_http_ssl_module.c: In function ‘ngx_http_ssl_npn_advertised’:
src/http/modules/ngx_http_ssl_module.c:445:5: error: expected expression before ‘}’ token
     }
     ^
objs/Makefile:1170: recipe for target 'objs/src/http/modules/ngx_http_ssl_module.o' failed
make[1]: *** [objs/src/http/modules/ngx_http_ssl_module.o] Error 1
make[1]: *** Attente des tâches non terminées....
make[1]: Leaving directory '/opt/nginx-1.11.1'
Makefile:8: recipe for target 'build' failed

But still working with https://raw.githubusercontent.com/felixbuenemann/sslconfig/b8ebac6a337e8e4e373dfee76e7dfac3cc6c56e6/patches/nginx_1_9_15_http2_spdy.patch

felixbuenemann · 2016-06-10T14:15:33Z

@angristan Can't reproduce, works fine for me on 1.11.1 with only ssl module, ssl + http2, ssl + spdy and ssl + http2 + spdy. So you'd have to be a bit more specific.

Maybe the patch got applied incorrectly?

This script should work:

#!/bin/sh
set -e
git clone https://github.com/felixbuenemann/sslconfig.git
cd sslconfig
git checkout updated-nginx-1.9.15-spdy-patch
cd ..
git clone https://github.com/nginx/nginx.git
cd nginx
git checkout release-1.11.1
git apply ../sslconfig/patches/nginx_1_9_15_http2_spdy.patch
./auto/configure --prefix=/opt/nginx-1.11.1 \
  --with-http_ssl_module \
  --with-http_spdy_module \
  --with-http_v2_module
make

angristan · 2016-06-10T14:30:12Z

I'm using the patch this way : https://github.com/Angristan/nginx-autoinstall/blob/master/nginx-autoinstall.sh#L420

kn007 · 2016-06-10T19:00:05Z

@angristan Please using the laster version patch and try it again:
https://raw.githubusercontent.com/felixbuenemann/sslconfig/updated-nginx-1.9.15-spdy-patch/patches/nginx_1_9_15_http2_spdy.patch

See this:
#36 (comment)

angristan · 2016-06-10T19:11:04Z

@kn007 this one is working thx

RaeesBhatti · 2016-06-13T14:03:59Z

How do you test if it is actually using SPDY? Chrome doesn't support it anymore and Firefox says it's using HTTP/1.1

HansVanEijsden · 2016-06-13T14:05:30Z

@raeesiqbal NGINX Amplify gives insights: https://amplify.nginx.com/

felixbuenemann · 2016-06-13T14:08:21Z

You can test with Firefox by disabling http2 for the port in nginx or add another port with only spdy.

kn007 · 2016-06-30T12:18:56Z

@wangqiliang 请用英文。而且不要以链接方式来评论。你可以尝试使用Google Translate来附上你想评论的内容。

wangqiliang sending a feedback about when using IE9 to visit website with nginx 1.11.1&spdy patch, sometime will return ~4-bits anomaly contents and TTFB has reached 30000ms.

When not using spdy patch, it was quite normal.

ghost · 2016-06-30T13:20:37Z

@kn007 Sorry, it is my fault.
I am sending a feedback that when I access the port 80 which I set a spdy parameter in the ngx/1.11.1 with patch, there is unexpected ~4-bit contents output with a timeout (in Firefox 46, 30000ms) .
( I should correct a mistake. It was not a 30000-ms-TTFB but a ERR_TIME_OUT.)
when I am not using this patch, it returns 301 to HTTPS normally.

kn007 · 2016-06-30T13:26:03Z

@wangqiliang It's OK. Please excuse any mistakes for my english.

felixbuenemann · 2016-06-30T22:23:42Z

@wangqiliang You can only use spdy on port 80, if you have a load balancer like haproxy in front of your nginx which handles SSL termination and ALPN/NPN negotiation. If you use spdy or http2 on a port without ssl in NGINX, it will always use that protocol without negotiation.

So the simple answer is, don't set spdy on port 80, only use it on 443.

ghost · 2016-07-01T03:06:34Z

@felixbuenemann
It's so kind of you.
I will check for it later.

ghost · 2016-07-01T08:58:39Z

@felixbuenemann @kn007 From Wikepedia,

For use within HTTPS, SPDY needs the TLS extension Next Protocol Negotiation (NPN),[24] thus browser and server support depends on the HTTPS library.

So that is my fault. Thanks for your kindness!

kn007 · 2016-10-15T01:19:19Z

Do not work in 1.11.5.
Found a repo to fix it: https://github.com/cujanovic/nginx-http2-spdy-patch

fmunteanu · 2016-10-16T16:20:28Z

@kn007, that repo patch does not work with you add dynamic modules.

natnet00 · 2016-11-11T18:40:34Z

@felixbuenemann
Your patch works flawlessly up to version 1.11.4
But starting with nginx 1.11.5 the patch unfortunately gives errors... would be great if this could be fixed.

felixbuenemann · 2016-11-12T14:52:52Z

@natnet00 Have you tried the patch linked by kn007 above?

natnet00 · 2016-11-14T12:09:18Z

@felixbuenemann nope have not tried it yet

lenovouser · 2016-11-23T22:22:27Z

@felixbuenemann both also doesn't work with 1.11.6 :/

injust · 2016-11-23T23:00:28Z

@lenovouser The patch linked by kn007 indeed works for 1.11.6.

lenovouser · 2016-11-23T23:20:13Z

@injust nope, see cujanovic/nginx-http2-spdy-patch/issues/1 and

kn007 · 2016-11-23T23:20:18Z

@lenovouser I am upgrade Nginx to 1.11.6 withi this path last week, it's working.

kn007 · 2016-11-23T23:21:43Z

@lenovouser Do you using nginx-spdy-1.11.5+.patch, it's working well.

lenovouser · 2016-11-23T23:25:13Z

@kn007 weird, what NGINX source are you pulling from? I use https://nginx.org/download/nginx-1.11.6.tar.gz

kn007 · 2016-11-23T23:26:58Z

@lenovouser Yes, i am using http://nginx.org/download/nginx-1.11.6.tar.gz

lenovouser · 2016-11-23T23:30:44Z

Which one of the two files are you using? Because both are failing for me

kn007 · 2016-11-23T23:44:13Z

@lenovouser
wget -c http://nginx.org/download/nginx-1.11.6.tar.gz
tar zxvf nginx-1.11.6.tar.gz && cd nginx-1.11.6/
wget -c https://raw.githubusercontent.com/cujanovic/nginx-http2-spdy-patch/master/nginx-spdy-1.11.5%2B.patch
patch -p1 < nginx-spdy-1.11.5+.patch

kn007 · 2016-11-23T23:54:46Z

@lenovouser

lenovouser · 2016-11-24T00:16:42Z

So weird. Works on my local machine, but the Docker container I am trying to build it in fails. Well. Seems like I have to go find out the bug myself. Thanks for your help!

kn007 · 2016-11-24T00:43:19Z

@lenovouser You are welcome, good luck!

lenovouser · 2016-11-24T02:22:31Z

So, I found the issue. In case anyone reads this in the future and you're using Alpine Linux, just do a apk add patch. That will update the patch binary. For some reason the one installed by default is buggy 😞

felixbuenemann · 2016-11-24T18:15:20Z

@lenovouser It's probably using a busybox version of patch by default.

pankajslingwal · 2020-05-11T07:43:32Z

So, I found the issue. In case anyone reads this in the future and you're using Alpine Linux, just do a apk add patch. That will update the patch binary. For some reason the one installed by default is buggy 😞

dude, you saved my day :)

felixbuenemann referenced this pull request May 27, 2016

Added patch to make NGINX support HTTP/2 and SPDY simultaneously

e384008

See blog.cloudflare.com/open-sourcing-our-nginx-http-2-spdy-code

felixbuenemann force-pushed the updated-nginx-1.9.15-spdy-patch branch from 08b4112 to b8ebac6 Compare May 27, 2016 16:59

felixbuenemann added 4 commits May 30, 2016 00:30

Fix broken spdy directives in nginx 1.9.7 patch

bafe8a3

The various spdy configuration directives were still marked as deprecated by the http v2 module in the existing patch. Rhis change removes the deprecation so that the directives defined by the spdy module can be used again.

Fix http2 without spdy compilation in 1.9.7 patch

f571dd2

Tis fixes a compile error caused by the 1.9.7 NGINX SPDY patch due to an unmatched else branch, if the http_v2 module was enabled, but the spdy module was disabled.

Add version number to NGINX 1.9.7 SPDY patch

5ed0960

This should make it clearer which version this patch is targeting, now that there is another patch for 1.9.15.

felixbuenemann force-pushed the updated-nginx-1.9.15-spdy-patch branch from b8ebac6 to 7c23d27 Compare May 29, 2016 22:30

gslin mentioned this pull request Jun 27, 2016

nginx 1.11.1 (security release) and CloudFlare SPDY + HTTP2 patch oerdnj/deb.sury.org#394

Closed

vkrasnov force-pushed the master branch from ecea5e2 to 2c72c7f Compare October 27, 2016 20:55

This was referenced Nov 24, 2016

Update Nginx patches to version 1.11.6 #53

Open

Patch doesn't work for 1.11.6 cujanovic/nginx-http2-spdy-patch#1

Closed

Update NGINX SPDY patch for 1.9.15 #36

Are you sure you want to change the base?

Update NGINX SPDY patch for 1.9.15 #36

Conversation

felixbuenemann commented May 27, 2016 • edited Loading

felixbuenemann commented May 27, 2016

felixbuenemann commented May 27, 2016

jgrahamc commented May 27, 2016

felixbuenemann commented May 27, 2016

felixbuenemann commented May 27, 2016 • edited Loading

kn007 commented May 29, 2016 • edited Loading

frostieDE commented May 29, 2016

felixbuenemann commented May 29, 2016

felixbuenemann commented May 29, 2016

felixbuenemann commented May 29, 2016

felixbuenemann commented May 29, 2016

kn007 commented May 29, 2016

felixbuenemann commented May 29, 2016

xetorixik commented Jun 2, 2016

kn007 commented Jun 2, 2016

angristan commented Jun 10, 2016

felixbuenemann commented Jun 10, 2016

angristan commented Jun 10, 2016

kn007 commented Jun 10, 2016 • edited Loading

angristan commented Jun 10, 2016

RaeesBhatti commented Jun 13, 2016

HansVanEijsden commented Jun 13, 2016

felixbuenemann commented Jun 13, 2016

kn007 commented Jun 30, 2016

ghost commented Jun 30, 2016

kn007 commented Jun 30, 2016

felixbuenemann commented Jun 30, 2016 • edited Loading

ghost commented Jul 1, 2016

ghost commented Jul 1, 2016

kn007 commented Oct 15, 2016 • edited Loading

fmunteanu commented Oct 16, 2016 • edited Loading

natnet00 commented Nov 11, 2016

felixbuenemann commented Nov 12, 2016

natnet00 commented Nov 14, 2016

lenovouser commented Nov 23, 2016

injust commented Nov 23, 2016

lenovouser commented Nov 23, 2016

kn007 commented Nov 23, 2016

kn007 commented Nov 23, 2016 • edited Loading

lenovouser commented Nov 23, 2016

kn007 commented Nov 23, 2016

lenovouser commented Nov 23, 2016

kn007 commented Nov 23, 2016

kn007 commented Nov 23, 2016 • edited Loading

lenovouser commented Nov 24, 2016

kn007 commented Nov 24, 2016 • edited Loading

lenovouser commented Nov 24, 2016

felixbuenemann commented Nov 24, 2016

pankajslingwal commented May 11, 2020

felixbuenemann commented May 27, 2016 •

edited

Loading

felixbuenemann commented May 27, 2016 •

edited

Loading

kn007 commented May 29, 2016 •

edited

Loading

kn007 commented Jun 10, 2016 •

edited

Loading

felixbuenemann commented Jun 30, 2016 •

edited

Loading

kn007 commented Oct 15, 2016 •

edited

Loading

fmunteanu commented Oct 16, 2016 •

edited

Loading

kn007 commented Nov 23, 2016 •

edited

Loading

kn007 commented Nov 23, 2016 •

edited

Loading

kn007 commented Nov 24, 2016 •

edited

Loading