Skip to content

Commit

Permalink
Fix setting group when only uid specified
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davmac314 committed Jun 24, 2020
1 parent f823b72 commit 9cf877b
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 12 deletions.
13 changes: 7 additions & 6 deletions doc/manpages/dinit-service.5.m4
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/dinitcheck.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
Expand Down
24 changes: 18 additions & 6 deletions src/includes/load-service.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 };
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/load-service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9cf877b

Please sign in to comment.