documentation for ngx_http_geo_module is inconsistent with the module's behavior when the server is accessed through a unix domain socket

[gdrbyKo1@…]

The documentation for ngx_http_geo_module states that "By default, the address is taken from the $remote_addr variable[...]", and that "If the value of a variable does not represent a valid IP address then the “255.255.255.255” address is used."

By this description, one could expect the following two blocks to behave exactly the same way:

geo $foo {
    255.255.255.255 0;
    default 1;
}

geo $remote_addr $bar {
    255.255.255.255 0;
    default 1;
}

but when the server is accessed through a unix domain socket, the variables $foo and $bar evaluate to two different values: 1 and 0, respectively.

Attached are a simple nginx config file and a python script which can be used to reproduce this issue.

[Maxim Dounin]

This seems to be a bug in the geo module, it doesn't know that there can be AF_UNIX addresses, and treats such addresses as AF_INET. The following patch should fix this:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1544114666 -10800
#      Thu Dec 06 19:44:26 2018 +0300
# Node ID 92140d36b98b9a38ccc2ac8a8e975792d76336f0
# Parent  2117637f64e981e0e14c3a4b0509252fefd8a78a
Geo: fixed handling of AF_UNIX client addresses (ticket #1684).

Previously, AF_UNIX client addresses were handled as AF_INET, leading
to unexpected results.

diff --git a/src/http/modules/ngx_http_geo_module.c b/src/http/modules/ngx_http_geo_module.c
--- a/src/http/modules/ngx_http_geo_module.c
+++ b/src/http/modules/ngx_http_geo_module.c
@@ -215,6 +215,13 @@ ngx_http_geo_cidr_variable(ngx_http_requ
         break;
 #endif
 
+#if (NGX_HAVE_UNIX_DOMAIN)
+    case AF_UNIX:
+        vv = (ngx_http_variable_value_t *)
+                  ngx_radix32tree_find(ctx->u.trees.tree, INADDR_NONE);
+        break;
+#endif
+
     default: /* AF_INET */
         sin = (struct sockaddr_in *) addr.sockaddr;
         inaddr = ntohl(sin->sin_addr.s_addr);
@@ -277,6 +284,12 @@ ngx_http_geo_range_variable(ngx_http_req
             break;
 #endif
 
+#if (NGX_HAVE_UNIX_DOMAIN)
+        case AF_UNIX:
+            inaddr = INADDR_NONE;
+            break;
+#endif
+
         default: /* AF_INET */
             sin = (struct sockaddr_in *) addr.sockaddr;
             inaddr = ntohl(sin->sin_addr.s_addr);

[gdrbyKo1@…]

Replying to mdounin:

This seems to be a bug in the geo module, it doesn't know that there can be AF_UNIX addresses, and treats such addresses as AF_INET. The following patch should fix this:

Thanks for your response. I just applied the patch, and with limited testing, it seems to fix the issue for the default cidr mode, but apparently in the ranges mode, there's a different issue, which makes it impossible to define rules which would match 255.255.255.255. For example, with a configuration like this:

geo $foo {
    ranges;
    default 1;
    255.255.255.255-255.255.255.255 0;
}

it exits with the message invalid range "255.255.255.255-255.255.255.255".
It's likely not related to this issue at all, but it just happens to get in the way when attempting to test the patch, because it's not possible to verify that connecting via a unix domain socket correctly makes the module treat the connection as coming from 255.255.255.255.
The module tests the start and end of the range for equality with INADDR_NONE, which is obsolete and the documentation (​https://www.gnu.org/software/libc/manual/html_node/Host-Address-Functions.html) says this:

It is obsolete because INADDR_NONE is a valid address (255.255.255.255)

Should this be tackled in another ticket?

[Maxim Dounin]

Replying to gdrbyKo1@…:

Thanks for your response. I just applied the patch, and with limited testing, it seems to fix the issue for the default cidr mode

Thanks for testing.

but apparently in the ranges mode, there's a different issue, which makes it impossible to define rules which would match 255.255.255.255.

This is because the 255.255.255.255 address cannot be a valid IP address except as a broadcast destination, and used to indicate errors when parsing addresses.

In the ranges mode, you can match 255.255.255.255 by using the default parameter. The particular problem can be tested as follows:

geo $rfoo {
    ranges;
    default 0;
    0.0.0.0-255.255.255.254 1;
}

geo $remote_addr $rbar {
    ranges;
    default 0;
    0.0.0.0-255.255.255.254 1;
}

Should this be tackled in another ticket?

No, thank you. I do not think there are practical reasons to allow 255.255.255.255 in ranges, and it is not needed to test the particular problem with unix sockets.

[gdrbyKo1@…]

Replying to mdounin:
That makes sense, thank you. Originally, I wanted to (ab?)use this module to create special locations accessible only locally, pretty much like it can be done with the ngx_http_access_module, except I wanted the returned status code to be 404 rather than 403, which, as far as I know, cannot be done with that module. So I ended up using ngx_http_geo_module like this:

geo $access_denied {
    default 1;
    255.255.255.255 0; # unix domain socket
    127.0.0.1 0;
}

location = /secret {
    if ($access_denied) {
        return 404;
    }
    return 200 "secret";
}

So by intuition, for the ranges mode, I came up with this:

geo $access_denied {
    ranges;
    default 1;
    255.255.255.255-255.255.255.255 0; # unix domain socket
    127.0.0.1-127.0.0.1 0;
}

Which resulted in the issue I described in my previous comment. But taking what you've said into consideration, I think the same could be achieved like this:

geo $access_denied {
    ranges;
    default 0; # unix domain socket
    127.0.0.1-127.0.0.1 0;
    0.0.0.0-126.255.255.255 1;
    127.0.0.2-255.255.255.254 1;
}

I think this issue is resolved now, thank you for your prompt replies and help :)

[Maxim Dounin <mdounin@…>]

In 7430:286ae954009d/nginx:

Geo: fixed handling of AF_UNIX client addresses (ticket #1684).

Previously, AF_UNIX client addresses were handled as AF_INET, leading
to unexpected results.

[Maxim Dounin]

Fix committed, thanks.