s6

commit e910005b1a337093109af29c7bf21b32c343ab56
parent fb6877e47d8a60b1e00ea55b2203589a43a610d0
Author: Laurent Bercot <ska-skaware@skarnet.org>
Date:   Mon,  4 Feb 2019 19:04:05 +0000

 Revert -I, but add uid/self and gid/self to uidgid accessrules checking

Diffstat:
M
NEWS
 | 
7
+++++--
M
doc/libs6/accessrules.html
 | 
15
+++++++++++----
M
doc/s6-ipcserver-access.html
 | 
7
+++----
M
doc/upgrade.html
 | 
4
+++-
M
src/conn-tools/s6-ipcserver-access.c
 | 
7
++-----
M
src/libs6/s6_accessrules_keycheck_uidgid.c
 | 
22
++++++++++++++++++----

6 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/NEWS b/NEWS
@@ -7,8 +7,11 @@ In 2.8.0.0
  - Adaptation to skalibs-2.8.0.0.
  - s6-log can now notify readiness with the new -d option.
  - s6-log now has a default line limit of 8 kB.
- - s6-ipcserver-access now takes a -I option to automatically accept
-connections from clients running with the same euid/egid pair.
+ - In the accessrules library, checking against uidgid now checks the
+uid/self key if the client and the server have the same uid, and the
+gid/self key if the client and the server have the same gid. That means
+s6-ipcserver-access can now be configured to allow/deny/specialcase
+connections where the client's credentials are the same as the server's.
 
 
 In 2.7.2.2
diff --git a/doc/libs6/accessrules.html b/doc/libs6/accessrules.html
@@ -124,10 +124,17 @@ is not S6_ACCESSRULES_NOTFOUND. If no match can be found in the whole list,
 <a name="uidgid" />
  <tt>s6_accessrules_keycheck_uidgid</tt> interprets <em>key</em> as a
 pointer to a structure containing an uid <em>u</em> and a gid <em>g</em>.
-The function first looks
-for a <tt>uid/<em>u</em></tt> match; if it cannot find one, it looks for a
-<tt>gid/<em>g</em></tt> match. If it cannot find one either, it checks
-<tt>uid/default</tt> and returns the result. </li>
+The following checks are performed, in this order (i.e. subsequent
+checks are not performed if a match is found):
+ <ul>
+  <li> If <em>u</em> is equal to the effective uid of the process, look
+for a <tt>uid/self</tt> match. </li>
+  <li> Look for a <tt>uid/<em>u</em></tt> match. </li>
+  <li> If <em>g</em> is equal to the effective gid of the process, look
+for a <tt>gid/self</tt> match. </li>
+  <li> Look for a <tt>gid/<em>g</em></tt> match. </li>
+  <li> Look for a <tt>uid/default</tt> match. </li>
+ </ul> </li>
  <li>
 <a name="reversedns" />
  <tt>s6_accessrules_keycheck_reversedns</tt> interprets <em>key</em>
diff --git a/doc/s6-ipcserver-access.html b/doc/s6-ipcserver-access.html
@@ -30,7 +30,7 @@ the application program on the s6-ipcserver command line.
 <h2> Interface </h2>
 
 <pre>
-     s6-ipcserver-access [ -v <em>verbosity</em> ] [ -E | -e ] [ -l <em>localname</em> ] [ -I ] [ -i <em>rulesdir</em> | -x <em>rulesfile</em> ] <em>prog...</em>
+     s6-ipcserver-access [ -v <em>verbosity</em> ] [ -E | -e ] [ -l <em>localname</em> ] [ -i <em>rulesdir</em> | -x <em>rulesfile</em> ] <em>prog...</em>
 </pre>
 
 <ul>
@@ -95,9 +95,6 @@ This is the default. </li>
  <li> <tt>-l&nbsp;<em>localname</em></tt>&nbsp;: use <em>localname</em>
 as the value for the ${PROTO}LOCALPATH environment variable, instead of
 looking it up via getsockname(). </li>
- <li> <tt>-I</tt>&nbsp;: accept identity connections. If a client connects
-with the same effective uid/gid pair as s6-ipcserver-access is running under,
-then the ruleset check is bypassed and the connection is accepted. </li>
  <li> <tt>-i&nbsp;<em>rulesdir</em></tt>&nbsp;: check client credentials
 against a filesystem-based database in the <em>rulesdir</em> directory. </li>
  <li> <tt>-x&nbsp;<em>rulesfile</em></tt>&nbsp;: check client credentials
@@ -141,7 +138,9 @@ them with the
 function. In other words, it tries to match:
 
 <ul>
+ <li> (if the client's effective uid is the same as <tt>s6-ipcserver-access</tt>'s effective uid) <tt>uid/self</tt> </li>
  <li> <tt>uid/</tt><em>uid</em> </li>
+ <li> (if the client's effective gid is the same as <tt>s6-ipcserver-access</tt>'s effective gid) <tt>gid/self</tt> </li>
  <li> <tt>gid/</tt><em>gid</em> </li>
  <li> <tt>uid/default</tt> </li>
 </ul>
diff --git a/doc/upgrade.html b/doc/upgrade.html
@@ -26,7 +26,9 @@
  <li> New <tt>-d <em>notif</em></tt> option to <a href="s6-log.html">s6-log</a>. </li>
  <li> New default for the <tt>-l <em>linelimit</em></tt> option to <a href="s6-log.html">s6-log</a>:
 8192 bytes. </li>
- <li> New <tt>-I</tt> option to <a href="s6-ipcserver-access.html">s6-ipcserver-access</a>. </li>
+ <li> In the <a href="libs6/accessrules.html">accessrules</a> library, checking against uid/gid
+now checks the <tt>uid/self</tt> key if the client and the server have the same uid, and
+the <tt>gid/self</tt> key if the client and the server have the same gid. </li>
 </ul>
 
 <h2> in 2.7.2.2 </h2>
diff --git a/src/conn-tools/s6-ipcserver-access.c b/src/conn-tools/s6-ipcserver-access.c
@@ -14,7 +14,7 @@
 #include <execline/config.h>
 #include <s6/accessrules.h>
 
-#define USAGE "s6-ipcserver-access [ -v verbosity ] [ -e | -E ] [ -l localname ] [ -I ] [ -i rulesdir | -x rulesfile ] prog..."
+#define USAGE "s6-ipcserver-access [ -v verbosity ] [ -e | -E ] [ -l localname ] [ -i rulesdir | -x rulesfile ] prog..."
 
 static unsigned int verbosity = 1 ;
 
@@ -118,14 +118,13 @@ int main (int argc, char const *const *argv, char const *const *envp)
   uid_t uid = 0 ;
   gid_t gid = 0 ;
   unsigned int rulestype = 0 ;
-  int identity = 0 ;
   int doenv = 1 ;
   PROG = "s6-ipcserver-access" ;
   {
     subgetopt_t l = SUBGETOPT_ZERO ;
     for (;;)
     {
-      int opt = subgetopt_r(argc, argv, "v:Eel:Ii:x:", &l) ;
+      int opt = subgetopt_r(argc, argv, "v:Eel:i:x:", &l) ;
       if (opt == -1) break ;
       switch (opt)
       {
@@ -133,7 +132,6 @@ int main (int argc, char const *const *argv, char const *const *envp)
         case 'E' : doenv = 0 ; break ;
         case 'e' : doenv = 1 ; break ;
         case 'l' : localname = l.arg ; break ;
-        case 'I' : identity = 1 ; break ;
         case 'i' : rules = l.arg ; rulestype = 1 ; break ;
         case 'x' : rules = l.arg ; rulestype = 2 ; break ;
         default : dieusage() ;
@@ -162,7 +160,6 @@ int main (int argc, char const *const *argv, char const *const *envp)
     if (!gid0_scan(x, &gid)) strerr_dieinvalid(100, tmp) ;
   }
 
-  if (identity && uid == geteuid() && gid == getegid()) goto accepted ;
   if (check(&params, rules, rulestype, uid, gid)) goto accepted ;
 
   if (verbosity >= 2) log_deny(getpid(), uid, gid) ;
diff --git a/src/libs6/s6_accessrules_keycheck_uidgid.c b/src/libs6/s6_accessrules_keycheck_uidgid.c
@@ -1,16 +1,30 @@
 /* ISC license. */
 
+#include <unistd.h>
+
 #include <skalibs/uint64.h>
 #include <skalibs/types.h>
 #include <s6/accessrules.h>
 
 s6_accessrules_result_t s6_accessrules_keycheck_uidgid (void const *key, void *data, s6_accessrules_params_t *params, s6_accessrules_backend_func_t_ref check1)
 {
+  uidgid_t const *uidgid = key ;
   char fmt[4 + UINT64_FMT] = "uid/" ;
-  s6_accessrules_result_t r = (*check1)(fmt, 4 + uid_fmt(fmt+4, ((uidgid_t const *)key)->left), data, params) ;
+  s6_accessrules_result_t r ;
+  if (uidgid->left == geteuid())
+  {
+    r = (*check1)("uid/self", 8, data, params) ;
+    if (r != S6_ACCESSRULES_NOTFOUND) return r ;
+  }
+  r = (*check1)(fmt, 4 + uid_fmt(fmt+4, uidgid->left), data, params) ;
   if (r != S6_ACCESSRULES_NOTFOUND) return r ;
+  if (uidgid->right == getegid())
+  {
+    r = (*check1)("gid/self", 8, data, params) ;
+    if (r != S6_ACCESSRULES_NOTFOUND) return r ;
+  }
   fmt[0] = 'g' ;
-  r = (*check1)(fmt, 4 + gid_fmt(fmt+4, ((uidgid_t const *)key)->right), data, params) ;
-  return (r != S6_ACCESSRULES_NOTFOUND) ? r :
-   (*check1)("uid/default", 11, data, params) ;
+  r = (*check1)(fmt, 4 + gid_fmt(fmt+4, uidgid->right), data, params) ;
+  if (r != S6_ACCESSRULES_NOTFOUND) return r ;
+  return (*check1)("uid/default", 11, data, params) ;
 }