From 4df09797df69a67e61d4e3f499e8647c0ab2139d Mon Sep 17 00:00:00 2001
From: Laurent Bercot <ska-skaware@skarnet.org>
Date: Mon, 16 Mar 2020 13:15:29 +0000
Subject:  Add -d to s6-sudod; prepare for 2.9.2.0

---
 NEWS                                        |   6 ++
 doc/index.html                              |   2 +-
 doc/s6-sudod.html                           |  15 +++-
 doc/upgrade.html                            |   6 ++
 package/info                                |   2 +-
 src/conn-tools/s6-accessrules-cdb-from-fs.c |   2 +-
 src/conn-tools/s6-ioconnect.c               |   2 +-
 src/conn-tools/s6-ipcclient.c               |   1 +
 src/conn-tools/s6-sudo.c                    |   1 +
 src/conn-tools/s6-sudoc.c                   |   2 +
 src/conn-tools/s6-sudod.c                   | 131 ++++++++++++++++------------
 11 files changed, 106 insertions(+), 64 deletions(-)

diff --git a/NEWS b/NEWS
index 0cf76c4..c5926fc 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,11 @@
 Changelog for s6.
 
+In 2.9.2.0
+----------
+
+ - New -d option to s6-sudod.
+
+
 In 2.9.1.0
 ----------
 
diff --git a/doc/index.html b/doc/index.html
index dadb82d..540e646 100644
--- a/doc/index.html
+++ b/doc/index.html
@@ -103,7 +103,7 @@ certain binaries that spawn scripts interpreted with
 <h3> Download </h3>
 
 <ul>
- <li> The current released version of s6 is <a href="s6-2.9.1.0.tar.gz">2.9.1.0</a>. </li>
+ <li> The current released version of s6 is <a href="s6-2.9.2.0.tar.gz">2.9.2.0</a>. </li>
  <li> Alternatively, you can checkout a copy of the
 <a href="//git.skarnet.org/cgi-bin/cgit.cgi/s6/">s6
 git repository</a>:
diff --git a/doc/s6-sudod.html b/doc/s6-sudod.html
index 171de48..53ec2f7 100644
--- a/doc/s6-sudod.html
+++ b/doc/s6-sudod.html
@@ -27,7 +27,7 @@ program over a Unix socket, then forks another program.
 <h2> Interface </h2>
 
 <pre>
-     s6-sudod [ -0 ] [ -1 ] [ -2 ] [ -t <em>timeout</em> ] [ <em>sargv...</em> ]
+     s6-sudod [ -0 ] [ -1 ] [ -2 ] [ -d ] [ -t <em>timeout</em> ] [ <em>sargv...</em> ]
 </pre>
 
 <ul>
@@ -66,6 +66,11 @@ run with its stdout pointing to <tt>/dev/null</tt> instead. </li>
  <li> <tt>-2</tt>&nbsp;: do not inherit stderr from s6-sudoc. The child will be
 run with its stderr being a copy of s6-sudod's stderr instead. (This is useful
 to still log the child's error messages without sending them to the client.) </li>
+ <li> <tt>-d</tt>&nbsp;: detach. The child will keep running until it naturally
+exits, even if the client disconnects. Setting this option also enforces
+<tt>-0</tt>, <tt>-1</tt> and <tt>-2</tt>. Bear in mind that this option
+relinquishes a lot of control over the child, and administrators should make sure
+it is appropriately short-lived. </li>
  <li> <tt>-t&nbsp;<em>timeout</em></tt>&nbsp;: if s6-sudod has not
 received all the needed data from the client after <em>timeout</em>
 milliseconds, it will exit without spawning a child. By default, <em>timeout</em>
@@ -150,7 +155,8 @@ transmitted to <em>sargv</em>.
 <h2> Notes </h2>
 
 <ul>
- <li> If s6-sudoc is killed, or exits after <em>timeoutrun</em> milliseconds,
+ <li> If the <tt>-d</tt> option to s6-sudod has not been given, and
+s6-sudoc is killed (or exits after <em>timeoutrun</em> milliseconds)
 while the server program is still running, s6-sudod will send a SIGTERM and a
 SIGCONT to its child, then exit 1. However, sending a SIGTERM to the child
 does not guarantee that it will die; and
@@ -164,6 +170,11 @@ handled transparently by the s6-sudoc + s6-sudod mechanism. The mechanism
 was designed to allow programs to gain privileges in specific situations:
 short-lived, simple, noninteractive processes. It was not designed to emulate
 the full suid functionality and will not go out of its way to do so. </li>
+ <li> Administrators should also make sure that it's not a problem if
+s6-sudod's child keeps running after the s6-sudoc client exits, if they
+have given the <tt>-d</tt> option to s6-sudod. In particular, they should
+study what happens if another connection to the same service occurs while
+an instance is still running. </li>
  <li> <em>sargv</em> may be empty. In that case, the client is in complete
 control of the command line executed as <em>serveruser</em>. This setup is
 permitted by s6-sudod, but it is very dangerous, and extreme attention should
diff --git a/doc/upgrade.html b/doc/upgrade.html
index bbf96f1..f7413f5 100644
--- a/doc/upgrade.html
+++ b/doc/upgrade.html
@@ -18,6 +18,12 @@
 
 <h1> What has changed in s6 </h1>
 
+<h2> in 2.9.2.0 </h2>
+
+<ul>
+ <li> New <tt>-d</tt> option to <a href="s6-sudod.html">s6-sudod</a>. </li>
+</ul>
+
 <h2> in 2.9.1.0 </h2>
 
 <ul>
diff --git a/package/info b/package/info
index 89d6b16..d9fd33f 100644
--- a/package/info
+++ b/package/info
@@ -1,4 +1,4 @@
 package=s6
-version=2.9.1.0
+version=2.9.2.0
 category=admin
 package_macro_name=S6
diff --git a/src/conn-tools/s6-accessrules-cdb-from-fs.c b/src/conn-tools/s6-accessrules-cdb-from-fs.c
index 8ad0b40..dd935bf 100644
--- a/src/conn-tools/s6-accessrules-cdb-from-fs.c
+++ b/src/conn-tools/s6-accessrules-cdb-from-fs.c
@@ -77,7 +77,7 @@ static void doit (struct cdb_make *c, stralloc *sa, size_t start)
     if (tmp.len > tmpbase + 4103)
     {
       cleanup() ;
-      strerr_diefu2sys(100, sa->s, "too big") ;
+      strerr_dief2sys(100, sa->s, " too big") ;
     }
     envlen = tmp.len - tmpbase - 3 ;
     tmp.len = tmpbase ;
diff --git a/src/conn-tools/s6-ioconnect.c b/src/conn-tools/s6-ioconnect.c
index eebb752..b838162 100644
--- a/src/conn-tools/s6-ioconnect.c
+++ b/src/conn-tools/s6-ioconnect.c
@@ -4,6 +4,7 @@
 #include <sys/socket.h>
 #include <errno.h>
 #include <signal.h>
+
 #include <skalibs/types.h>
 #include <skalibs/allreadwrite.h>
 #include <skalibs/sgetopt.h>
@@ -19,7 +20,6 @@
 #define USAGE "s6-ioconnect [ -t timeout ] [ -r fdr ] [ -w fdw ] [ -0 ] [ -1 ] [ -6 ] [ -7 ]"
 #define dieusage() strerr_dieusage(100, USAGE)
 
-
 typedef struct ioblah_s ioblah_t, *ioblah_t_ref ;
 struct ioblah_s
 {
diff --git a/src/conn-tools/s6-ipcclient.c b/src/conn-tools/s6-ipcclient.c
index cbbb54d..42521de 100644
--- a/src/conn-tools/s6-ipcclient.c
+++ b/src/conn-tools/s6-ipcclient.c
@@ -1,6 +1,7 @@
 /* ISC license. */
 
 #include <string.h>
+
 #include <skalibs/sgetopt.h>
 #include <skalibs/strerr2.h>
 #include <skalibs/env.h>
diff --git a/src/conn-tools/s6-sudo.c b/src/conn-tools/s6-sudo.c
index d4e3b03..2408927 100644
--- a/src/conn-tools/s6-sudo.c
+++ b/src/conn-tools/s6-sudo.c
@@ -4,6 +4,7 @@
 #include <skalibs/sgetopt.h>
 #include <skalibs/strerr2.h>
 #include <skalibs/djbunix.h>
+
 #include <s6/config.h>
 
 #define USAGE "s6-sudo [ -q | -Q | -v ] [ -p bindpath ] [ -l localname ] [ -e ] [ -t timeout ] [ -T timeoutrun ] path [ args... ]"
diff --git a/src/conn-tools/s6-sudoc.c b/src/conn-tools/s6-sudoc.c
index c8cb747..fe6af5d 100644
--- a/src/conn-tools/s6-sudoc.c
+++ b/src/conn-tools/s6-sudoc.c
@@ -7,6 +7,7 @@
 #include <signal.h>
 #include <sys/uio.h>
 #include <sys/wait.h>
+
 #include <skalibs/types.h>
 #include <skalibs/sgetopt.h>
 #include <skalibs/buffer.h>
@@ -17,6 +18,7 @@
 #include <skalibs/env.h>
 #include <skalibs/unix-timed.h>
 #include <skalibs/unixmessage.h>
+
 #include "s6-sudo.h"
 
 #define USAGE "s6-sudoc [ -e ] [ -t timeoutconn ] [ -T timeoutrun ] [ args... ]"
diff --git a/src/conn-tools/s6-sudod.c b/src/conn-tools/s6-sudod.c
index 8b2b9aa..8b92532 100644
--- a/src/conn-tools/s6-sudod.c
+++ b/src/conn-tools/s6-sudod.c
@@ -6,6 +6,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <signal.h>
+
 #include <skalibs/types.h>
 #include <skalibs/allreadwrite.h>
 #include <skalibs/sgetopt.h>
@@ -20,41 +21,77 @@
 #include <skalibs/djbunix.h>
 #include <skalibs/unix-timed.h>
 #include <skalibs/unixmessage.h>
+
 #include "s6-sudo.h"
 
-#define USAGE "s6-sudod [ -0 ] [ -1 ] [ -2 ] [ -t timeout ] args..."
+#define USAGE "s6-sudod [ -0 ] [ -1 ] [ -2 ] [ -d ] [ -t timeout ] args..."
 #define dieusage() strerr_dieusage(100, USAGE)
 #define dienomem() strerr_diefu1sys(111, "stralloc_catb")
 
+static int handle_signals (pid_t pid, int *wstat)
+{
+  int done = 0 ;
+  for (;;)
+  {
+    int sig = selfpipe_read() ;
+    switch (sig)
+    {
+      case -1 : strerr_diefu1sys(111, "read from selfpipe") ;
+      case 0 : return done ;
+      case SIGCHLD :
+      {
+        int w ;
+        pid_t r = wait_pid_nohang(pid, &w) ;
+        if ((r < 0) && (errno != ECHILD))
+          strerr_diefu1sys(111, "wait_pid_nohang") ;
+        else if (r > 0)
+        {
+          done = 1 ;
+          *wstat = w ;
+        }
+        break ;
+      }
+      default :
+        strerr_dief1sys(101, "internal inconsistency, please submit a bug-report") ;
+    }
+  }
+}
+
 int main (int argc, char const *const *argv, char const *const *envp)
 {
-  subgetopt_t l = SUBGETOPT_ZERO ;
+  iopause_fd x[2] = { { .events = IOPAUSE_READ }, { .fd = 0, .events = 0, .revents = 0 } } ;
   unixmessage_t m ;
-  unsigned int nullfds = 0, t = 0 ;
+  unsigned int nullfds = 0 ;
   pid_t pid ;
+  int wstat ;
   size_t envc = env_len(envp) ;
   uint32_t cargc, cenvc, carglen, cenvlen ;
-  int spfd ;
   tain_t deadline = TAIN_INFINITE_RELATIVE ;
   PROG = "s6-sudod" ;
-  for (;;)
+
   {
-    int opt = subgetopt_r(argc, argv, "012t:", &l) ;
-    if (opt < 0) break ;
-    switch (opt)
+    subgetopt_t l = SUBGETOPT_ZERO ;
+    unsigned int t = 0 ;
+    for (;;)
     {
-      case '0' : nullfds |= 1 ; break ;
-      case '1' : nullfds |= 2 ; break ;
-      case '2' : nullfds |= 4 ; break ;
-      case 't' : if (!uint0_scan(l.arg, &t)) dieusage() ; break ;
-      default : dieusage() ;
+      int opt = subgetopt_r(argc, argv, "012dt:", &l) ;
+      if (opt < 0) break ;
+      switch (opt)
+      {
+        case '0' : nullfds |= 1 ; break ;
+        case '1' : nullfds |= 2 ; break ;
+        case '2' : nullfds |= 4 ; break ;
+        case 'd' : nullfds |= 15 ; break ;
+        case 't' : if (!uint0_scan(l.arg, &t)) dieusage() ; break ;
+        default : dieusage() ;
+      }
     }
+    argc -= l.ind ; argv += l.ind ;
+    if (t) tain_from_millisecs(&deadline, t) ;
   }
-  argc -= l.ind ; argv += l.ind ;
-  if (t) tain_from_millisecs(&deadline, t) ;
+
   if ((ndelay_on(0) < 0) || (ndelay_on(1) < 0))
     strerr_diefu1sys(111, "make socket non-blocking") ;
-
   tain_now_set_stopwatch_g() ;
   tain_add_g(&deadline, &deadline) ;
   buffer_putnoflush(buffer_1small, S6_SUDO_BANNERB, S6_SUDO_BANNERB_LEN) ;
@@ -136,8 +173,8 @@ int main (int argc, char const *const *argv, char const *const *envp)
       if ((j < envc) && !tenvp[j][len+1]) tenvp[j] = var ;
     }
 
-    spfd = selfpipe_init() ;
-    if (spfd < 0) strerr_diefu1sys(111, "selfpipe_init") ;
+    x[0].fd = selfpipe_init() ;
+    if (x[0].fd < 0) strerr_diefu1sys(111, "selfpipe_init") ;
     if (selfpipe_trap(SIGCHLD) < 0) strerr_diefu1sys(111, "trap SIGCHLD") ;
     if (pipe(p) < 0) strerr_diefu1sys(111, "pipe") ;
     if (coe(p[1]) < 0) strerr_diefu1sys(111, "coe pipe") ;
@@ -185,49 +222,27 @@ int main (int argc, char const *const *argv, char const *const *envp)
   if (!buffer_timed_flush_g(buffer_1small, &deadline))
     strerr_diefu1sys(111, "send confirmation to client") ;
 
+  for (;;)
   {
-    iopause_fd x[2] = { { .fd = 0, .events = 0 }, { .fd = spfd, .events = IOPAUSE_READ } } ;
-    int cont = 1 ;
-    while (cont)
+    if (iopause_g(x, 1 + !x[1].revents, 0) < 0) strerr_diefu1sys(111, "iopause") ;
+    if (x[0].revents && handle_signals(pid, &wstat)) break ;
+    if (x[1].revents && !(nullfds & 8))
     {
-      if (iopause_g(x, 2, 0) < 0) strerr_diefu1sys(111, "iopause") ;
-      if (x[1].revents)
-      {
-        for (;;)
-        {
-          int c = selfpipe_read() ;
-          if (c < 0) strerr_diefu1sys(111, "read from selfpipe") ;
-          else if (!c) break ;
-          else if (c == SIGCHLD)
-          {
-            int wstat ;
-            c = wait_pid_nohang(pid, &wstat) ;
-            if ((c < 0) && (errno != ECHILD))
-              strerr_diefu1sys(111, "wait_pid_nohang") ;
-            else if (c > 0)
-            {
-              char pack[UINT_PACK] ;
-              uint_pack_big(pack, (unsigned int)wstat) ;
-              buffer_putnoflush(buffer_1small, pack, UINT_PACK) ;
-              cont = 0 ;
-            }
-          }
-          else
-            strerr_dief1sys(101, "internal inconsistency, please submit a bug-report") ;
-        }
-      }
-      if (x[0].revents && cont)
-      {
-        kill(pid, SIGTERM) ;
-        kill(pid, SIGCONT) ;
-        x[0].fd = -1 ;
-        return 1 ;
-      }
+      kill(pid, SIGTERM) ;
+      kill(pid, SIGCONT) ;
+      return 1 ;
     }
   }
-  if (ndelay_off(1) < 0)
-    strerr_diefu1sys(111, "set stdout blocking") ;
-  if (!buffer_flush(buffer_1small))
-    strerr_diefu1sys(111, "write status to client") ;
+
+  if (!x[1].revents)
+  {
+    char pack[UINT_PACK] ;
+    uint_pack_big(pack, (unsigned int)wstat) ;
+    buffer_putnoflush(buffer_1small, pack, UINT_PACK) ;
+    if (ndelay_off(1) < 0)
+      strerr_diefu1sys(111, "set stdout blocking") ;
+    if (!buffer_flush(buffer_1small))
+      strerr_diefu1sys(111, "write status to client") ;
+  }
   return 0 ;
 }
-- 
cgit v1.2.3