[PATCH] SSL: support ALPN (IETF's successor to NPN)

Valentin V. Bartenev vbart at nginx.com
Wed Nov 13 20:17:02 UTC 2013


On Monday 04 November 2013 14:27:44 Piotr Sikora wrote:
[..]
> # HG changeset patch
> # User Piotr Sikora <piotr at 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 <piotr at 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



More information about the nginx-devel mailing list