- Bugfixes in unixmessage/skaclient (short writes / fd leakage / DoS)v2.1.0.0

 - ABI change: unixmessage protocol header is now 6 bytes (was 8)
 - API change: skaclient_start(_async) now takes an "options" argument
 - version increase to 2.1.0.0

10 files changed, 130 insertions, 48 deletions

diff --git a/src/libunixonacid/skaclient-internal.h b/src/libunixonacid/skaclient-internal.h
index f0a9bfe..3c73d1f 100644
--- a/src/libunixonacid/skaclient-internal.h
+++ b/src/libunixonacid/skaclient-internal.h
@@ -4,12 +4,10 @@
 #define SKACLIENT_INTERNAL_H
 
 #include <skalibs/kolbak.h>
-#include <skalibs/skaclient.h>
 #include <skalibs/unixmessage.h>
+#include <skalibs/skaclient.h>
 
 extern int skaclient_init (skaclient_t *, int, char *, unsigned int, char *, unsigned int, char *, unsigned int, char *, unsigned int, kolbak_closure_t *, unsigned int, char const *, unsigned int) ;
-extern int skaclient_start_async_th (skaclient_t *, char *, unsigned int, char *, unsigned int, char *, unsigned int, char *, unsigned int, kolbak_closure_t *, unsigned int, char const *, char const *, unsigned int) ;
-extern int skaclient_startf_async_th (skaclient_t *, char *, unsigned int, char *, unsigned int, char *, unsigned int, char *, unsigned int, kolbak_closure_t *, unsigned int, char const *, char const *const *, char const *const *, uint32, char const *, unsigned int) ;
 extern int skaclient_start_cb (unixmessage_t const *, skaclient_cbdata_t *) ;
 
 #endif
diff --git a/src/libunixonacid/skaclient_start.c b/src/libunixonacid/skaclient_start.c
index a7e3e67..85021cb 100644
--- a/src/libunixonacid/skaclient_start.c
+++ b/src/libunixonacid/skaclient_start.c
@@ -1,6 +1,7 @@
 /* ISC license. */
 
 #include <errno.h>
+#include <skalibs/uint32.h>
 #include <skalibs/kolbak.h>
 #include <skalibs/skaclient.h>
 #include <skalibs/tai.h>
@@ -19,6 +20,7 @@ int skaclient_start (
   kolbak_closure_t *q,
   unsigned int qlen,
   char const *path,
+  uint32 options,
   char const *before,
   unsigned int beforelen,
   char const *after,
@@ -29,7 +31,7 @@ int skaclient_start (
   skaclient_cbdata_t blah ;
   unixmessage_t m ;
   register int r ;
-  if (!skaclient_start_async(a, bufss, bufsn, auxbufss, auxbufsn, bufas, bufan, auxbufas, auxbufan, q, qlen, path, before, beforelen, after, afterlen, &blah)) return 0 ;
+  if (!skaclient_start_async(a, bufss, bufsn, auxbufss, auxbufsn, bufas, bufan, auxbufas, auxbufan, q, qlen, path, options, before, beforelen, after, afterlen, &blah)) return 0 ;
   if (!skaclient_timed_flush(a, deadline, stamp))
   {
     register int e = errno ;
diff --git a/src/libunixonacid/skaclient_start_async.c b/src/libunixonacid/skaclient_start_async.c
index 4dbbf1c..79ea5df 100644
--- a/src/libunixonacid/skaclient_start_async.c
+++ b/src/libunixonacid/skaclient_start_async.c
@@ -1,6 +1,7 @@
 /* ISC license. */
 
 #include <errno.h>
+#include <skalibs/uint32.h>
 #include <skalibs/error.h>
 #include <skalibs/kolbak.h>
 #include <skalibs/skaclient.h>
@@ -21,6 +22,7 @@ int skaclient_start_async (
   kolbak_closure_t *q,
   unsigned int qlen,
   char const *path,
+  uint32 options,
   char const *before,
   unsigned int beforelen,
   char const *after,
@@ -38,14 +40,13 @@ int skaclient_start_async (
     return 0 ;
   }
   a->pid = 0 ;
-  a->options = 0 ;
+  a->options = options & ~SKACLIENT_OPTION_WAITPID ;
   if (!kolbak_enqueue(&a->kq, (unixmessage_handler_func_t_ref)&skaclient_start_cb, blah))
   {
     skaclient_end(a) ;
     return 0 ;
   }
-  blah->asyncin = &a->asyncin ;
-  blah->asyncout = &a->asyncout ;
+  blah->a = a ;
   blah->after = after ;
   blah->afterlen = afterlen ;
   return 1 ;
diff --git a/src/libunixonacid/skaclient_start_cb.c b/src/libunixonacid/skaclient_start_cb.c
index 4a82b9b..ff11a21 100644
--- a/src/libunixonacid/skaclient_start_cb.c
+++ b/src/libunixonacid/skaclient_start_cb.c
@@ -3,16 +3,24 @@
 #include <errno.h>
 #include <skalibs/bytestr.h>
 #include <skalibs/error.h>
-#include <skalibs/skaclient.h>
 #include <skalibs/unixmessage.h>
+#include <skalibs/skaclient.h>
 #include "skaclient-internal.h"
 
 int skaclient_start_cb (unixmessage_t const *m, skaclient_cbdata_t *blah)
 {
   if (m->len != blah->afterlen
    || byte_diff(m->s, m->len, blah->after)
-   || m->nfds != 1) return (errno = EPROTO, 0) ;
-  blah->asyncin->fd = m->fds[0] ;
-  blah->asyncout->fd = m->fds[0] ;
+   || m->nfds != 1)
+  {
+    unixmessage_drop(m) ;
+    return (errno = EPROTO, 0) ;
+  }
+  blah->a->asyncin.fd = m->fds[0] ;
+  blah->a->asyncout.fd = m->fds[0] ;
+  if (!(blah->a->options & SKACLIENT_OPTION_ASYNC_ACCEPT_FDS))
+    unixmessage_receiver_refuse_fds(&blah->a->asyncin) ;
+  if (!(blah->a->options & SKACLIENT_OPTION_SYNC_ACCEPT_FDS))
+    unixmessage_receiver_refuse_fds(&blah->a->syncin) ;
   return 1 ;
 }
diff --git a/src/libunixonacid/skaclient_startf_async.c b/src/libunixonacid/skaclient_startf_async.c
index eaeba6c..dd1688e 100644
--- a/src/libunixonacid/skaclient_startf_async.c
+++ b/src/libunixonacid/skaclient_startf_async.c
@@ -53,8 +53,7 @@ int skaclient_startf_async (
     skaclient_end(a) ;
     return 0 ;
   }
-  blah->asyncin = &a->asyncin ;
-  blah->asyncout = &a->asyncout ;
+  blah->a = a ;
   blah->after = after ;
   blah->afterlen = afterlen ;
   return 1 ;
diff --git a/src/libunixonacid/unixmessage_put.c b/src/libunixonacid/unixmessage_put.c
index f6db23b..908bc31 100644
--- a/src/libunixonacid/unixmessage_put.c
+++ b/src/libunixonacid/unixmessage_put.c
@@ -8,6 +8,7 @@
 #include <skalibs/bitarray.h>
 #include <skalibs/bytestr.h>
 #include <skalibs/diuint.h>
+#include <skalibs/error.h>
 #include <skalibs/stralloc.h>
 #include <skalibs/genalloc.h>
 #include <skalibs/siovec.h>
@@ -49,6 +50,8 @@ static inline int copyfds (char *s, int const *fds, unsigned int n, unsigned cha
 static int reserve_and_copy (unixmessage_sender_t *b, unsigned int len, int const *fds, unsigned int nfds, unsigned char const *bits)
 {
   diuint cur = { .left = b->data.len, .right = b->fds.len } ;
+  if (len > UNIXMESSAGE_MAXSIZE || nfds > UNIXMESSAGE_MAXFDS)
+    return (errno = EPROTO, 0) ;
   if (!genalloc_readyplus(diuint, &b->offsets, 1)
    || !genalloc_readyplus(int, &b->fds, nfds)
    || !stralloc_readyplus(&b->data, len))
diff --git a/src/libunixonacid/unixmessage_receive.c b/src/libunixonacid/unixmessage_receive.c
index 21491fa..5fa16c4 100644
--- a/src/libunixonacid/unixmessage_receive.c
+++ b/src/libunixonacid/unixmessage_receive.c
@@ -5,7 +5,8 @@
 #include <errno.h>
 #include <sys/socket.h>
 #include <sys/uio.h>
-#include <skalibs/uint.h>
+#include <skalibs/uint16.h>
+#include <skalibs/uint32.h>
 #include <skalibs/cbuffer.h>
 #include <skalibs/djbunix.h>
 #include <skalibs/error.h>
@@ -45,12 +46,12 @@ static int unixmessage_receiver_fill (unixmessage_receiver_t *b)
     .msg_iov = iov,
     .msg_iovlen = 2,
     .msg_flags = 0,
-    .msg_control = ancilbuf,
-    .msg_controllen = sizeof(ancilbuf)
+    .msg_control = b->fds_ok & 1 ? ancilbuf : 0,
+    .msg_controllen = b->fds_ok & 1 ? sizeof(ancilbuf) : 0
   } ;
   unsigned int auxlen ;
   int r = -1 ;
-  if (cbuffer_isfull(&b->mainb) || cbuffer_isfull(&b->auxb))
+  if (cbuffer_isfull(&b->mainb) || ((b->fds_ok & 1) && cbuffer_isfull(&b->auxb)))
     return (errno = ENOBUFS, -1) ;
   {
     siovec_t v[2] ;
@@ -62,6 +63,7 @@ static int unixmessage_receiver_fill (unixmessage_receiver_t *b)
     r = recvmsg(b->fd, &msghdr, awesomeflags) ;
     if (!r || (r < 0 && errno != EINTR)) return r ;
   }
+  if (b->fds_ok & 1)
   {
     struct cmsghdr *c = CMSG_FIRSTHDR(&msghdr) ;
     if (c)
@@ -69,16 +71,32 @@ static int unixmessage_receiver_fill (unixmessage_receiver_t *b)
       if (c->cmsg_level != SOL_SOCKET
        || c->cmsg_type != SCM_RIGHTS) return (errno = EPROTO, -1) ;
       auxlen = (unsigned int)(c->cmsg_len - (CMSG_DATA(c) - (unsigned char *)c)) ;
+      if (auxlen && !(b->fds_ok & 2))
+      {
+        register unsigned int i = auxlen/sizeof(int) ;
+        while (i--) fd_close(((int *)CMSG_DATA(c))[i]) ;
+        return (errno = EPROTO, -1) ;
+      }
 #ifndef SKALIBS_HASCMSGCLOEXEC
       {
         register unsigned int i = 0 ;
         for (; i < auxlen/sizeof(int) ; i++)
-          if (coe(((int *)CMSG_DATA(c))[i]) < 0) return -1 ;
+          if (coe(((int *)CMSG_DATA(c))[i]) < 0)
+          {
+            int e = errno ;
+            i++ ;
+            while (i--) fd_close(((int *)CMSG_DATA(c))[i]) ;
+            errno = e ;
+            return -1 ;
+          }
       }
 #endif
-      if (msghdr.msg_flags & MSG_CTRUNC) return (errno = EPROTO, -1) ;
-      if (cbuffer_put(&b->auxb, (char *)CMSG_DATA(c), auxlen) < auxlen)
+      if ((msghdr.msg_flags & MSG_CTRUNC) || cbuffer_put(&b->auxb, (char *)CMSG_DATA(c), auxlen) < auxlen)
+      {
+        register unsigned int i = auxlen/sizeof(int) ;
+        while (i--) fd_close(((int *)CMSG_DATA(c))[i]) ;
         return (errno = ENOBUFS, -1) ;
+      }
     }
   }
   cbuffer_WSEEK(&b->mainb, r) ;
@@ -89,21 +107,25 @@ int unixmessage_receive (unixmessage_receiver_t *b, unixmessage_t *m)
 {
   if (b->maindata.len == b->mainlen && b->auxdata.len == b->auxlen)
   {
-    char pack[sizeof(unsigned int) << 1] ;
-    if (cbuffer_len(&b->mainb) < sizeof(unsigned int) << 1)
+    char pack[6] ;
+    if (cbuffer_len(&b->mainb) < 6)
     {
       register int r = sanitize_read(unixmessage_receiver_fill(b)) ;
       if (r <= 0) return r ;
-      if (cbuffer_len(&b->mainb) < sizeof(unsigned int) << 1)
-        return (errno = EWOULDBLOCK, 0) ;
+      if (cbuffer_len(&b->mainb) < 6) return (errno = EWOULDBLOCK, 0) ;
     }
-    cbuffer_get(&b->mainb, pack, sizeof(unsigned int) << 1) ;
-    uint_unpack_big(pack, &b->mainlen) ;
-    uint_unpack_big(pack + sizeof(unsigned int), &b->auxlen) ;
+    cbuffer_get(&b->mainb, pack, 6) ;
+    uint32_unpack_big(pack, &b->mainlen) ;
+    if (b->fds_ok & 1) uint16_unpack_big(pack + 4, &b->auxlen) ;
+    else b->auxlen = 0 ;
     b->auxlen *= sizeof(int) ;
-    if (!stralloc_ready(&b->maindata, b->mainlen)) return -1 ;
+    if (b->mainlen > UNIXMESSAGE_MAXSIZE
+     || b->auxlen > ((b->fds_ok & 2) ? UNIXMESSAGE_MAXFDS * sizeof(int) : 0))
+      return (errno = EPROTO, -1) ;
+    if (!stralloc_ready(&b->maindata, b->mainlen)
+     || !stralloc_ready(&b->auxdata, b->auxlen))
+      return -1 ;
     b->maindata.len = 0 ;
-    if (!stralloc_ready(&b->auxdata, b->auxlen)) return -1 ;
     b->auxdata.len = 0 ;
   }
 
@@ -124,6 +146,6 @@ int unixmessage_receive (unixmessage_receiver_t *b, unixmessage_t *m)
   m->s = b->maindata.s ;
   m->len = b->maindata.len ;
   m->fds = (int *)b->auxdata.s ;
-  m->nfds = b->auxlen / sizeof(int) ;
+  m->nfds = b->auxdata.len / sizeof(int) ;
   return 1 ;
 }
diff --git a/src/libunixonacid/unixmessage_receiver_free.c b/src/libunixonacid/unixmessage_receiver_free.c
index 353797b..74654d0 100644
--- a/src/libunixonacid/unixmessage_receiver_free.c
+++ b/src/libunixonacid/unixmessage_receiver_free.c
@@ -1,12 +1,29 @@
 /* ISC license. */
 
 #include <skalibs/stralloc.h>
+#include <skalibs/djbunix.h>
 #include <skalibs/unixmessage.h>
 
 void unixmessage_receiver_free (unixmessage_receiver_t *b)
 {
+  register unsigned int h = b->maindata.len ;
   b->fd = -1 ;
   stralloc_free(&b->maindata) ;
+  h = h != b->mainlen || b->auxdata.len != b->auxlen || cbuffer_len(&b->auxb) ;
+  if (h)
+  {
+    register unsigned int n = b->auxdata.len / sizeof(int) ;
+    while (n--) fd_close(((int *)b->auxdata.s)[n]) ;
+  }
   stralloc_free(&b->auxdata) ;
-  b->mainlen = b->auxlen = 0 ;
+  if (h)
+  {
+    register unsigned int n = cbuffer_len(&b->auxb) / sizeof(int) ;
+    if (n)
+    {
+      int fds[n] ;
+      cbuffer_get(&b->auxb, (char *)fds, n * sizeof(int)) ;
+      while (n--) fd_close(fds[n]) ;
+    }
+  }
 }
diff --git a/src/libunixonacid/unixmessage_receiver_init.c b/src/libunixonacid/unixmessage_receiver_init.c
index 5f702cf..430ccac 100644
--- a/src/libunixonacid/unixmessage_receiver_init.c
+++ b/src/libunixonacid/unixmessage_receiver_init.c
@@ -13,5 +13,6 @@ int unixmessage_receiver_init (unixmessage_receiver_t *b, int fd, char *mainbuf,
   b->mainlen = b->auxlen = 0 ;
   b->maindata = stralloc_zero ;
   b->auxdata = stralloc_zero ;
+  b->fds_ok = 3 ;
   return 1 ;
 }
diff --git a/src/libunixonacid/unixmessage_sender_flush.c b/src/libunixonacid/unixmessage_sender_flush.c
index bd4c704..bea3585 100644
--- a/src/libunixonacid/unixmessage_sender_flush.c
+++ b/src/libunixonacid/unixmessage_sender_flush.c
@@ -6,9 +6,10 @@
 #include <sys/uio.h>
 #include <unistd.h>
 #include <errno.h>
-#include <skalibs/uint.h>
+#include <skalibs/uint16.h>
+#include <skalibs/uint32.h>
 #include <skalibs/diuint.h>
-#include <skalibs/stralloc.h>
+#include <skalibs/allreadwrite.h>
 #include <skalibs/genalloc.h>
 #include <skalibs/djbunix.h>
 #include <skalibs/unixmessage.h>
@@ -18,21 +19,51 @@
 #define MSG_NOSIGNAL 0
 #endif
 
+ /*
+    XXX: sendmsg/recvmsg is badly, badly specified.
+    XXX: We assume ancillary data is attached to the first byte.
+ */
+
 int unixmessage_sender_flush (unixmessage_sender_t *b)
 {
-  diuint last = { .left = b->data.len, .right = genalloc_len(int, &b->fds) } ;
+  diuint last = { .left = (uint32)b->data.len, .right = genalloc_len(int, &b->fds) } ;
   diuint *offsets = genalloc_s(diuint, &b->offsets) ;
   unsigned int n = genalloc_len(diuint, &b->offsets) ;
-  unsigned int oldhead = b->head ;
+  register int r ;
+
+  if (b->shorty) /* we had a short write, gotta send the remainder first */
+  {
+    diuint *next = b->head+1 < n ? offsets + b->head+1 : &last ;
+    unsigned int len = next->left - offsets[b->head].left ;
+    if (b->shorty <= len)
+      r = fd_write(b->fd, b->data.s + offsets[b->head].left + (len - b->shorty), b->shorty) ;
+    else
+    {
+      unsigned int nfds = next->right - offsets[b->head].right ;
+      char pack[6] ;
+      struct iovec v[2] =
+      {
+        { .iov_base = pack + 6 - (b->shorty - len), .iov_len = b->shorty - len },
+        { .iov_base = b->data.s + offsets[b->head].left, .iov_len = len }
+      } ;
+      uint32_pack_big(pack, (uint32)len) ;
+      uint16_pack_big(pack + 4, (uint16)nfds) ;
+      r = fd_writev(b->fd, v, 2) ;
+    }
+    if (r <= 0) return 0 ;
+    b->shorty -= r ;
+    if (b->shorty) return (errno = EWOULDBLOCK, 0) ;
+  }
+
   for (; b->head < n ; b->head++)
   {
     diuint *next = b->head+1 < n ? offsets + b->head+1 : &last ;
     unsigned int len = next->left - offsets[b->head].left ;
     unsigned int nfds = next->right - offsets[b->head].right ;
-    char pack[sizeof(unsigned int) << 1] ;
+    char pack[6] ;
     struct iovec v[2] =
     {
-      { .iov_base = pack, .iov_len = sizeof(unsigned int) << 1 },
+      { .iov_base = pack, .iov_len = 6 },
       { .iov_base = b->data.s + offsets[b->head].left, .iov_len = len }
     } ;
     char ancilbuf[CMSG_SPACE(nfds * sizeof(int))] ;
@@ -45,8 +76,8 @@ int unixmessage_sender_flush (unixmessage_sender_t *b)
       .msg_control = nfds ? ancilbuf : 0,
       .msg_controllen = nfds ? sizeof(ancilbuf) : 0
     } ;
-    uint_pack_big(pack, len) ;
-    uint_pack_big(pack + sizeof(unsigned int), nfds) ;
+    uint32_pack_big(pack, (uint32)len) ;
+    uint16_pack_big(pack + 4, (uint16)nfds) ;
     if (nfds)
     {
       struct cmsghdr *cp = CMSG_FIRSTHDR(&hdr) ;
@@ -60,14 +91,9 @@ int unixmessage_sender_flush (unixmessage_sender_t *b)
         ((int *)CMSG_DATA(cp))[i] = fd < 0 ? -(fd+1) : fd ;
       }
     }
-    for (;;)
-    {
-      register int r = sendmsg(b->fd, &hdr, MSG_NOSIGNAL) ;
-      if (r == -1 && errno == EINTR) continue ;
-      if (r < (int)(len + (sizeof(unsigned int) << 1)))
-        return -(int)(b->head-oldhead)-1 ;
-      break ;
-    }
+    do r = sendmsg(b->fd, &hdr, MSG_NOSIGNAL) ;
+    while (r < 0 && errno == EINTR) ;
+    if (r <= 0) return 0 ;
 #ifndef SKALIBS_HASANCILAUTOCLOSE
     if (nfds)
     {
@@ -79,10 +105,15 @@ int unixmessage_sender_flush (unixmessage_sender_t *b)
       }
     }
 #endif
+    if ((unsigned int)r < 6 + len)
+    {
+      b->shorty = 6 + len - r ;
+      return (errno = EWOULDBLOCK, 0) ;
+    }
   }
   b->data.len = 0 ;
   genalloc_setlen(int, &b->fds, 0) ;
   genalloc_setlen(diuint, &b->offsets, 0) ;
   b->head = 0 ;
-  return (int)(n - oldhead) ;
+  return 1 ;
 }