On Monday 04 November 2013 14:27:44 Piotr Sikora wrote:
[..]
> # HG changeset patch
> # User Piotr Sikora <pi...@cloudflare.com>
> # Date 1383560396 28800
> # Mon Nov 04 02:19:56 2013 -0800
> # Node ID 78d793c51d5aa0ba8eec48340de49bfc3d17c97d
> # Parent dea321e5c0216efccbb23e84bbce7cf3e28f130c
> SSL: support ALPN (IETF's successor to NPN).
I'm very unhappy with lots of #if(def)-s are introduced by the patch.
Is there something can be done with that?
>
> Signed-off-by: Piotr Sikora <pi...@cloudflare.com>
>
> diff -r dea321e5c021 -r 78d793c51d5a 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 Mon Nov 04 02:19:56 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)
Indentation problem, should be:
#if (defined TLSEXT_TYPE_application_layer_protocol_negotiation \
|| defined TLSEXT_TYPE_next_proto_neg)
Also, please note that we usually put "\" at column 79.
> +#define NGX_HTTP_NPN_ADVERTISE "\x08http/1.1"
> +#endif
> +
> +
> +#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
> +static int ngx_http_ssl_alpn_select(ngx_ssl_conn_t *ssl_conn,
> + const unsigned char **out, unsigned char *outlen,
> + const unsigned char *in, unsigned int inlen, void *arg);
> +#endif
>
> #ifdef TLSEXT_TYPE_next_proto_neg
> static int ngx_http_ssl_npn_advertised(ngx_ssl_conn_t *ssl_conn,
> @@ -274,10 +285,66 @@ static ngx_http_variable_t ngx_http_ssl
> static ngx_str_t ngx_http_ssl_sess_id_ctx = ngx_string("HTTP");
>
>
> +#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
> +
> +static int
> +ngx_http_ssl_alpn_select(ngx_ssl_conn_t *ssl_conn, const unsigned char **out,
> + unsigned char *outlen, const unsigned char *in, unsigned int inlen,
> + void *arg)
> +{
> + unsigned int srvlen;
> + unsigned char *srv;
> +#if (NGX_DEBUG)
> + unsigned int i;
> +#endif
> +#if (NGX_HTTP_SPDY)
> + ngx_http_connection_t *hc;
> +#endif
> +#if (NGX_HTTP_SPDY || NGX_DEBUG)
> + ngx_connection_t *c;
> +
> + c = ngx_ssl_get_connection(ssl_conn);
> +#endif
> +
> +#if (NGX_DEBUG)
> + for (i = 0; i < inlen; i += in[i] + 1) {
> + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, c->log, 0,
> + "SSL ALPN supported by client: %*s", in[i],
> &in[i + 1]);
Your email client broke the patch here.
> + }
> +#endif
> +
> +#if (NGX_HTTP_SPDY)
> + hc = c->data;
> +
> + if (hc->addr_conf->spdy) {
> + srv = (unsigned char *) NGX_SPDY_NPN_ADVERTISE
> NGX_HTTP_NPN_ADVERTISE;
> + srvlen = sizeof(NGX_SPDY_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1;
> +
> + } else
> +#endif
> + {
> + srv = (unsigned char *) NGX_HTTP_NPN_ADVERTISE;
> + srvlen = sizeof(NGX_HTTP_NPN_ADVERTISE) - 1;
> + }
> +
> + if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen,
> + in, inlen)
But the SSL_select_next_proto() function is missing if OpenSSL was built
with OPENSSL_NO_NEXTPROTONEG.
> + != OPENSSL_NPN_NEGOTIATED)
> + {
> + return SSL_TLSEXT_ERR_NOACK;
> + }
> +
> + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, c->log, 0,
> + "SSL ALPN selected: %*s", *outlen, *out);
> +
> + return SSL_TLSEXT_ERR_OK;
> +}
> +
> +#endif
> +
> +
> #ifdef TLSEXT_TYPE_next_proto_neg
>
> -#define NGX_HTTP_NPN_ADVERTISE "\x08http/1.1"
> -
> static int
> ngx_http_ssl_npn_advertised(ngx_ssl_conn_t *ssl_conn,
> const unsigned char **out, unsigned int *outlen, void *arg)
> @@ -542,6 +609,10 @@ ngx_http_ssl_merge_srv_conf(ngx_conf_t *
>
> #endif
>
> +#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
> + SSL_CTX_set_alpn_select_cb(conf->ssl.ctx, ngx_http_ssl_alpn_select,
> NULL);
> +#endif
> +
> #ifdef TLSEXT_TYPE_next_proto_neg
> SSL_CTX_set_next_protos_advertised_cb(conf->ssl.ctx,
> ngx_http_ssl_npn_advertised, NULL);
> diff -r dea321e5c021 -r 78d793c51d5a 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 Mon Nov 04 02:19:56 2013 -0800
> @@ -1349,11 +1349,12 @@ 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_next_proto_neg \
> + && !defined TLSEXT_TYPE_application_layer_protocol_negotiation)
I would prefer:
#if (NGX_HTTP_SPDY && NGX_HTTP_SSL \
&& !defined TLSEXT_TYPE_next_proto_neg \
&& !defined TLSEXT_TYPE_application_layer_protocol_negotiation)
> 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 "
Maybe I'm wrong since English isn't my native language, but should it be:
"nginx was built without OpenSSL ALPN or NPN " (s/and/or/)
?
> + "support, SPDY is not enabled for %s",
> lsopt->addr);
> }
> #endif
>
> diff -r dea321e5c021 -r 78d793c51d5a 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 Mon Nov 04 02:19:56 2013 -0800
> @@ -728,18 +728,31 @@ 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;
> }
> +#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
I'm not sure that we need to check NPN if from ALPN we know that some protocol
was selected and it's not spdy.
wbr, Valentin V. Bartenev
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
