Opened 2 years ago

Closed 2 years ago

#2400 closed enhancement (fixed)

listen on localhost can fail (sort of a docker and glibc issue, worth working around?)

Reported by: blowfishpro@… Owned by:
Priority: minor Milestone:
Component: nginx-core Version: 1.23.x
Keywords: docker localhost listen glibc getaddrinfo Cc:
uname -a: Linux be43d003a089 5.10.109-0-virt #1-Alpine SMP Mon, 28 Mar 2022 11:20:52 +0000 aarch64 GNU/Linux

(note, this is not actually alpine, it's debian from the docker library nginx image):

# cat /etc/os-release
PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"
NAME="Debian GNU/Linux"
VERSION_ID="11"
VERSION="11 (bullseye)"
VERSION_CODENAME=bullseye
ID=debian
nginx -V: nginx version: nginx/1.23.1
built by gcc 10.2.1 20210110 (Debian 10.2.1-6)
built with OpenSSL 1.1.1k 25 Mar 2021 (running with OpenSSL 1.1.1n 15 Mar 2022)
TLS SNI support enabled
configure arguments: --prefix=/etc/nginx --sbin-path=/usr/sbin/nginx --modules-path=/usr/lib/nginx/modules --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --pid-path=/var/run/nginx.pid --lock-path=/var/run/nginx.lock --http-client-body-temp-path=/var/cache/nginx/client_temp --http-proxy-temp-path=/var/cache/nginx/proxy_temp --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp --http-uwsgi-temp-path=/var/cache/nginx/uwsgi_temp --http-scgi-temp-path=/var/cache/nginx/scgi_temp --user=nginx --group=nginx --with-compat --with-file-aio --with-threads --with-http_addition_module --with-http_auth_request_module --with-http_dav_module --with-http_flv_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_mp4_module --with-http_random_index_module --with-http_realip_module --with-http_secure_link_module --with-http_slice_module --with-http_ssl_module --with-http_stub_status_module --with-http_sub_module --with-http_v2_module --with-mail --with-mail_ssl_module --with-stream --with-stream_realip_module --with-stream_ssl_module --with-stream_ssl_preread_module --with-cc-opt='-g -O2 -ffile-prefix-map=/data/builder/debuild/nginx-1.23.1/debian/debuild-base/nginx-1.23.1=. -fstack-protector-strong -Wformat -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fPIC' --with-ld-opt='-Wl,-z,relro -Wl,-z,now -Wl,--as-needed -pie'

Description

Steps to reproduce:

Run nginx with the following conditions:

  • Inside docker
  • On a distro that uses glibc (typically anything but alpine)
  • Do not enable ipv6 on the container (this is the default)
  • Have a listen directive that specifies localhost (e.g. listen localhost:80;

Observed Behavior:

Nginx fails with the following error message (this can be seen just by validating the config with -t as well):

nginx: [emerg] a duplicate listen 127.0.0.1:80 in /etc/nginx/conf.d/default.conf:2

Desired behavior:

We only attempt to bind to 127.0.0.1 once, and nginx starts up successfully

Explanation

Two longstanding issues from glibc and docker are at play here:

Immediate workarounds for users

  • Specify 127.0.0.1 instead of localhost - this makes supporting systems with different combinations of ipv4/ipv6 annoying
  • Don't limit to the loopback, instead use allow and deny directives to limit traffic from only that container network
  • Have some other process resolve localhost uniquely and generate the nginx config, inserting one or more listen directives as necessary

Possible workarounds that nginx could implement

  • De-duplicate the addresses returned by getaddrinfo
  • Rewrite "localhost" to null in the getaddrinfo call - in this case the unique loopback addresses appear to be returned correctly
    • I'm not sure if this could break other cases where /etc/hosts resolves localhost to something different
    • This requires passing a port, fortunately we have one

Additional notes

  • This is very easy to reproduce using the docker library nginx images. Just change the listen directive in /etc/nginx/conf.d/default.conf and it appears immediately.
  • Other container runtimes don't appear to have this issue - they assign a working ipv6 address to the loopback interface
  • Alpine linux does not use glibc and does not appear to have this issue - getaddrinfo returns the single unique ipv4 loopback address in this case
  • Neither of the issues in question seem to have gotten any traction for multiple years

Change History (5)

comment:1 by Maxim Dounin, 2 years ago

Type: defectenhancement

This looks like an issue in glibc to me. It shouldn't try to convert 127.0.0.1 to ::1, these are different addresses in different address families. And this wrong conversion causes issues with Docker images with IPv6 disabled which use standard /etc/hosts with both 127.0.0.1 and ::1 for localhost.

The issue seems to be in nss/nss_files/files-hosts.c, blame points to Ulrich Drepper's commit in 2006, and the only details available seems to be the following:

2006-11-19  Ulrich Drepper  <drepper@redhat.com>
* nss/nss_files/files-hosts.c (LINE_PARSER): Support IPv6-style
addresses for IPv4 queries if they can be mapped.

This wasn't important for nginx till listen directive was changed to use all IP addresses returned by getaddrinfo() in nginx 1.15.10 (4f9b72a229c1):

    *) Change: when using a hostname in the "listen" directive nginx now
       creates listening sockets for all addresses the hostname resolves to
       (previously, only the first address was used).

An obvious workaround on the OS configuration side would be to remove ::1 localhost from /etc/hosts on systems without IPv6. I tend to think it's not something Docker can or should do though, as the issue equally affects non-Docker systems without IPv6.

On nginx side, the most obvious workaround would be to use explicit addresses to listen on instead of using the localhost name. In particular, using just 127.0.0.1 might be a good option unless there are IPv6-only hosts.

As for proposed workarounds to be implemented in nginx, I'm certainly against any special handling of the localhost name.

Filtering out duplicate addresses from a particular name might be indeed a reasonable option though. I don't see any cases where it can break things, given that all IP addresses are from the particular name resolution. Further, it might help in other cases when a name maps to multiple identical IP addresses for some reason.

Below is a quick patch which implements filtering out duplicate addresses.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1665889924 -10800
#      Sun Oct 16 06:12:04 2022 +0300
# Node ID ea87002ec2c30891eef51083f4b0d7d5c910cd98
# Parent  81b4326daac70d6de70abbc3fe36d4f6e3da54a2
Filtering duplicate addresses in listen (ticket #2400).

Due to the glibc bug[1], getaddrinfo("localhost") with AI_ADDRCONFIG
on a typical host with glibc and without IPv6 returns two 127.0.0.1
addresses, and therefore "listen localhost:80;" used to result the
"duplicate ... address and port pair" after 4f9b72a229c1.

Fix is to explicitly filter out duplicate addresses returned during
resolution of a name.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=14969

diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -3963,7 +3963,7 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
 
     ngx_str_t              *value, size;
     ngx_url_t               u;
-    ngx_uint_t              n;
+    ngx_uint_t              n, i;
     ngx_http_listen_opt_t   lsopt;
 
     cscf->listen = 1;
@@ -4289,6 +4289,16 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
     }
 
     for (n = 0; n < u.naddrs; n++) {
+
+        for (i = 0; i < n; i++) {
+            if (ngx_cmp_sockaddr(u.addrs[n].sockaddr, u.addrs[n].socklen,
+                                 u.addrs[i].sockaddr, u.addrs[i].socklen, 0)
+                == NGX_OK)
+            {
+                goto next;
+            }
+        }
+
         lsopt.sockaddr = u.addrs[n].sockaddr;
         lsopt.socklen = u.addrs[n].socklen;
         lsopt.addr_text = u.addrs[n].name;
@@ -4297,6 +4307,9 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
         if (ngx_http_add_listen(cf, cscf, &lsopt) != NGX_OK) {
             return NGX_CONF_ERROR;
         }
+
+    next:
+        continue;
     }
 
     return NGX_CONF_OK;
diff --git a/src/mail/ngx_mail_core_module.c b/src/mail/ngx_mail_core_module.c
--- a/src/mail/ngx_mail_core_module.c
+++ b/src/mail/ngx_mail_core_module.c
@@ -308,7 +308,7 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
     ngx_str_t                  *value, size;
     ngx_url_t                   u;
     ngx_uint_t                  i, n, m;
-    ngx_mail_listen_t          *ls, *als;
+    ngx_mail_listen_t          *ls, *als, *nls;
     ngx_mail_module_t          *module;
     ngx_mail_core_main_conf_t  *cmcf;
 
@@ -333,7 +333,7 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
 
     cmcf = ngx_mail_conf_get_module_main_conf(cf, ngx_mail_core_module);
 
-    ls = ngx_array_push_n(&cmcf->listen, u.naddrs);
+    ls = ngx_array_push(&cmcf->listen);
     if (ls == NULL) {
         return NGX_CONF_ERROR;
     }
@@ -571,17 +571,37 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
     als = cmcf->listen.elts;
 
     for (n = 0; n < u.naddrs; n++) {
-        ls[n] = ls[0];
+
+        for (i = 0; i < n; i++) {
+            if (ngx_cmp_sockaddr(u.addrs[n].sockaddr, u.addrs[n].socklen,
+                                 u.addrs[i].sockaddr, u.addrs[i].socklen, 0)
+                == NGX_OK)
+            {
+                goto next;
+            }
+        }
 
-        ls[n].sockaddr = u.addrs[n].sockaddr;
-        ls[n].socklen = u.addrs[n].socklen;
-        ls[n].addr_text = u.addrs[n].name;
-        ls[n].wildcard = ngx_inet_wildcard(ls[n].sockaddr);
+        if (n != 0) {
+            nls = ngx_array_push(&cmcf->listen);
+            if (nls == NULL) {
+                return NGX_CONF_ERROR;
+            }
+
+            *nls = *ls;
 
-        for (i = 0; i < cmcf->listen.nelts - u.naddrs + n; i++) {
+        } else {
+            nls = ls;
+        }
+
+        nls->sockaddr = u.addrs[n].sockaddr;
+        nls->socklen = u.addrs[n].socklen;
+        nls->addr_text = u.addrs[n].name;
+        nls->wildcard = ngx_inet_wildcard(nls->sockaddr);
+
+        for (i = 0; i < cmcf->listen.nelts - 1; i++) {
 
             if (ngx_cmp_sockaddr(als[i].sockaddr, als[i].socklen,
-                                 ls[n].sockaddr, ls[n].socklen, 1)
+                                 nls->sockaddr, nls->socklen, 1)
                 != NGX_OK)
             {
                 continue;
@@ -589,9 +609,12 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
 
             ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
                                "duplicate \"%V\" address and port pair",
-                               &ls[n].addr_text);
+                               &nls->addr_text);
             return NGX_CONF_ERROR;
         }
+
+    next:
+        continue;
     }
 
     return NGX_CONF_OK;
diff --git a/src/stream/ngx_stream_core_module.c b/src/stream/ngx_stream_core_module.c
--- a/src/stream/ngx_stream_core_module.c
+++ b/src/stream/ngx_stream_core_module.c
@@ -578,7 +578,7 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
     ngx_str_t                    *value, size;
     ngx_url_t                     u;
     ngx_uint_t                    i, n, backlog;
-    ngx_stream_listen_t          *ls, *als;
+    ngx_stream_listen_t          *ls, *als, *nls;
     ngx_stream_core_main_conf_t  *cmcf;
 
     cscf->listen = 1;
@@ -602,7 +602,7 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
 
     cmcf = ngx_stream_conf_get_module_main_conf(cf, ngx_stream_core_module);
 
-    ls = ngx_array_push_n(&cmcf->listen, u.naddrs);
+    ls = ngx_array_push(&cmcf->listen);
     if (ls == NULL) {
         return NGX_CONF_ERROR;
     }
@@ -889,20 +889,40 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
     als = cmcf->listen.elts;
 
     for (n = 0; n < u.naddrs; n++) {
-        ls[n] = ls[0];
+
+        for (i = 0; i < n; i++) {
+            if (ngx_cmp_sockaddr(u.addrs[n].sockaddr, u.addrs[n].socklen,
+                                 u.addrs[i].sockaddr, u.addrs[i].socklen, 0)
+                == NGX_OK)
+            {
+                goto next;
+            }
+        }
 
-        ls[n].sockaddr = u.addrs[n].sockaddr;
-        ls[n].socklen = u.addrs[n].socklen;
-        ls[n].addr_text = u.addrs[n].name;
-        ls[n].wildcard = ngx_inet_wildcard(ls[n].sockaddr);
+        if (n != 0) {
+            nls = ngx_array_push(&cmcf->listen);
+            if (nls == NULL) {
+                return NGX_CONF_ERROR;
+            }
+
+            *nls = *ls;
 
-        for (i = 0; i < cmcf->listen.nelts - u.naddrs + n; i++) {
-            if (ls[n].type != als[i].type) {
+        } else {
+            nls = ls;
+        }
+
+        nls->sockaddr = u.addrs[n].sockaddr;
+        nls->socklen = u.addrs[n].socklen;
+        nls->addr_text = u.addrs[n].name;
+        nls->wildcard = ngx_inet_wildcard(nls->sockaddr);
+
+        for (i = 0; i < cmcf->listen.nelts - 1; i++) {
+            if (nls->type != als[i].type) {
                 continue;
             }
 
             if (ngx_cmp_sockaddr(als[i].sockaddr, als[i].socklen,
-                                 ls[n].sockaddr, ls[n].socklen, 1)
+                                 nls->sockaddr, nls->socklen, 1)
                 != NGX_OK)
             {
                 continue;
@@ -910,9 +930,12 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
 
             ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
                                "duplicate \"%V\" address and port pair",
-                               &ls[n].addr_text);
+                               &nls->addr_text);
             return NGX_CONF_ERROR;
         }
+
+    next:
+        continue;
     }
 
     return NGX_CONF_OK;

comment:2 by blowfishpro@…, 2 years ago

For what it's worth I do think Docker is doing something wrong here - either it should assign ::1 to the loopback interface or not write ::1 to /etc/hosts, doing one but not the other seems like misconfiguration. But you're also right that there are possibly a lot of ways that misconfiguration could arise.

If removing the duplicate addresses seems reasonable then I'm for that. But I would also understand if the nginx maintainers don't feel like this is worth fixing. At least now the cause and workarounds are documented.

comment:3 by Maxim Dounin, 2 years ago

The /etc/hosts file is usually something static provided by OS, and it is expected to work with different OS settings without requiring modifications. For example, /etc/hosts with both ::1 and 127.0.0.1 works flawlessly on FreeBSD with IPv6 both enabled and disabled. Similarly, it works fine on Alpine with musl libc.

While Docker probably can implement a workaround to conditionally remove ::1 if IPv6 is not enabled, I personally don't think it is something Docker should do. Rather, it is something to be addressed on the OS level, or, more specifically, in glibc.

Either way, I tend to think it would be good to implement a workaround in nginx, especially considering it is more or less trivial. When a name in the listen directive resolves into two identical addresses, it is clear enough that the intention is to listen on the address, and complaining about duplicate listen addresses is mostly meaningless.

comment:4 by Maxim Dounin <mdounin@…>, 2 years ago

In 8104:4cc2bfeff46c/nginx:

Filtering duplicate addresses in listen (ticket #2400).

Due to the glibc bug[1], getaddrinfo("localhost") with AI_ADDRCONFIG
on a typical host with glibc and without IPv6 returns two 127.0.0.1
addresses, and therefore "listen localhost:80;" used to result in
"duplicate ... address and port pair" after 4f9b72a229c1.

Fix is to explicitly filter out duplicate addresses returned during
resolution of a name.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=14969

comment:5 by Maxim Dounin, 2 years ago

Resolution: fixed
Status: newclosed

Workaround committed, thanks for prodding this.

Note: See TracTickets for help on using tickets.