inet: getnameinfo fix serv for abstract socket [BZ #27634]

  Abstract sockets were not copied because they began with
\0. They can contain any character, so the full size is used
consistent with how they are created (man 7 unix).
---
 inet/Makefile          |  2 +-
 inet/getnameinfo.c     | 11 +++++++-
 inet/tst-getni-local.c | 63 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 inet/tst-getni-local.c
  

* Daniel Black:

> @@ -459,6 +459,15 @@ gni_serv_local (struct scratch_buffer *tmpbuf,
>  	       const struct sockaddr *sa, socklen_t addrlen,
>  	       char *serv, socklen_t servlen, int flags)
>  {
> +  if (((const struct sockaddr_un *) sa)->sun_path[0] == '\0')
> +    {
> +      /* Abstract socket */
> +      socklen_t len = addrlen - offsetof (struct sockaddr_un, sun_path);
> +      if (len > servlen)
> +	return EAI_OVERFLOW;
> +      memcpy (serv, ((const struct sockaddr_un *) sa)->sun_path, len);
> +      return 0;
> +    }
>    return checked_copy
>      (serv, servlen, ((const struct sockaddr_un *) sa)->sun_path);
>  }

Is this really useful as an interface?  The caller would still have to
know the struct sockaddr_un layout to figure out the length.  So it
could just struct sockaddr_un directly.

In general, getnameinfo itself is not very portable when applied to
AF_UNIX addresses.  Not all systems that have AF_UNIX also implement
it for getnameinfo.  How the path is mapped between the host and
service names also differs.  Therefore, I think applications should
look for AF_UNIX addresses directly.  There is also no string
transformation needed, unlike for the other address families.
  

Hey Florian,

On Sun, Mar 28, 2021 at 5:57 AM Florian Weimer <fw@deneb.enyo.de> wrote:

> * Daniel Black:
>
> > @@ -459,6 +459,15 @@ gni_serv_local (struct scratch_buffer *tmpbuf,
> >              const struct sockaddr *sa, socklen_t addrlen,
> >              char *serv, socklen_t servlen, int flags)
> >  {
> > +  if (((const struct sockaddr_un *) sa)->sun_path[0] == '\0')
> > +    {
> > +      /* Abstract socket */
> > +      socklen_t len = addrlen - offsetof (struct sockaddr_un, sun_path);
> > +      if (len > servlen)
> > +     return EAI_OVERFLOW;
> > +      memcpy (serv, ((const struct sockaddr_un *) sa)->sun_path, len);
> > +      return 0;
> > +    }
> >    return checked_copy
> >      (serv, servlen, ((const struct sockaddr_un *) sa)->sun_path);
> >  }
>
> Is this really useful as an interface?  The caller would still have to
> know the struct sockaddr_un layout to figure out the length.  So it
> could just struct sockaddr_un directly.
>

Partially, though with the prevalence of systemd socket activation
providing a
neat ListenStream=@abstract name, coders can make broad assumptions
that the only null character is the first.

A simplified version of my use case is attached, where I get a bunch of FDs
that I didn't create and want to log their nature.

Currently I get this:

$  systemd-socket-activate -l @abstract -l /tmp/ss.sock -l 3333
 ./resolve-by-fd
Listening on @abstract as 3.
Listening on /tmp/ss.sock as 4.
Listening on [::]:3333 as 5.
Communication attempt on fd 5.
Execing ./resolve-by-fd (./resolve-by-fd)
Using systemd activated socket :: port 3333
Using systemd activated socket localhost port /tmp/ss.sock
Using systemd activated socket localhost port @tmp/ss.sock

Notably not:
Using systemd activated socket localhost port @abstract

I can quite easily do without the width modifier in my code.

While in the meantime I've got a work around that looks like:

  sbuf[0] == '\0' ? addr.un.sun_path : sbuf

(which isn't terrible).


> In general, getnameinfo itself is not very portable when applied to
> AF_UNIX addresses.  Not all systems that have AF_UNIX also implement
> it for getnameinfo.


Point taken, and I'm looking at pretty much systemd as a Linux only
implementation.

  How the path is mapped between the host and
> service names also differs.


With the portability differences, in the case that a service name happens
to start with \0
(is there one?) there are two possible cases:

* sbuf gets populated with exactly what was in sun_path, I thought this was
pretty portable.
* if NI_MAXSERV wasn't used, you may end up with EAI_OVERFLOW

Therefore, I think applications should
> look for AF_UNIX addresses directly.  There is also no string
> transformation needed, unlike for the other address families.
>

I was just looking to make the most of a single interface.

Thanks for the review and detailed thoughts.
  

Is this really useful as an interface?  The caller would still have to
> know the struct sockaddr_un layout to figure out the length.  So it
> could just struct sockaddr_un directly.
>

I missed NI_MAXSERV is only 32 which is ok(maybe) for abstract sockets but
real path lengths, not so much.
  

* Daniel Black:

>> How the path is mapped between the host and
>> service names also differs.
>
> With the portability differences, in the case that a service name
> happens to start with \0 (is there one?) there are two possible
> cases:
>
> * sbuf gets populated with exactly what was in sun_path, I thought this was
> pretty portable.
> * if NI_MAXSERV wasn't used, you may end up with EAI_OVERFLOW

What I meant is that some implementations put the socket name into the
host name, not the service name.  The length restrictions you
discovered might be related to that.