commit 4ff50278467b57bcaa33d1d052452de8a35f996c
author James Kuszmaul <james.kuszmaul@bluerivertech.com> Fri Jan 07 18:31:13 2022 -0800
committer James Kuszmaul <james.kuszmaul@bluerivertech.com> Fri Jan 07 18:31:13 2022 -0800
tree 0c2ed3ab089fabb069ea404ad1f9403f5585ea6f
parent 2ca441b75270f0d8807c2de328423c1637b36a01

Have starter also set supplemental groups

We rely on these for resources accessed by some processes.

Change-Id: Idf22b6263ebc313c8abb63fe93bce2cc3ef24274
Signed-off-by: James Kuszmaul <james.kuszmaul@bluerivertech.com>

aos/starter/starterd.cc

diff --git a/aos/starter/starterd.cc b/aos/starter/starterd.cc
index b40776d..a5a340a 100644
--- a/aos/starter/starterd.cc
+++ b/aos/starter/starterd.cc
@@ -24,6 +24,10 @@
         return 1;
       }
     }
+    // Change the real and effective IDs to the user we're running as. The
+    // effective IDs mean files we access (like shared memory) will happen as
+    // that user. The real IDs allow child processes with an different effective
+    // ID to still participate in signal sending/receiving.
     constexpr int kUnchanged = -1;
     if (setresgid(/* ruid */ gid, /* euid */ gid,
                   /* suid */ kUnchanged) != 0) {

aos/starter/starterd_lib.cc

diff --git a/aos/starter/starterd_lib.cc b/aos/starter/starterd_lib.cc
index 7bf2e0d..00b094f 100644
--- a/aos/starter/starterd_lib.cc
+++ b/aos/starter/starterd_lib.cc
@@ -1,6 +1,7 @@
 #include "starterd_lib.h"
 
 #include <fcntl.h>
+#include <grp.h>
 #include <pwd.h>
 #include <sys/fsuid.h>
 #include <sys/prctl.h>
@@ -24,11 +25,11 @@
                 ? application->executable_name()->string_view()
                 : application->name()->string_view()),
       args_(1),
-      user_(application->has_user() ? FindUid(application->user()->c_str())
+      user_name_(application->has_user() ? application->user()->str() : ""),
+      user_(application->has_user() ? FindUid(user_name_.c_str())
                                     : std::nullopt),
-      group_(application->has_user()
-                 ? FindPrimaryGidForUser(application->user()->c_str())
-                 : std::nullopt),
+      group_(application->has_user() ? FindPrimaryGidForUser(user_name_.c_str())
+                                     : std::nullopt),
       autostart_(application->autostart()),
       event_loop_(event_loop),
       start_timer_(event_loop_->AddTimer([this] {
@@ -91,6 +92,23 @@
   }
 
   if (group_) {
+    CHECK(!user_name_.empty());
+    // The manpage for setgroups says we just need CAP_SETGID, but empirically
+    // we also need the effective UID to be 0 to make it work. user_ must also
+    // be set so we change this effective UID back later.
+    CHECK(user_);
+    if (seteuid(0) == -1) {
+      write_pipe_.Write(
+          static_cast<uint32_t>(aos::starter::LastStopReason::SET_GRP_ERR));
+      PLOG(FATAL) << "Could not seteuid(0) for " << name_
+                  << " in preparation for setting groups";
+    }
+    if (initgroups(user_name_.c_str(), *group_) == -1) {
+      write_pipe_.Write(
+          static_cast<uint32_t>(aos::starter::LastStopReason::SET_GRP_ERR));
+      PLOG(FATAL) << "Could not initialize normal groups for " << name_
+                  << " as " << user_name_ << " with " << *group_;
+    }
     if (setgid(*group_) == -1) {
       write_pipe_.Write(
           static_cast<uint32_t>(aos::starter::LastStopReason::SET_GRP_ERR));

aos/starter/starterd_lib.h

diff --git a/aos/starter/starterd_lib.h b/aos/starter/starterd_lib.h
index 1809326..8c70960 100644
--- a/aos/starter/starterd_lib.h
+++ b/aos/starter/starterd_lib.h
@@ -117,6 +117,7 @@
   std::string name_;
   std::string path_;
   std::vector<char *> args_;
+  std::string user_name_;
   std::optional<uid_t> user_;
   std::optional<gid_t> group_;