[RFC] Allow disabling abstract unix socket paths NUL-padding

2024-03-05 Thread Tristan

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 
Date: Wed, 6 Mar 2024 06:00:14 +
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 
   See also: fd-hard-limit, maxconn
 
 unix-bind [ prefix  ] [ mode  ] [ user  ] [ uid  ]
-  [ group  ] [ gid  ]
+  [ group  ] [ 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  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 , , ,  and 
-  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  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 , , ,  and  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 [ ...]
   Removes environment variables specified in arguments. This can be useful to
@@ -5502,7 +5514,8 @@ bind / [, ...] [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  [:[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

Re: [PATCH 1/1] CI: skip scheduled builds on forks

2024-03-05 Thread Willy Tarreau
On Wed, Feb 21, 2024 at 05:05:39PM +0100, Ilya Shipitsin wrote:
> tracking bleeding edge changes with some rare platforms or modern
> compilers on scheduled basis is not what usually forks do. let's
> skip by default in forks, if some fork is interested, it might be
> enabled locally

Thank you! Indeed if we can save some resources it's always better.
I don't know the part of this job that currently works fine or fails
though. Now merged.

Willy



Re: [PATCH 1/1] CI: enable monthly build only test on netbsd-9.3

2024-03-05 Thread Willy Tarreau
On Mon, Feb 19, 2024 at 10:14:59PM +0100, Ilya Shipitsin wrote:
> it is interesting to try https://github.com/vmactions/netbsd-vm actions

Now merged, thanks!
Willy



Re: [PATCH 1/1] CI: run more smoke tests on config syntax to check memory related issues

2024-03-05 Thread Willy Tarreau
On Sat, Feb 17, 2024 at 08:42:28PM +0100, Ilya Shipitsin wrote:
> config syntax check seems add a value on testing code path not
> covered by VTest, also checks are very fast

Applied, thanks! We'll see if it triggers anything, that could
indeed be helpful.

Willy



Re: [PATCH] CLEANUP: assorted typo fixes in the code and comments

2024-03-05 Thread Willy Tarreau
On Thu, Feb 22, 2024 at 10:12:27AM +0100, Ilya Shipitsin wrote:
> This is 39th iteration of typo fixes

Now merged, thank you! I split it in two because the one on
resolvers and stick-tables directly affects the code (it renames
a function argument) and I want to make it easy to drop it in case
backports need/don't need it.

Thanks,
Willy