Configuration: Don't corrupt abstract socket names

The commit that added support for Unix sockets accepts abstract sockets
using '@' in the config, but we stored it internally using '\0'.

We want to support abstract sockets transparently to the user, so that
if the user configures unitd with '@', if we receive a query about the
current configuration, the user should see the same exact thing that was
configured.  So, this commit avoids the transformation in the internal
state file, storing user input pristine, and we only transform the '@'
in temporary strings.

This commit fixes another bug, where we try to connect to abstract
sockets with a trailing '\0' in their name due to calling twice
nxt_sockaddr_parse() on the same string.  By calling that function only
once with each copy of the string, we have fixed that bug.

The following code was responsible for this bug, which the second time
it was called, considered these sockets as file-backed (not abstract)
Unix socket, and so appended a '\0' to the socket name.

    $ grepc -tfd nxt_sockaddr_unix_parse . | grep -A10 @
        if (path[0] == '@') {
            path[0] = '\0';
            socklen--;
    #if !(NXT_LINUX)
            nxt_thread_log_error(NXT_LOG_ERR,
                                 "abstract unix domain sockets are not supported");
            return NULL;
    #endif
        }

        sa = nxt_sockaddr_alloc(mp, socklen, addr->length);

This bug was found thanks to some experiment about using 'const' for
some strings.

And here's some history:

-  9041d276fc6a ("nxt_sockaddr_parse() introducted.")

   This commit introduced support for abstract Unix sockets, but they
   only worked as "servers", and not as "listeners".  We corrupted the
   JSON config file, and stored a \u0000.  This also caused calling
   connect(2) with a bogus trailing null byte, which tried to connect to
   a different abstract socket.

-  d8e0768a5bae ("Fixed support for abstract Unix sockets.")

   This commit (partially) fixed support for abstract Unix sockets, so
   they they worked also as listeners.  We still corrupted the JSON
   config file, and stored a \u0000.  This caused calling connect(2)
   (and now bind(2) too) with a bogus trailing null byte.

-  e2aec6686a4d ("Storing abstract sockets with @ internally.")

   This commit fixed the problem by which we were corrupting the config
   file, but only for "listeners", not for "servers".  (It also fixes
   the issue about the terminating '\0'.)  We completely forgot about
   "servers", and other callers of the same function.

To reproduce the problem, I used the following config:

```json
{
	"listeners": {
		"*:80": {
			"pass": "routes/u"
		},
		"unix:@abstract": {
			"pass": "routes/a"
		}
	},

	"routes": {
		"u": [{
			"action": {
				"pass": "upstreams/u"
			}
		}],
		"a": [{
			"action": {
				"return": 302,
				"location": "/i/am/not/at/home/"
			}
		}]
	},

	"upstreams": {
		"u": {
			"servers": {
				"unix:@abstract": {}
			}
		}
	}
}
```

And then check the state file:

    $ sudo cat /opt/local/nginx/unit/master/var/lib/unit/conf.json \
    | jq . \
    | grep unix;
        "unix:@abstract": {
            "unix:\u0000abstract": {}

After this patch, the state file has a '@' as expected:

    $ sudo cat /opt/local/nginx/unit/unix/var/lib/unit/conf.json \
    | jq . \
    | grep unix;
        "unix:@abstract": {
            "unix:@abstract": {}

Regarding the trailing null byte, here are some tests:

    $ sudo strace -f -e 'bind,connect' /opt/local/nginx/unit/d8e0/sbin/unitd \
    |& grep abstract;
    [pid 22406] bind(10, {sa_family=AF_UNIX, sun_path=@"abstract\0"}, 12) = 0
    [pid 22410] connect(134, {sa_family=AF_UNIX, sun_path=@"abstract\0"}, 12) = 0
    ^C
    $ sudo killall unitd
    $ sudo strace -f -e 'bind,connect' /opt/local/nginx/unit/master/sbin/unitd \
    |& grep abstract;
    [pid 22449] bind(10, {sa_family=AF_UNIX, sun_path=@"abstract"}, 11) = 0
    [pid 22453] connect(134, {sa_family=AF_UNIX, sun_path=@"abstract\0"}, 12) = -1 ECONNREFUSED (Connection refused)
    ^C
    $ sudo killall unitd
    $ sudo strace -f -e 'bind,connect' /opt/local/nginx/unit/unix/sbin/unitd \
    |& grep abstract;
    [pid 22488] bind(10, {sa_family=AF_UNIX, sun_path=@"abstract"}, 11) = 0
    [pid 22492] connect(134, {sa_family=AF_UNIX, sun_path=@"abstract"}, 11) = 0
    ^C

Fixes: 9041d276fc6a ("nxt_sockaddr_parse() introducted.")
Fixes: d8e0768a5bae ("Fixed support for abstract Unix sockets.")
Fixes: e2aec6686a4d ("Storing abstract sockets with @ internally.")
Link: <https://github.com/nginx/unit/pull/1108>
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Cc: Liam Crilly <liam.crilly@nginx.com>
Cc: Zhidao Hong <z.hong@f5.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>

1 files changed, 11 insertions, 6 deletions

diff --git a/src/nxt_conf_validation.c b/src/nxt_conf_validation.c
index c843b265..bf18cd1a 100644
--- a/src/nxt_conf_validation.c
+++ b/src/nxt_conf_validation.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (C) Valentin V. Bartenev
  * Copyright (C) NGINX, Inc.
+ * Copyright 2024, Alejandro Colomar <alx@kernel.org>
  */
 
 #include <nxt_main.h>
@@ -1936,10 +1937,13 @@ static nxt_int_t
 nxt_conf_vldt_proxy(nxt_conf_validation_t *vldt, nxt_conf_value_t *value,
     void *data)
 {
-    nxt_str_t       name;
+    nxt_str_t       name, *ret;
     nxt_sockaddr_t  *sa;
 
-    nxt_conf_get_string(value, &name);
+    ret = nxt_conf_get_string_dup(value, vldt->pool, &name);
+    if (nxt_slow_path(ret == NULL)) {
+        return NXT_ERROR;
+    }
 
     if (nxt_str_start(&name, "http://", 7)) {
         name.length -= 7;
@@ -2913,13 +2917,11 @@ nxt_conf_vldt_object_iterator(nxt_conf_validation_t *vldt,
 
     for ( ;; ) {
         member = nxt_conf_next_object_member(value, &name, &index);
-
         if (member == NULL) {
             return NXT_OK;
         }
 
         ret = validator(vldt, &name, member);
-
         if (ret != NXT_OK) {
             return ret;
         }
@@ -3268,16 +3270,19 @@ nxt_conf_vldt_server(nxt_conf_validation_t *vldt, nxt_str_t *name,
     nxt_conf_value_t *value)
 {
     nxt_int_t       ret;
+    nxt_str_t       str;
     nxt_sockaddr_t  *sa;
 
     ret = nxt_conf_vldt_type(vldt, name, value, NXT_CONF_VLDT_OBJECT);
-
     if (ret != NXT_OK) {
         return ret;
     }
 
-    sa = nxt_sockaddr_parse(vldt->pool, name);
+    if (nxt_slow_path(nxt_str_dup(vldt->pool, &str, name) == NULL)) {
+        return NXT_ERROR;
+    }
 
+    sa = nxt_sockaddr_parse(vldt->pool, &str);
     if (sa == NULL) {
         return nxt_conf_vldt_error(vldt, "The \"%V\" is not valid "
                                    "server address.", name);