#187 closed defect (fixed)
incorrect handling of bind option on wildcard listen addresses
Reported by: | Roman Odaisky | Owned by: | somebody |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | nginx-core | Version: | 1.3.x |
Keywords: | Cc: | ||
uname -a: | N/A | ||
nginx -V: |
nginx/1.2.1 — fix tested;
nginx/1.3.3 — code unchanged, issue presumed to persist. |
Description
Given the configuration
listen 10.2.2.2:80;
...
listen *:80;
or alternatively,
listen [2001:db8::42]:80;
...
listen [::]:80;
or any other configuration where a wildcard listen directive is present along with non-wildcard entries for the same port, nginx may fail to perform bind() correctly only once, depending on the order in which it encounters the directives. This is due to faulty sorting in ngx_http_optimize_servers according to ngx_http_cmp_conf_addrs criterion, which is not guaranteed to place the wildcard entry/entries at the end of the list as expected by ngx_http_init_listening. Eventually the problem causes EADDRINUSE upon startup due to multiple bind() calls. The following simple patch fixes the problem, presuming the wildcard field always equals 0 or 1.
-
src/http/ngx_http.c
a b 1608 1608 first = (ngx_http_conf_addr_t *) one; 1609 1609 second = (ngx_http_conf_addr_t *) two; 1610 1610 1611 if (first->opt.wildcard ) {1611 if (first->opt.wildcard != second->opt.wildcard) { 1612 1612 /* a wildcard address must be the last resort, shift it to the end */ 1613 return 1;1613 return (int)first->opt.wildcard - (int)second->opt.wildcard; 1614 1614 } 1615 1615 1616 1616 if (first->opt.bind && !second->opt.bind) {
--
HTH
Roman Odaisky
Change History (9)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
Most definitely. I encountered this in an actual setup. Minimal example to reproduce:
server { listen 127.0.0.1:58080; } server { listen 58080 sndbuf=42; }
Swap the entries in the configuration file and everything works correctly. Otherwise, the comparator is invoked exactly once, sees the bind option on the right side, fails to see the wildcard option on the right side, and erroneously returns -1, causing swapping of the entries.
The problem is triggered when addresses in listen directives differ and when socket options are present. I would guess this is rare with IPv4, but will be commonplace with IPv6 (due to the ipv6only=on socket option and multitude of available addresses).
The usefulness of IP-based virtual hosts is obvious, DNS flexibility.
comment:3 by , 12 years ago
Ok, thanks, the problem is clear. It's in fact due to bind option (in your above config - implicitly set by "sndbuf" parameter) was not ignored for wildcard addresses, and caused separate bind on wildcard address. I would suggest the following patch, which looks more logical for me:
# HG changeset patch # User Maxim Dounin <mdounin@mdounin.ru> # Date 1342179498 -14400 # Node ID c2f751648432a57fe0e9c88ea1ca313f5fa5c8d8 # Parent 5f74b296240eceebefa97009e9f1e1f62cf4e80a Ignore bind option on wildcard addresses (ticket #187). Without this patch config server { listen 127.0.0.1:58080; } server { listen 58080 sndbuf=42; } resulted in two binds, while the opposite order server { listen 58080 sndbuf=42; } server { listen 127.0.0.1:58080; } in only wildcard one. This happened due to the "bind" option set by the "sndbuf" parameter, which caused wrong order after sorting with ngx_http_cmp_conf_addrs(). diff --git a/src/http/ngx_http.c b/src/http/ngx_http.c --- a/src/http/ngx_http.c +++ b/src/http/ngx_http.c @@ -1618,7 +1618,7 @@ ngx_http_cmp_conf_addrs(const void *one, return -1; } - if (!first->opt.bind && second->opt.bind) { + if (!first->opt.bind && second->opt.bind && !second->opt.wildcard) { /* shift explicit bind()ed addresses to the start */ return 1; }
comment:4 by , 12 years ago
Priority: | major → minor |
---|---|
Status: | new → accepted |
Summary: | [patch] Failure to detect wildcard listening → incorrect handling of bind option on wildcard listen addresses |
comment:5 by , 12 years ago
This makes the comparator fail the strict antisymmetry property (cmp(a, b) = -1 <=> cmp(b, a) = 1), which has the potential to break the sorting in subtle ways (do you think it was easy to discover the problem in the first place?).
My patch, the only function of which is restoring antisymmetry on the wildcard comparison, causes the entries to be sorted as the code originally intended:
Non-wildcard, bind
Non-wildcard, non-bind
Wildcard, bind
Wildcard, non-bind
(in other words, ORDER BY wildcard ASC, bind DESC), which, if I understand the code correctly, is right. If another ordering is required, then the proper procedure is to specify the ordering, then implement the comparator to satisfy strict weak ordering requirements (the comparator got this right with the bind option but not with the wildcard option).
So I suggest either the original patch, or to handle the wildcard option like bind:
-
src/http/ngx_http.c
a b 1608 1608 first = (ngx_http_conf_addr_t *) one; 1609 1609 second = (ngx_http_conf_addr_t *) two; 1610 1610 1611 if (first->opt.wildcard ) {1611 if (first->opt.wildcard && !second->opt.wildcard) { 1612 1612 /* a wildcard address must be the last resort, shift it to the end */ 1613 1613 return 1; 1614 1614 } 1615 1615 1616 if (!first->opt.wildcard && second->opt.wildcard) { 1617 return -1; 1618 } 1619 1616 1620 if (first->opt.bind && !second->opt.bind) { 1617 1621 /* shift explicit bind()ed addresses to the start */ 1618 1622 return -1;
I can't reproduce this. Is this actually exploitable?
ngx_sort() does an "insertion sort", and even though the comparator may erroneously report that "first == *" where "*" is a wildcard address, "*" will eventually be checked via the "first" parameter, and moved appropriately.