Hello,
Earlier, I ran into the issue outlined in
https://github.com/haproxy/haproxy/issues/977
Namely, that HAProxy will NUL-pad (as suffix) abstract unix socket
paths, causing interop issues with other programs.
I raised a new feature request to make this behaviour tunable
(https://github.com/haproxy/haproxy/issues/2479).
Since I do quite want this, I also came up with a patch for it, attached
to this email.
Marked as RFC for now because there hasn't really been a broad
discussion on the topic yet.
Thanks,
Tristan
From 24962900bd6e537cdc2fd293359d2092c93ff683 Mon Sep 17 00:00:00 2001
From: Tristan <tris...@mangadex.org>
Date: Wed, 6 Mar 2024 06:00:14 +0000
Subject: [PATCH] MINOR: socket: Allow disabling abstract unix socket paths
NUL-padding
When an abstract unix socket is bound by HAProxy, NUL bytes are
appended at the end of its path until sun_path is filled (for a
total of 108 characters).
Here we add an option to pass only the non-NUL
length of that path to connect/bind calls, such that
the effective path of the socket's name is as humanly written.
This is achieved with the unix-bind tightsocklen { on | off }
global directive option, and mimicks socat's unix-tightsocklen
in its semantics.
Fixes GH issues #977 and #2479
This probably doesn't need backporting.
---
doc/configuration.txt | 34 ++++++++++++++++-------
include/haproxy/global-t.h | 1 +
include/haproxy/tools.h | 9 ++++++
reg-tests/server/abns_tightsocklen.vtc | 38 ++++++++++++++++++++++++++
src/cfgparse-global.c | 23 +++++++++++++++-
src/sock_unix.c | 2 +-
src/tools.c | 2 +-
7 files changed, 96 insertions(+), 13 deletions(-)
create mode 100644 reg-tests/server/abns_tightsocklen.vtc
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 787103fe0..5d755741f 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2668,18 +2668,30 @@ ulimit-n <number>
See also: fd-hard-limit, maxconn
unix-bind [ prefix <prefix> ] [ mode <mode> ] [ user <user> ] [ uid <uid> ]
- [ group <group> ] [ gid <gid> ]
+ [ group <group> ] [ gid <gid> ] [ tightsocklen { on | off } ]
Fixes common settings to UNIX listening sockets declared in "bind" statements.
This is mainly used to simplify declaration of those UNIX sockets and reduce
the risk of errors, since those settings are most commonly required but are
- also process-specific. The <prefix> setting can be used to force all socket
- path to be relative to that directory. This might be needed to access another
- component's chroot. Note that those paths are resolved before HAProxy chroots
- itself, so they are absolute. The <mode>, <user>, <uid>, <group> and <gid>
- all have the same meaning as their homonyms used by the "bind" statement. If
- both are specified, the "bind" statement has priority, meaning that the
- "unix-bind" settings may be seen as process-wide default settings.
+ also process-specific.
+
+ The <prefix> setting can be used to force all socket path to be relative to
+ that directory. This might be needed to access another component's chroot. Note
+ that those paths are resolved before HAProxy chroots itself, so they are absolute.
+
+ The <mode>, <user>, <uid>, <group> and <gid> all have the same meaning as their
+ homonyms used by the "bind" statement. If both are specified, the "bind" statement
+ has priority, meaning that the "unix-bind" settings may be seen as process-wide
+ default settings.
+
+ The tightsocklen argument controls the behaviour of the socket path when binding
+ abstract unix domain sockets. The address of a socket (sun_path) is by convention
+ 108-characters long on UNIX, but it is not mandatory, and shorter paths usually
+ bind successfully. When set to 'off' (the default), trailing NUL bytes are added
+ to the connect()/bind() syscalls to satisfy this convention. When set to 'on',
+ nothing is appended to the path even if the result is shorter than 108 characters,
+ and HAProxy attempts to bind it as-is. This setting effectively behaves like
+ socat's unix-tightsocklen option.
unsetenv [<name> ...]
Removes environment variables specified in arguments. This can be useful to
@@ -5502,7 +5514,8 @@ bind /<path> [, ...] [param*]
sun_path length is used for the address length. Some other programs
such as socat use the string length only by default. Pass the option
",unix-tightsocklen=0" to any abstract socket definition in socat to
- make it compatible with HAProxy's.
+ make it compatible with HAProxy's, or enable the same behaviour in
+ HAProxy via the unix-bind directive in the global section.
See also : "source", "option forwardfor", "unix-bind" and the PROXY protocol
documentation, and section 5 about bind options.
@@ -10805,7 +10818,8 @@ server <name> <address>[:[port]] [param*]
sun_path length is used for the address length. Some other programs
such as socat use the string length only by default. Pass the option
",unix-tightsocklen=0" to any abstract socket definition in socat to
- make it compatible with HAProxy's.
+ make it compatible with HAProxy's, or enable the same behaviour in
+ HAProxy via the unix-bind directive in the global section.
See also: "default-server", "http-send-name-header" and section 5 about
server options
diff --git a/include/haproxy/global-t.h b/include/haproxy/global-t.h
index 25536df14..c7494cd9e 100644
--- a/include/haproxy/global-t.h
+++ b/include/haproxy/global-t.h
@@ -207,6 +207,7 @@ struct global {
gid_t gid; /* -1 to leave unchanged */
mode_t mode; /* 0 to leave unchanged */
} ux;
+ uint tightsocklen; /* when enabled, do not include trailing NULs in connect()/bind() to abstract unix sockets */
} unix_bind;
struct proxy *cli_fe; /* the frontend holding the stats settings */
int numa_cpu_mapping;
diff --git a/include/haproxy/tools.h b/include/haproxy/tools.h
index 3726f6353..71ed59804 100644
--- a/include/haproxy/tools.h
+++ b/include/haproxy/tools.h
@@ -41,6 +41,7 @@
#include <import/eb32tree.h>
#include <haproxy/api.h>
#include <haproxy/chunk.h>
+#include <haproxy/global.h>
#include <haproxy/intops.h>
#include <haproxy/namespace-t.h>
#include <haproxy/protocol-t.h>
@@ -722,12 +723,20 @@ static inline int get_host_port(const struct sockaddr_storage *addr)
/* returns address len for <addr>'s family, 0 for unknown families */
static inline int get_addr_len(const struct sockaddr_storage *addr)
{
+ char *sun_path;
+
switch (addr->ss_family) {
case AF_INET:
return sizeof(struct sockaddr_in);
case AF_INET6:
return sizeof(struct sockaddr_in6);
case AF_UNIX:
+ sun_path = ((struct sockaddr_un *)addr)->sun_path;
+ if (global.unix_bind.tightsocklen && sun_path[0] == '\0') {
+ // skip the first byte from strlen, as it is NUL for abns (but count it)
+ return offsetof(struct sockaddr_un, sun_path) + 1 + strlen(sun_path+1);
+ }
+
return sizeof(struct sockaddr_un);
}
return 0;
diff --git a/reg-tests/server/abns_tightsocklen.vtc b/reg-tests/server/abns_tightsocklen.vtc
new file mode 100644
index 000000000..cdad92a91
--- /dev/null
+++ b/reg-tests/server/abns_tightsocklen.vtc
@@ -0,0 +1,38 @@
+varnishtest "Abstract unix socket + tightsocklen"
+feature ignore_unknown_macro
+feature cmd "command -v curl"
+
+# abns@ sockets are not available on freebsd
+#EXCLUDE_TARGETS=freebsd,osx,generic
+#REGTEST_TYPE=devel
+
+haproxy h1 -W -S -conf {
+ global
+ stats socket "${tmpdir}/h1/stats" level admin expose-fd listeners
+ unix-bind tightsocklen on
+
+ defaults
+ mode http
+ log global
+ option httplog
+ timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+ timeout client "${HAPROXY_TEST_TIMEOUT-5s}"
+ timeout server "${HAPROXY_TEST_TIMEOUT-5s}"
+
+ listen testme
+ bind "fd@${testme}"
+ server f2 abns@hap-f2
+
+ frontend f2
+ bind abns@hap-f2
+ http-request return status 200 content-type text/plain string "ok"
+} -start
+
+shell {
+ curl -sfS --abstract-unix-socket hap-f2 "http://host/"; | grep "ok"
+}
+
+client c1 -connect ${h1_testme_sock} {
+ txreq -url "/"
+ rxresp
+} -run
diff --git a/src/cfgparse-global.c b/src/cfgparse-global.c
index f31e7a05a..0ed4e0f17 100644
--- a/src/cfgparse-global.c
+++ b/src/cfgparse-global.c
@@ -968,7 +968,28 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
continue;
}
- ha_alert("parsing [%s:%d] : '%s' only supports the 'prefix', 'mode', 'uid', 'gid', 'user' and 'group' options.\n",
+ if (strcmp(args[cur_arg], "tightsocklen") == 0) {
+ if (!*args[cur_arg + 1]) {
+ ha_alert("parsing [%s:%d] : '%s' : missing argument, use either 'on' or 'off'.\n",
+ file, linenum, args[cur_arg]);
+ return ERR_ALERT | ERR_FATAL;
+ }
+
+ if (strcmp(args[cur_arg + 1], "on") == 0)
+ global.unix_bind.tightsocklen = 1;
+ else if (strcmp(args[cur_arg + 1], "off") == 0)
+ global.unix_bind.tightsocklen = 0;
+ else {
+ ha_alert("parsing [%s:%d] : '%s' : invalid argument '%s', use either 'on' or 'off'.\n",
+ file, linenum, args[cur_arg], args[cur_arg+1]);
+ return ERR_ALERT | ERR_FATAL;
+ }
+
+ cur_arg += 2;
+ continue;
+ }
+
+ ha_alert("parsing [%s:%d] : '%s' only supports the 'prefix', 'mode', 'uid', 'gid', 'user', 'group', 'tightsocklen' options.\n",
file, linenum, args[0]);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;
diff --git a/src/sock_unix.c b/src/sock_unix.c
index ef749a53a..0fab3a438 100644
--- a/src/sock_unix.c
+++ b/src/sock_unix.c
@@ -294,7 +294,7 @@ int sock_unix_bind_receiver(struct receiver *rx, char **errmsg)
goto bind_close_return;
}
- if (!ext && bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
+ if (!ext && bind(fd, (struct sockaddr *)&addr, get_addr_len((struct sockaddr_storage *) &addr)) < 0) {
/* note that bind() creates the socket <tempname> on the file system */
if (errno == EADDRINUSE) {
/* the old process might still own it, let's retry */
diff --git a/src/tools.c b/src/tools.c
index e1ba2411e..715799d91 100644
--- a/src/tools.c
+++ b/src/tools.c
@@ -918,7 +918,7 @@ struct sockaddr_storage *str2ip2(const char *str, struct sockaddr_storage *sa, i
* - 'abns@' -> force address to belong to the abstract namespace (Linux
* only). These sockets are just like Unix sockets but without
* the need for an underlying file system. The address is a
- * string. Technically it's like a Unix socket with a zero in
+ * string. Technically it's like a Unix socket with a NUL in
* the first byte of the address.
* - "fd@" => an integer must follow, and is a file descriptor number.
*
--
2.44.0
