On Thursday 14 November 2013 13:23:55 Piotr Sikora wrote:
> Hey Valentin,
> updated patch with fixed style and no SPDY check in case ALPN was
> selected attached.
[..]
Sorry for the long delay again. It would be nice to see ALPN sometime
around SPDY/3.1.
>
> SSL_select_next_proto() is available in OpenSSL-1.0.2 now, even when
> compiled with "no-nextprotoneg".
Yes, it looks ok now.
> # HG changeset patch
> # User Piotr Sikora <pi...@cloudflare.com>
> # Date 1384462707 28800
> # Thu Nov 14 12:58:27 2013 -0800
> # Node ID d848f32a9b677157ae2bddd3771509d73eb8e4d6
> # Parent dea321e5c0216efccbb23e84bbce7cf3e28f130c
> SSL: support ALPN (IETF's successor to NPN).
>
> Signed-off-by: Piotr Sikora <pi...@cloudflare.com>
>
> diff -r dea321e5c021 -r d848f32a9b67 src/http/modules/ngx_http_ssl_module.c
> --- a/src/http/modules/ngx_http_ssl_module.c Thu Oct 31 18:23:49 2013 +0400
> +++ b/src/http/modules/ngx_http_ssl_module.c Thu Nov 14 12:58:27 2013 -0800
> @@ -17,6 +17,17 @@ typedef ngx_int_t (*ngx_ssl_variable_han
> #define NGX_DEFAULT_CIPHERS "HIGH:!aNULL:!MD5"
> #define NGX_DEFAULT_ECDH_CURVE "prime256v1"
>
> +#if (defined TLSEXT_TYPE_application_layer_protocol_negotiation
> \
> + || defined TLSEXT_TYPE_next_proto_neg)
> +#define NGX_HTTP_NPN_ADVERTISE "\x08http/1.1"
> +#endif
These conditions look very excessive. I think we could leave this definition
without them.
[..]
> diff -r dea321e5c021 -r d848f32a9b67 src/http/ngx_http.c
> --- a/src/http/ngx_http.c Thu Oct 31 18:23:49 2013 +0400
> +++ b/src/http/ngx_http.c Thu Nov 14 12:58:27 2013 -0800
> @@ -1349,11 +1349,13 @@ ngx_http_add_address(ngx_conf_t *cf, ngx
> }
> }
>
> -#if (NGX_HTTP_SPDY && NGX_HTTP_SSL && !defined TLSEXT_TYPE_next_proto_neg)
> +#if (NGX_HTTP_SPDY && NGX_HTTP_SSL
> \
> + && !defined TLSEXT_TYPE_application_layer_protocol_negotiation
> \
> + && !defined TLSEXT_TYPE_next_proto_neg)
> if (lsopt->spdy && lsopt->ssl) {
> ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
> - "nginx was built without OpenSSL NPN support, "
> - "SPDY is not enabled for %s", lsopt->addr);
> + "nginx was built without OpenSSL ALPN and NPN "
> + "support, SPDY is not enabled for %s",
> lsopt->addr);
> }
> #endif
>
I've checked out some references and it looks like I was right.
See here for example:
http://www.esl-library.com/blog/2013/07/25/or-or-and-in-negative-sentences/
http://forum.wordreference.com/showthread.php?t=577487&p=3234156#post3234156
> diff -r dea321e5c021 -r d848f32a9b67 src/http/ngx_http_request.c
> --- a/src/http/ngx_http_request.c Thu Oct 31 18:23:49 2013 +0400
> +++ b/src/http/ngx_http_request.c Thu Nov 14 12:58:27 2013 -0800
> @@ -728,17 +728,33 @@ ngx_http_ssl_handshake_handler(ngx_conne
>
> c->ssl->no_wait_shutdown = 1;
>
> -#if (NGX_HTTP_SPDY && defined TLSEXT_TYPE_next_proto_neg)
> +#if (NGX_HTTP_SPDY
> \
> + && (defined TLSEXT_TYPE_application_layer_protocol_negotiation
> \
> + || defined TLSEXT_TYPE_next_proto_neg))
> {
> unsigned int len;
> const unsigned char *data;
> static const ngx_str_t spdy = ngx_string(NGX_SPDY_NPN_NEGOTIATED);
>
> - SSL_get0_next_proto_negotiated(c->ssl->connection, &data, &len);
> +#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
> + SSL_get0_alpn_selected(c->ssl->connection, &data, &len);
>
> if (len == spdy.len && ngx_strncmp(data, spdy.data, spdy.len) == 0) {
> ngx_http_spdy_init(c->read);
> return;
> +
> + } else if (len == 0)
> +#endif
> +
> + {
> +#ifdef TLSEXT_TYPE_next_proto_neg
> + SSL_get0_next_proto_negotiated(c->ssl->connection, &data, &len);
> +
> + if (len == spdy.len && ngx_strncmp(data, spdy.data, spdy.len) == 0) {
> + ngx_http_spdy_init(c->read);
> + return;
> + }
> +#endif
> }
> }
> #endif
It seems to me, this part looks cleaner if you will avoid duplication
of spdy check:
diff -r ea4be2ca702e src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c Thu Nov 14 12:58:27 2013 -0800
+++ b/src/http/ngx_http_request.c Tue Jan 28 20:12:44 2014 +0400
@@ -713,25 +713,21 @@ ngx_http_ssl_handshake_handler(ngx_conne
const unsigned char *data;
static const ngx_str_t spdy = ngx_string(NGX_SPDY_NPN_NEGOTIATED);
+ len = 0;
+
#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
SSL_get0_alpn_selected(c->ssl->connection, &data, &len);
+#endif
+
+#ifdef TLSEXT_TYPE_next_proto_neg
+ if (len == 0) {
+ SSL_get0_next_proto_negotiated(c->ssl->connection, &data, &len);
+ }
+#endif
if (len == spdy.len && ngx_strncmp(data, spdy.data, spdy.len) == 0) {
ngx_http_spdy_init(c->read);
return;
-
- } else if (len == 0)
-#endif
-
- {
-#ifdef TLSEXT_TYPE_next_proto_neg
- SSL_get0_next_proto_negotiated(c->ssl->connection, &data, &len);
-
- if (len == spdy.len && ngx_strncmp(data, spdy.data, spdy.len) == 0) {
- ngx_http_spdy_init(c->read);
- return;
- }
-#endif
}
}
#endif
> diff -r dea321e5c021 -r d848f32a9b67 src/http/ngx_http_spdy.h
> --- a/src/http/ngx_http_spdy.h Thu Oct 31 18:23:49 2013 +0400
> +++ b/src/http/ngx_http_spdy.h Thu Nov 14 12:58:27 2013 -0800
> @@ -17,7 +17,8 @@
>
> #define NGX_SPDY_VERSION 2
>
> -#ifdef TLSEXT_TYPE_next_proto_neg
> +#if (defined TLSEXT_TYPE_application_layer_protocol_negotiation
> \
> + || defined TLSEXT_TYPE_next_proto_neg)
> #define NGX_SPDY_NPN_ADVERTISE "\x06spdy/2"
> #define NGX_SPDY_NPN_NEGOTIATED "spdy/2"
> #endif
Same here. Let's just remove preprocessor condition at all.
wbr, Valentin V. Bartenev
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
