Browse Source

run-child-proc: initialize supplementary groups for named run-as

This is practically portable (exists on Linux, BSDs, Solaris, etc)
despite not being POSIX, so enable it unconditionally without
extra checks. It can still be disabled explicitly if needed.

Fixes https://github.com/davmac314/dinit/issues/151
Daniel Kolesa 11 months ago
parent
commit
63f60db867
9 changed files with 62 additions and 1 deletions
  1. 4 0
      BUILD
  2. 10 0
      BUILD_MESON
  3. 2 1
      build/Makefile
  4. 3 0
      build/tools/mconfig-gen.cc
  5. 9 0
      configure
  6. 2 0
      doc/manpages/dinit-service.5.m4
  7. 5 0
      meson.build
  8. 6 0
      meson_options.txt
  9. 21 0
      src/run-child-proc.cc

+ 4 - 0
BUILD

@@ -122,6 +122,10 @@ USE_UTMPX=1|0
     like "who" to work correctly (the service configuration items "inittab-id" and "inittab-line"
     have no effect if this is disabled). If not set to any value, support is enabled for certain
     systems automatically and disabled for all others.
+USE_INITGROUPS=1|0
+    Whether to initialize supplementary groups for run-as services. The C API for this is not
+    in POSIX, but is in practice supported on just about every relevant system, so it is enabled
+    by default. If it is not supported on yours, you can explicitly disable it.
 
 
 Running test suite

+ 10 - 0
BUILD_MESON

@@ -117,6 +117,16 @@ Custom options:
                              Available values : enabled, disabled, auto
                              Default value : auto
 
+ use-initgroups            : Whether to use initgroups to initialize supplementary
+                             groups in services that use run-as with a name. Without
+                             this the services may potentially misbehave as process
+                             and its child processes won't recognize being in the
+                             supplementary groups, only their primary group. However,
+                             some systems may not implement the initgroups API, so
+                             it may be necessary to disable it on those.
+                             Available vlaues : enabled, disabled, auto
+                             Default value : auto
+
  dinit-sbindir             : Default full path to the dinit executables.
                              For some reasons Dinit dont follow Meson's default
                              sbindir option. see "Why we use another option

+ 2 - 1
build/Makefile

@@ -12,7 +12,8 @@ includes/mconfig.h: ../mconfig tools/mconfig-gen.cc version.conf
 	./tools/mconfig-gen SBINDIR=$(SBINDIR) SYSCONTROLSOCKET=$(SYSCONTROLSOCKET) \
 		SHUTDOWN_PREFIX=$(SHUTDOWN_PREFIX) VERSION=$(VERSION) \
 		$(if $(SUPPORT_CGROUPS),SUPPORT_CGROUPS=$(SUPPORT_CGROUPS),) \
-		$(if $(USE_UTMPX),USE_UTMPX=$(USE_UTMPX),) > includes/mconfig.h
+		$(if $(USE_UTMPX),USE_UTMPX=$(USE_UTMPX),) \
+		$(if $(USE_INITGROUPS),USE_INITGROUPS=$(USE_INITGROUPS),) > includes/mconfig.h
 
 clean:
 	rm -f includes/mconfig.h

+ 3 - 0
build/tools/mconfig-gen.cc

@@ -67,6 +67,9 @@ int main(int argc, char **argv)
     if (vars.find("USE_UTMPX") != vars.end()) {
         cout << "#define USE_UTMPX " << vars["USE_UTMPX"] << "\n";
     }
+    if (vars.find("USE_INITGROUPS") != vars.end()) {
+        cout << "#define USE_INITGROUPS " << vars["USE_INITGROUPS"] << "\n";
+    }
     if (vars.find("SUPPORT_CGROUPS") != vars.end()) {
         cout << "#define SUPPORT_CGROUPS " << vars["SUPPORT_CGROUPS"] << "\n";
     }

+ 9 - 0
configure

@@ -139,6 +139,8 @@ Optional options:
   --disable-cgroups             Disable Cgroups support
   --enable-utmpx                Enable manipulating the utmp/utmpx database via the related POSIX functions [auto]
   --disable-utmpx               Disable manipulating the utmp/utmpx database via the related POSIX functions
+  --enable-initgroups           Enable initialization of supplementary groups for run-as [default]
+  --disable-initgroups          Disable initialization of supplementary groups for run-as
 
 Build variables:
   Note: build variables can be passed in the environment, or as $0 argument (as "var=VALUE").
@@ -183,6 +185,8 @@ for var in PREFIX \
            SHUTDOWN_PREFIX \
            BUILD_SHUTDOWN \
            SUPPORT_CGROUPS \
+           USE_UTMPX \
+           USE_INITGROUPS \
            SYSCONTROLSOCKET
 do
     unset $var
@@ -212,6 +216,8 @@ for arg in "$@"; do
         --disable-cgroups|--enable-cgroups=no) SUPPORT_CGROUPS=0 ;;
         --enable-utmpx|--enable-utmpx=yes) USE_UTMPX=1 ;;
         --disable-utmpx|--enable-utmpx=no) USE_UTMPX=0 ;;
+        --enable-initgroups|--enable-initgroups=yes) USE_INITGROUPS=1 ;;
+        --disable-initgroups|--enable-initgroups=no) USE_INITGROUPS=0 ;;
         CXX=*|CXX_FOR_BUILD=*|CXXFLAGS_FOR_BUILD=*|CPPFLAGS_FOR_BUILD=*\
         |LDFLAGS_FOR_BUILD=*|CXXFLAGS=*|CXXFLAGS_EXTRA=*|TEST_CXXFLAGS=*\
         |TEST_CXXFLAGS_EXTRA|LDFLAGS=*|LDFLAGS_EXTRA=*|TEST_LDFLAGS=*\
@@ -366,6 +372,9 @@ _EOF
 if [ -n "${USE_UTMPX:-}" ]; then
     echo "USE_UTMPX=$USE_UTMPX" >> mconfig
 fi
+if [ -n "${USE_INITGROUPS:-}" ]; then
+    echo "USE_INITGROUPS=$USE_INITGROUPS" >> mconfig
+fi
 if [ -n "${CXX_FOR_BUILD:-}" ]; then
     {
         echo ""

+ 2 - 0
doc/manpages/dinit-service.5.m4

@@ -156,6 +156,8 @@ Specifies which user to run the process(es) for this service as.
 Specify as a username or numeric ID.
 If specified by name, the group for the process will also be set to the primary
 group of the specified user.
+Supplementary groups will be initialized unless disabled, not supported on
+the platform, or the user could not be found in passwd database.
 .TP
 \fBenv\-file\fR = \fIfile\fR
 Specifies a file containing value assignments for environment variables, in the same

+ 5 - 0
meson.build

@@ -66,6 +66,11 @@ if use_utmpx.enabled()
 elif use_utmpx.disabled()
     mconfig_data.set('USE_UTMPX', '0')
 endif
+if use_initgroups.enabled()
+    mconfig_data.set('USE_INITGROUPS', '1')
+elif use_initgroups.disabled()
+    mconfig_data.set('USE_INITGROUPS', '0')
+endif
 
 ## Outputs
 subdir('src')

+ 6 - 0
meson_options.txt

@@ -40,6 +40,12 @@ option(
     value : 'auto',
     description : 'Whether to build support for manipulating the utmp/utmpx database via the related POSIX functions.'
 )
+option(
+    'use-initgroups',
+    type : 'feature',
+    value : 'auto',
+    description : 'Whether to use initgroups to initialize supplementary groups for run-as services.'
+)
 option(
     'dinit-sbindir',
     type : 'string',

+ 21 - 0
src/run-child-proc.cc

@@ -19,6 +19,15 @@ extern std::string cgroups_path;
 extern bool have_cgroups_path;
 #endif
 
+#ifndef USE_INITGROUPS
+// Don't do OS checks, as it's supported practically everwhere
+#define USE_INITGROUPS 1
+#endif
+
+#if USE_INITGROUPS
+#include <grp.h>
+#endif
+
 // Move an fd, if necessary, to another fd. The destination fd must be available (not open).
 // if fd is specified as -1, returns -1 immediately. Returns 0 on success.
 static int move_fd(int fd, int dest)
@@ -325,6 +334,18 @@ void base_process_service::run_child_proc(run_proc_params params) noexcept
         err.stage = exec_stage::SET_UIDGID;
         // We must set group first (i.e. before we drop privileges)
         if (setregid(gid, gid) != 0) goto failure_out;
+#if USE_INITGROUPS
+        // Initialize supplementary groups unless disabled; non-POSIX API
+        if (gid != gid_t(-1)) {
+            errno = 0;
+            // null result with no errno indicates missing passwd entry
+            auto *pw = getpwuid(uid);
+            if (pw) {
+                if (initgroups(pw->pw_name, gid) != 0) goto failure_out;
+            }
+            else if (errno) goto failure_out;
+        }
+#endif
         if (setreuid(uid, uid) != 0) goto failure_out;
     }