Datagram disassociation should be done via AF_UNSPEC

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Datagram disassociation should be done via AF_UNSPEC

astian
(Sorry if you are getting this message duplicated, I tried sending it to
tech@ before but it seems to get stuck somewhere along the way.)

I found this while looking at netcat and it seemed odd:

        if (family == AF_UNIX && uflag) {
                if (connect(s, NULL, 0) == -1)
                        err(1, "connect");
        }

POSIX 2008 [0] says:

  For SOCK_DGRAM sockets, [...] If the sa_family member of address is
  AF_UNSPEC, the socket's peer address shall be reset.

It also says:

  Issue 7
  [...]
    - Austin Group Interpretation 1003.1-2001 #188 is applied, changing
      the method used to reset a peer address for a datagram socket.
  [...]

Indeed, in POSIX 2004 [1] it said:

  For SOCK_DGRAM sockets, [...] If address is a null address for the
  protocol, the socket's peer address shall be reset.

Which is similar to connect(2):

  Datagram sockets may dissolve the association by connecting to an
  invalid address, such as a null address.

Now, I don't know if "null address for the protocol" was at any point in
time supposed to be interpreted as NULL-pointer/zero-size as opposed to
zeroed structure for example (notice that AF_UNSPEC is 0) or whether
such a thing ever worked in OpenBSD, but it certainly doesn't now and
hasn't for a while: it fails in sys_connect -> sockargs when checking
for the sockaddr size.

OpenBSD does support disassociating via a zeroed sockaddr, but the
syscall fails and it shouldn't; in fact, it supports disassociating by
using *any* sort of address as long as it's consistent in terms of size
because the disassociation (sodisconnect) happens before even looking at
the user-supplied sockaddr.  I believe this should to be cleaned up but
it is a change in syscall semantics (according to CVS this code,
soconnect, mostly comes directly from NetBSD from 23 years ago).

I did not look at what other BSD do, but below is a possible minimal
patch that aims to retain most of the current behaviour, which might or
might not be TRTTD.  It should apply to "current".

Cheers.

0: https://pubs.opengroup.org/onlinepubs/9699919799/functions/connect.html
1: https://pubs.opengroup.org/onlinepubs/009695399/functions/connect.html

--- sys/kern/uipc_socket.c 2019-08-09 17:27:45 +0000
+++ sys/kern/uipc_socket.c 2019-08-10 17:41:07 +0000
@@ -66,6 +66,8 @@ void soreaper(void *);
 void soput(void *);
 int somove(struct socket *, int);
 
+int nam2sa(struct mbuf *, struct sockaddr **);
+
 void filt_sordetach(struct knote *kn);
 int filt_soread(struct knote *kn, long hint);
 void filt_sowdetach(struct knote *kn);
@@ -344,7 +346,8 @@ soaccept(struct socket *so, struct mbuf
 int
 soconnect(struct socket *so, struct mbuf *nam)
 {
- int error;
+ struct sockaddr *sa;
+ int error = 0;
 
  soassertlocked(so);
 
@@ -353,14 +356,16 @@ soconnect(struct socket *so, struct mbuf
  /*
  * If protocol is connection-based, can only connect once.
  * Otherwise, if connected, try to disconnect first.
- * This allows user to disconnect by connecting to, e.g.,
- * a null address.
+ * This allows the user to disconnect by connecting to an invalid
+ * address, although only a "zero" address (AF_UNSPEC) will not
+ * result in error.
  */
  if (so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING) &&
     ((so->so_proto->pr_flags & PR_CONNREQUIRED) ||
     (error = sodisconnect(so))))
  error = EISCONN;
- else
+ else if (!(so->so_type == SOCK_DGRAM && nam2sa(nam, &sa) == 0 &&
+    sa->sa_family == AF_UNSPEC))
  error = (*so->so_proto->pr_usrreq)(so, PRU_CONNECT,
     NULL, nam, NULL, curproc);
  return (error);
@@ -1965,6 +1970,19 @@ sohasoutofband(struct socket *so)
 }
 
 int
+nam2sa(struct mbuf *nam, struct sockaddr **soa)
+{
+ struct sockaddr *sa = mtod(nam, struct sockaddr *);
+
+ if (nam->m_len < offsetof(struct sockaddr, sa_data))
+ return EINVAL;
+ if (sa->sa_len != nam->m_len)
+ return EINVAL;
+ *soa = sa;
+ return 0;
+}
+
+int
 soo_kqfilter(struct file *fp, struct knote *kn)
 {
  struct socket *so = kn->kn_fp->f_data;
--- lib/libc/sys/connect.2 2019-08-10 17:34:42 +0000
+++ lib/libc/sys/connect.2 2019-08-10 17:38:22 +0000
@@ -73,7 +73,7 @@ only once; datagram sockets may use
 .Fn connect
 multiple times to change their association.
 Datagram sockets may dissolve the association
-by connecting to an invalid address, such as a null address.
+by connecting to an address of family AF_UNSPEC.
 .Pp
 If the socket is in non-blocking mode and the connection cannot be
 completed immediately, or if it is interrupted by a signal,
--- usr.bin/nc/netcat.c 2019-08-09 17:30:13 +0000
+++ usr.bin/nc/netcat.c 2019-08-10 17:47:16 +0000
@@ -629,7 +629,9 @@ main(int argc, char *argv[])
  tls_free(tls_cctx);
  }
  if (family == AF_UNIX && uflag) {
- if (connect(s, NULL, 0) == -1)
+ /* AF_UNSPEC == 0 */
+ if (connect(s, &(struct sockaddr){0},
+    sizeof(struct sockaddr)) == -1)
  err(1, "connect");
  }