Hi Tristan,
On Wed, Mar 06, 2024 at 07:32:55AM +0000, Tristan wrote:
> 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).
Yep, thanks for this.
> 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.
Sure. There's one thing that bothers me in the way the option is configured,
which is that "unix-bind" takes options from the "bind" line, and here we're
adding a special case where this option is only valid for this line, and it
experiences special treatment. If we do it the "unix-bind" way, it means the
option will also be valid for each "bind" line. I understand the problem it
could cause when parsing it (i.e. was the global setting already set or not)
but that also enlights one of the difficulties related to interpreting the
configuration based on settings in the global section.
I'm still tempted to think that this should be seen as a different address
family since that's basically what we're doing, changing the way addresses
are used. Indeed, except for socket names that are exactly 107-chars long,
there's no way any address of the previous family can interact with one of
the new family.
We could just have "abns2" and declare that it's the second version of the
abns format and know that this one is interoperable with a number of other
components. It would even be easier to deal with for those who document
such components because their integration docs would just indicate that
the address has to be placed after "abns2@" in haproxy, instead of saying
"and don't forget to adjust this global setting in your config, unless you
already use abns anywhere else".
I had a look, and I think everything begins in str2sa_range() in this
block:
else if (ss.ss_family == AF_UNIX) {
...
}
By adding a new AF_CUST_ABNS2 family I think we can extend sock_unix
to properly deal with that. Indeed, right now it seems that your patch
does nothing for the receivers, bind() still uses sizeof(), and the
sock_unix_addrcmp() function continues to compare the full name, making
such sockets non-transferable across reloads. So it's unclear to me how
this can work in your test :-/
Having an explicit family would allow to manage all of this at once:
mostly everywhere we have a test for AF_UNIX we could add || AF_CUST_ABNS2
and make the special case of the address length there. I don't *think*
the socket transfer mechanism in sock.c needs to be touched because
retrieving the socket's address should give us its original length.
Thus I think it would be preferable to go that route rather than having
to deal with all exceptions everywhere based on a global-only option.
We can help on this, of course.
Thanks!
Willy
