Fix setting group when only uid specified

For run-as, and socket-uid, the primary group of the specified user - if
specified by name - should be set as the group, unless otherwise
specified.

diff --git a/doc/manpages/dinit-service.5.m4 b/doc/manpages/dinit-service.5.m4
index 5d0ca8d92940c1a1903e7f59042f239f9b92e028..bf6972501f86fd2ecdbc7e16ee3838716e93e8fd 100644 (file)
--- a/doc/manpages/dinit-service.5.m4
+++ b/doc/manpages/dinit-service.5.m4
@@ -91,8 +91,9 @@ Specifies the working directory for this service. For a scripted service, this
 affects both the start command and the stop command.
 .TP
 \fBrun\-as\fR = \fIuser-id\fR
-Specifies which user to run the process(es) for this service as. The group id
-for the process will also be set to the primary group of the specified user.
+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.
 .TP
 \fBrestart\fR = {yes | true | no | false}
 Indicates whether the service should automatically restart if it stops for
@@ -205,11 +206,11 @@ Normally this will be 600 (user access only), 660 (user and group
 access), or 666 (all users). The default is 666.
 .TP
 \fBsocket\-uid\fR = {\fInumeric-user-id\fR | \fIusername\fR}
-Specifies the user that should own the activation socket. If
-\fBsocket\-uid\fR is specified without also specifying \fBsocket-gid\fR, then
+Specifies the user (name or numeric ID) that should own the activation socket. If
+\fBsocket\-uid\fR is specified as a name without also specifying \fBsocket-gid\fR, then
 the socket group is the primary group of the specified user (as found in the
-system user database, normally \fI/etc/passwd\fR). If the socket owner is not
-specified, the socket will be owned by the user id of the Dinit process.
+system user database, normally \fI/etc/passwd\fR). If the \fBsocket\-uid\fR setting is
+not provided, the socket will be owned by the user id of the \fBdinit\fR process.
 .TP
 \fBsocket\-gid\fR = {\fInumeric-group-id\fR | \fIgroup-name\fR}
 Specifies the group of the activation socket. See discussion of

diff --git a/src/dinitcheck.cc b/src/dinitcheck.cc
index 0f0402a6246dedadd518a66de5ce2d557685244f..cafb0fcaf24e2f25802c44782533a415fe67c0a7 100644 (file)
--- a/src/dinitcheck.cc
+++ b/src/dinitcheck.cc
@@ -336,6 +336,8 @@ service_record *load_service(service_set_t &services, const std::string &name,
         throw service_description_exc(name, "Error while reading service description.");
     }
 
+    settings.finalise();
+
     if (settings.service_type != service_type_t::INTERNAL && settings.command.length() == 0) {
         report_service_description_err(name, "Service command not specified.");
     }

diff --git a/src/includes/load-service.h b/src/includes/load-service.h
index 2f6b2780f13e13a6f8da9ab0e014f83c1c4738ac..8363f3b38cef73ed08443990b12179fce8cdd398 100644 (file)
--- a/src/includes/load-service.h
+++ b/src/includes/load-service.h
@@ -275,8 +275,8 @@ inline string read_setting_value(string_iterator & i, string_iterator end,
 
 // Parse a userid parameter which may be a numeric user ID or a username. If a name, the
 // userid is looked up via the system user database (getpwnam() function). In this case,
-// the associated group is stored in the location specified by the group_p parameter iff
-// it is not null and iff it contains the value -1.
+// the associated group is stored in the location specified by the group_p parameter if
+// it is not null.
 inline uid_t parse_uid_param(const std::string &param, const std::string &service_name, const char *setting_name, gid_t *group_p)
 {
     const char * uid_err_msg = "Specified user id contains invalid numeric characters "
@@ -286,7 +286,7 @@ inline uid_t parse_uid_param(const std::string &param, const std::string &servic
     // a user manages to give themselves a username that parses as a number.
     std::size_t ind = 0;
     try {
-        // POSIX does not specify whether uid_t is an signed or unsigned, but regardless
+        // POSIX does not specify whether uid_t is a signed or unsigned type, but regardless
         // is is probably safe to assume that valid values are positive. We'll also assert
         // that the value range fits within "unsigned long long" since it seems unlikely
         // that would ever not be the case.
@@ -320,7 +320,7 @@ inline uid_t parse_uid_param(const std::string &param, const std::string &servic
         }
     }
 
-    if (group_p && *group_p != (gid_t)-1) {
+    if (group_p) {
         *group_p = pwent->pw_gid;
     }
 
@@ -590,6 +590,7 @@ class service_settings_wrapper
     // Note: Posix allows that uid_t and gid_t may be unsigned types, but eg chown uses -1 as an
     // invalid value, so it's safe to assume that we can do the same:
     uid_t socket_uid = -1;
+    gid_t socket_uid_gid = -1;  // primary group of socket user if known
     gid_t socket_gid = -1;
     // Restart limit interval / count; default is 10 seconds, 3 restarts:
     timespec restart_interval = { .tv_sec = 10, .tv_nsec = 0 };
@@ -603,6 +604,7 @@ class service_settings_wrapper
     std::string readiness_var;  // environment var to hold readiness fd
 
     uid_t run_as_uid = -1;
+    gid_t run_as_uid_gid = -1; // primary group of "run as" uid if known
     gid_t run_as_gid = -1;
 
     string chain_to_name;
@@ -611,6 +613,16 @@ class service_settings_wrapper
     char inittab_id[sizeof(utmpx().ut_id)] = {0};
     char inittab_line[sizeof(utmpx().ut_line)] = {0};
     #endif
+
+    // Finalise settings (after processing all setting lines)
+    void finalise()
+    {
+        // If socket_gid hasn't been explicitly set, but the socket_uid was specified as a name (and
+        // we therefore recovered the primary group), use the primary group of the specified user.
+        if (socket_gid == (gid_t)-1) socket_gid = socket_uid_gid;
+        // likewise for "run as" gid/uid.
+        if (run_as_gid == (gid_t)-1) run_as_gid = run_as_uid_gid;
+    }
 };
 
 // Process a service description line. In general, parse the setting value and record the parsed value
@@ -671,7 +683,7 @@ void process_service_line(settings_wrapper &settings, const char *name, string &
     }
     else if (setting == "socket-uid") {
         string sock_uid_s = read_setting_value(i, end, nullptr);
-        settings.socket_uid = parse_uid_param(sock_uid_s, name, "socket-uid", &settings.socket_gid);
+        settings.socket_uid = parse_uid_param(sock_uid_s, name, "socket-uid", &settings.socket_uid_gid);
     }
     else if (setting == "socket-gid") {
         string sock_gid_s = read_setting_value(i, end, nullptr);
@@ -828,7 +840,7 @@ void process_service_line(settings_wrapper &settings, const char *name, string &
     }
     else if (setting == "run-as") {
         string run_as_str = read_setting_value(i, end, nullptr);
-        settings.run_as_uid = parse_uid_param(run_as_str, name, "run-as", &settings.run_as_gid);
+        settings.run_as_uid = parse_uid_param(run_as_str, name, "run-as", &settings.run_as_uid_gid);
     }
     else if (setting == "chain-to") {
         settings.chain_to_name = read_setting_value(i, end, nullptr);

diff --git a/src/load-service.cc b/src/load-service.cc
index fe39771546251b4d8e482dff34c5ba47ea202661..e1e946ef229ac690324e33347bf8290f67acf74f 100644 (file)
--- a/src/load-service.cc
+++ b/src/load-service.cc
@@ -303,6 +303,7 @@ service_record * dirload_service_set::load_reload_service(const char *name, serv
 
         service_file.close();
 
+        settings.finalise();
         auto service_type = settings.service_type;
 
         if (service_type == service_type_t::PROCESS || service_type == service_type_t::BGPROCESS