Welcome! Log In Create A New Profile

Advanced

Re: [nginx] SSL: support ALPN (IETF's successor to NPN).

Ruslan Ermilov
January 30, 2014 04:56AM
On Wed, Jan 29, 2014 at 03:51:50PM +0000, Valentin Bartenev wrote:
> details: http://hg.nginx.org/nginx/rev/01e2a5bcdd8f
> branches:
> changeset: 5545:01e2a5bcdd8f
> user: Piotr Sikora <piotr@cloudflare.com>
> date: Tue Jan 28 15:33:49 2014 -0800
> description:
> SSL: support ALPN (IETF's successor to NPN).
>
> Signed-off-by: Piotr Sikora <piotr@cloudflare.com>

This change breaks compilation with clang on systems
where OpenSSL doesn't have ALPN support:

: cc -c -pipe -O -Wall -Wextra -Wpointer-arith -Wconditional-uninitialized -Wno-unused-parameter -Werror -g -I src/core -I src/event -I src/event/modules -I src/os/unix -I /opt/local/include -I objs -I src/http -I src/http/modules \
: -o objs/src/http/ngx_http_request.o \
: src/http/ngx_http_request.c
: src/http/ngx_http_request.c:728:44: error: variable 'data' may be uninitialized when used here [-Werror,-Wconditional-uninitialized]
: if (len == spdy.len && ngx_strncmp(data, spdy.data, spdy.len) == 0) {
: ^~~~
: src/core/ngx_string.h:53:56: note: expanded from macro 'ngx_strncmp'
: #define ngx_strncmp(s1, s2, n) strncmp((const char *) s1, (const char *) s2, n)
: ^
: src/http/ngx_http_request.c:713:38: note: initialize the variable 'data' to silence this warning
: const unsigned char *data;
: ^
: = NULL
: 1 error generated.
: make[1]: *** [objs/src/http/ngx_http_request.o] Error 1
: make: *** [build] Error 2

clang(1) mistakenly thinks that "len" can be non-zero, and under
this condition "data" may be uninitialized.

It's possible to rewrite the code such as that it doesn't trigger
the warning:

diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -713,11 +713,8 @@ 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) {
@@ -725,6 +722,10 @@ ngx_http_ssl_handshake_handler(ngx_conne
}
#endif

+#else /* TLSEXT_TYPE_next_proto_neg */
+ 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;

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

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

Valentin Bartenev 2935 January 29, 2014 10:52AM

Re: [nginx] SSL: support ALPN (IETF's successor to NPN).

Ruslan Ermilov 1075 January 30, 2014 04:56AM

Re: [nginx] SSL: support ALPN (IETF's successor to NPN).

Maxim Dounin 755 January 30, 2014 07:46AM

Re: [nginx] SSL: support ALPN (IETF's successor to NPN).

Valentin V. Bartenev 587 January 30, 2014 08:08AM

Re: [nginx] SSL: support ALPN (IETF's successor to NPN).

Maxim Dounin 651 January 30, 2014 09:06AM

Re: [nginx] SSL: support ALPN (IETF's successor to NPN).

Piotr Sikora 483 January 30, 2014 03:16PM

Re: [nginx] SSL: support ALPN (IETF's successor to NPN).

Ruslan Ermilov 552 January 31, 2014 05:18AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 247
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready