Merge pull request #7816 from poettering/chase-pid

Make MAINPID= and PIDFile= handling more restrictive (and other stuff)

coccinelle/enotsup.cocci

@@ -0,0 +1,4 @@
+@@
+@@
+- ENOTSUP
++ EOPNOTSUPP

man/systemd-notify.xml

@@ -122,6 +122,15 @@
         <citerefentry><refentrytitle>sd_notify</refentrytitle><manvolnum>3</manvolnum></citerefentry>.</para></listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><option>--uid=</option><replaceable>USER</replaceable></term>
+
+        <listitem><para>Set the user ID to send the notification from. Takes a UNIX user name or numeric UID. When
+        specified the notification message will be sent with the specified UID as sender, in place of the user the
+        command was invoked as. This option requires sufficient privileges in order to be able manipulate the user
+        identity of the process.</para></listitem>
+      </varlistentry>
+
       <varlistentry>
         <term><option>--status=</option></term>
 

man/systemd.service.xml

@@ -264,16 +264,14 @@
       <varlistentry>
         <term><varname>PIDFile=</varname></term>
 
-        <listitem><para>Takes an absolute filename pointing to the
-        PID file of this daemon. Use of this option is recommended for
-        services where <varname>Type=</varname> is set to
-        <option>forking</option>. systemd will read the PID of the
-        main process of the daemon after start-up of the service.
-        systemd will not write to the file configured here, although
-        it will remove the file after the service has shut down if it
-        still exists.
-        </para>
-        </listitem>
+        <listitem><para>Takes an absolute path referring to the PID file of the service. Usage of this option is
+        recommended for services where <varname>Type=</varname> is set to <option>forking</option>. The service manager
+        will read the PID of the main process of the service from this file after start-up of the service. The service
+        manager will not write to the file configured here, although it will remove the file after the service has shut
+        down if it still exists. The PID file does not need to be owned by a privileged user, but if it is owned by an
+        unprivileged user additional safety restrictions are enforced: the file may not be a symlink to a file owned by
+        a different user (neither directly nor indirectly), and the PID file must refer to a process already belonging
+        to the service.</para></listitem>
       </varlistentry>
 
       <varlistentry>

src/basic/fs-util.c

@@ -611,16 +611,32 @@ int inotify_add_watch_fd(int fd, int what, uint32_t mask) {
         return r;
 }
 
+static bool safe_transition(const struct stat *a, const struct stat *b) {
+        /* Returns true if the transition from a to b is safe, i.e. that we never transition from unprivileged to
+         * privileged files or directories. Why bother? So that unprivileged code can't symlink to privileged files
+         * making us believe we read something safe even though it isn't safe in the specific context we open it in. */
+
+        if (a->st_uid == 0) /* Transitioning from privileged to unprivileged is always fine */
+                return true;
+
+        return a->st_uid == b->st_uid; /* Otherwise we need to stay within the same UID */
+}
+
 int chase_symlinks(const char *path, const char *original_root, unsigned flags, char **ret) {
         _cleanup_free_ char *buffer = NULL, *done = NULL, *root = NULL;
         _cleanup_close_ int fd = -1;
         unsigned max_follow = 32; /* how many symlinks to follow before giving up and returning ELOOP */
+        struct stat previous_stat;
         bool exists = true;
         char *todo;
         int r;
 
         assert(path);
 
+        /* Either the file may be missing, or we return an fd to the final object, but both make no sense */
+        if ((flags & (CHASE_NONEXISTENT|CHASE_OPEN)) == (CHASE_NONEXISTENT|CHASE_OPEN))
+                return -EINVAL;
+
         /* This is a lot like canonicalize_file_name(), but takes an additional "root" parameter, that allows following
          * symlinks relative to a root directory, instead of the root of the host.
          *
@@ -658,6 +674,11 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
         if (fd < 0)
                 return -errno;
 
+        if (flags & CHASE_SAFE) {
+                if (fstat(fd, &previous_stat) < 0)
+                        return -errno;
+        }
+
         todo = buffer;
         for (;;) {
                 _cleanup_free_ char *first = NULL;
@@ -719,6 +740,16 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
                         if (fd_parent < 0)
                                 return -errno;
 
+                        if (flags & CHASE_SAFE) {
+                                if (fstat(fd_parent, &st) < 0)
+                                        return -errno;
+
+                                if (!safe_transition(&previous_stat, &st))
+                                        return -EPERM;
+
+                                previous_stat = st;
+                        }
+
                         safe_close(fd);
                         fd = fd_parent;
 
@@ -753,6 +784,12 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
 
                 if (fstat(child, &st) < 0)
                         return -errno;
+                if ((flags & CHASE_SAFE) &&
+                    !safe_transition(&previous_stat, &st))
+                        return -EPERM;
+
+                previous_stat = st;
+
                 if ((flags & CHASE_NO_AUTOFS) &&
                     fd_is_fs_type(child, AUTOFS_SUPER_MAGIC) > 0)
                         return -EREMOTE;
@@ -785,6 +822,16 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
 
                                 free(done);
 
+                                if (flags & CHASE_SAFE) {
+                                        if (fstat(fd, &st) < 0)
+                                                return -errno;
+
+                                        if (!safe_transition(&previous_stat, &st))
+                                                return -EPERM;
+
+                                        previous_stat = st;
+                                }
+
                                 /* Note that we do not revalidate the root, we take it as is. */
                                 if (isempty(root))
                                         done = NULL;
@@ -839,6 +886,19 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
                 done = NULL;
         }
 
+        if (flags & CHASE_OPEN) {
+                int q;
+
+                /* Return the O_PATH fd we currently are looking to the caller. It can translate it to a proper fd by
+                 * opening /proc/self/fd/xyz. */
+
+                assert(fd >= 0);
+                q = fd;
+                fd = -1;
+
+                return q;
+        }
+
         return exists;
 }
 

src/basic/fs-util.h

@@ -81,9 +81,11 @@ union inotify_event_buffer {
 int inotify_add_watch_fd(int fd, int what, uint32_t mask);
 
 enum {
-        CHASE_PREFIX_ROOT = 1,   /* If set, the specified path will be prefixed by the specified root before beginning the iteration */
-        CHASE_NONEXISTENT = 2,   /* If set, it's OK if the path doesn't actually exist. */
-        CHASE_NO_AUTOFS = 4,     /* If set, return -EREMOTE if autofs mount point found */
+        CHASE_PREFIX_ROOT = 1U << 0,   /* If set, the specified path will be prefixed by the specified root before beginning the iteration */
+        CHASE_NONEXISTENT = 1U << 1,   /* If set, it's OK if the path doesn't actually exist. */
+        CHASE_NO_AUTOFS   = 1U << 2,   /* If set, return -EREMOTE if autofs mount point found */
+        CHASE_SAFE        = 1U << 3,   /* If set, return EPERM if we ever traverse from unprivileged to privileged files or directories */
+        CHASE_OPEN        = 1U << 4,   /* If set, return an O_PATH object to the final component */
 };
 
 int chase_symlinks(const char *path_with_prefix, const char *root, unsigned flags, char **ret);

src/core/dbus-util.c

@@ -93,12 +93,12 @@ int bus_set_transient_usec_internal(
                 UnitWriteFlags flags,
                 sd_bus_error *error) {
 
-        usec_t v;
+        uint64_t v;
         int r;
 
         assert(p);
 
-        r = sd_bus_message_read(message, "u", &v);
+        r = sd_bus_message_read(message, "t", &v);
         if (r < 0)
                 return r;
 
@@ -106,7 +106,7 @@ int bus_set_transient_usec_internal(
                 char *n, ts[FORMAT_TIMESPAN_MAX];
 
                 if (fix_0)
-                        *p = v ?: USEC_INFINITY;
+                        *p = v != 0 ? v: USEC_INFINITY;
                 else
                         *p = v;
 

src/core/manager.c

@@ -1879,27 +1879,35 @@ static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, ui
         return 0;
 }
 
-static void manager_invoke_notify_message(Manager *m, Unit *u, pid_t pid, const char *buf, FDSet *fds) {
+static void manager_invoke_notify_message(
+                Manager *m,
+                Unit *u,
+                const struct ucred *ucred,
+                const char *buf,
+                FDSet *fds) {
+
         _cleanup_strv_free_ char **tags = NULL;
 
         assert(m);
         assert(u);
+        assert(ucred);
         assert(buf);
 
-        tags = strv_split(buf, "\n\r");
+        tags = strv_split(buf, NEWLINE);
         if (!tags) {
                 log_oom();
                 return;
         }
 
         if (UNIT_VTABLE(u)->notify_message)
-                UNIT_VTABLE(u)->notify_message(u, pid, tags, fds);
+                UNIT_VTABLE(u)->notify_message(u, ucred, tags, fds);
         else if (DEBUG_LOGGING) {
                 _cleanup_free_ char *x = NULL, *y = NULL;
 
-                x = cescape(buf);
+                x = ellipsize(buf, 20, 90);
                 if (x)
-                        y = ellipsize(x, 20, 90);
+                        y = cescape(x);
+
                 log_unit_debug(u, "Got notification message \"%s\", ignoring.", strnull(y));
         }
 }
@@ -1976,7 +1984,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
                 }
         }
 
-        if (!ucred || ucred->pid <= 0) {
+        if (!ucred || !pid_is_valid(ucred->pid)) {
                 log_warning("Received notify message without valid credentials. Ignoring.");
                 return 0;
         }
@@ -2000,15 +2008,15 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
          * to avoid notifying the same one multiple times. */
         u1 = manager_get_unit_by_pid_cgroup(m, ucred->pid);
         if (u1)
-                manager_invoke_notify_message(m, u1, ucred->pid, buf, fds);
+                manager_invoke_notify_message(m, u1, ucred, buf, fds);
 
         u2 = hashmap_get(m->watch_pids1, PID_TO_PTR(ucred->pid));
         if (u2 && u2 != u1)
-                manager_invoke_notify_message(m, u2, ucred->pid, buf, fds);
+                manager_invoke_notify_message(m, u2, ucred, buf, fds);
 
         u3 = hashmap_get(m->watch_pids2, PID_TO_PTR(ucred->pid));
         if (u3 && u3 != u2 && u3 != u1)
-                manager_invoke_notify_message(m, u3, ucred->pid, buf, fds);
+                manager_invoke_notify_message(m, u3, ucred, buf, fds);
 
         if (!u1 && !u2 && !u3)
                 log_warning("Cannot find unit for notify message of PID "PID_FMT".", ucred->pid);

src/core/service.c

@@ -866,9 +866,45 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) {
         cgroup_context_dump(&s->cgroup_context, f, prefix);
 }
 
+static int service_is_suitable_main_pid(Service *s, pid_t pid, int prio) {
+        Unit *owner;
+
+        assert(s);
+        assert(pid_is_valid(pid));
+
+        /* Checks whether the specified PID is suitable as main PID for this service. returns negative if not, 0 if the
+         * PID is questionnable but should be accepted if the source of configuration is trusted. > 0 if the PID is
+         * good */
+
+        if (pid == getpid_cached() || pid == 1) {
+                log_unit_full(UNIT(s), prio, 0, "New main PID "PID_FMT" is the manager, refusing.", pid);
+                return -EPERM;
+        }
+
+        if (pid == s->control_pid) {
+                log_unit_full(UNIT(s), prio, 0, "New main PID "PID_FMT" is the control process, refusing.", pid);
+                return -EPERM;
+        }
+
+        if (!pid_is_alive(pid)) {
+                log_unit_full(UNIT(s), prio, 0, "New main PID "PID_FMT" does not exist or is a zombie.", pid);
+                return -ESRCH;
+        }
+
+        owner = manager_get_unit_by_pid(UNIT(s)->manager, pid);
+        if (owner == UNIT(s)) {
+                log_unit_debug(UNIT(s), "New main PID "PID_FMT" belongs to service, we are happy.", pid);
+                return 1; /* Yay, it's definitely a good PID */
+        }
+
+        return 0; /* Hmm it's a suspicious PID, let's accept it if configuration source is trusted */
+}
+
 static int service_load_pid_file(Service *s, bool may_warn) {
+        char procfs[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int)];
         _cleanup_free_ char *k = NULL;
-        int r;
+        _cleanup_close_ int fd = -1;
+        int r, prio;
         pid_t pid;
 
         assert(s);
@@ -876,30 +912,47 @@ static int service_load_pid_file(Service *s, bool may_warn) {
         if (!s->pid_file)
                 return -ENOENT;
 
-        r = read_one_line_file(s->pid_file, &k);
-        if (r < 0) {
-                if (may_warn)
-                        log_unit_info_errno(UNIT(s), r, "PID file %s not readable (yet?) after %s: %m", s->pid_file, service_state_to_string(s->state));
-                return r;
-        }
+        prio = may_warn ? LOG_INFO : LOG_DEBUG;
+
+        fd = chase_symlinks(s->pid_file, NULL, CHASE_OPEN|CHASE_SAFE, NULL);
+        if (fd == -EPERM)
+                return log_unit_full(UNIT(s), prio, fd, "Permission denied while opening PID file or unsafe symlink chain: %s", s->pid_file);
+        if (fd < 0)
+                return log_unit_full(UNIT(s), prio, fd, "Can't open PID file %s (yet?) after %s: %m", s->pid_file, service_state_to_string(s->state));
+
+        /* Let's read the PID file now that we chased it down. But we need to convert the O_PATH fd chase_symlinks() returned us into a proper fd first. */
+        xsprintf(procfs, "/proc/self/fd/%i", fd);
+        r = read_one_line_file(procfs, &k);
+        if (r < 0)
+                return log_unit_error_errno(UNIT(s), r, "Can't convert PID files %s O_PATH file descriptor to proper file descriptor: %m", s->pid_file);
 
         r = parse_pid(k, &pid);
-        if (r < 0) {
-                if (may_warn)
-                        log_unit_info_errno(UNIT(s), r, "Failed to read PID from file %s: %m", s->pid_file);
-                return r;
-        }
+        if (r < 0)
+                return log_unit_full(UNIT(s), prio, r, "Failed to parse PID from file %s: %m", s->pid_file);
 
-        if (!pid_is_alive(pid)) {
-                if (may_warn)
-                        log_unit_info(UNIT(s), "PID "PID_FMT" read from file %s does not exist or is a zombie.", pid, s->pid_file);
-                return -ESRCH;
+        if (s->main_pid_known && pid == s->main_pid)
+                return 0;
+
+        r = service_is_suitable_main_pid(s, pid, prio);
+        if (r < 0)
+                return r;
+        if (r == 0) {
+                struct stat st;
+
+                /* Hmm, it's not clear if the new main PID is safe. Let's allow this if the PID file is owned by root */
+
+                if (fstat(fd, &st) < 0)
+                        return log_unit_error_errno(UNIT(s), errno, "Failed to fstat() PID file O_PATH fd: %m");
+
+                if (st.st_uid != 0) {
+                        log_unit_error(UNIT(s), "New main PID "PID_FMT" does not belong to service, and PID file is not owned by root. Refusing.", pid);
+                        return -EPERM;
+                }
+
+                log_unit_debug(UNIT(s), "New main PID "PID_FMT" does not belong to service, but we'll accept it since PID file is owned by root.", pid);
         }
 
         if (s->main_pid_known) {
-                if (pid == s->main_pid)
-                        return 0;
-
                 log_unit_debug(UNIT(s), "Main PID changing: "PID_FMT" -> "PID_FMT, s->main_pid, pid);
 
                 service_unwatch_main_pid(s);
@@ -915,7 +968,7 @@ static int service_load_pid_file(Service *s, bool may_warn) {
         if (r < 0) /* FIXME: we need to do something here */
                 return log_unit_warning_errno(UNIT(s), r, "Failed to watch PID "PID_FMT" for service: %m", pid);
 
-        return 0;
+        return 1;
 }
 
 static void service_search_main_pid(Service *s) {
@@ -2983,7 +3036,7 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
                 /* Forking services may occasionally move to a new PID.
                  * As long as they update the PID file before exiting the old
                  * PID, they're fine. */
-                if (service_load_pid_file(s, false) == 0)
+                if (service_load_pid_file(s, false) > 0)
                         return;
 
                 s->main_pid = 0;
@@ -3408,37 +3461,55 @@ static bool service_notify_message_authorized(Service *s, pid_t pid, char **tags
         return true;
 }
 
-static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) {
+static void service_notify_message(
+                Unit *u,
+                const struct ucred *ucred,
+                char **tags,
+                FDSet *fds) {
+
         Service *s = SERVICE(u);
         bool notify_dbus = false;
         const char *e;
         char **i;
+        int r;
 
         assert(u);
+        assert(ucred);
 
-        if (!service_notify_message_authorized(SERVICE(u), pid, tags, fds))
+        if (!service_notify_message_authorized(SERVICE(u), ucred->pid, tags, fds))
                 return;
 
         if (DEBUG_LOGGING) {
                 _cleanup_free_ char *cc = NULL;
 
                 cc = strv_join(tags, ", ");
-                log_unit_debug(u, "Got notification message from PID "PID_FMT" (%s)", pid, isempty(cc) ? "n/a" : cc);
+                log_unit_debug(u, "Got notification message from PID "PID_FMT" (%s)", ucred->pid, isempty(cc) ? "n/a" : cc);
         }
 
         /* Interpret MAINPID= */
         e = strv_find_startswith(tags, "MAINPID=");
         if (e && IN_SET(s->state, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) {
-                if (parse_pid(e, &pid) < 0)
-                        log_unit_warning(u, "Failed to parse MAINPID= field in notification message: %s", e);
-                else if (pid == s->control_pid)
-                        log_unit_warning(u, "A control process cannot also be the main process");
-                else if (pid == getpid_cached() || pid == 1)
-                        log_unit_warning(u, "Service manager can't be main process, ignoring sd_notify() MAINPID= field");
-                else if (pid != s->main_pid) {
-                        service_set_main_pid(s, pid);
-                        unit_watch_pid(UNIT(s), pid);
-                        notify_dbus = true;
+                pid_t new_main_pid;
+
+                if (parse_pid(e, &new_main_pid) < 0)
+                        log_unit_warning(u, "Failed to parse MAINPID= field in notification message, ignoring: %s", e);
+                else if (!s->main_pid_known || new_main_pid != s->main_pid) {
+
+                        r = service_is_suitable_main_pid(s, new_main_pid, LOG_WARNING);
+                        if (r == 0) {
+                                /* The new main PID is a bit suspicous, which is OK if the sender is privileged. */
+
+                                if (ucred->uid == 0) {
+                                        log_unit_debug(u, "New main PID "PID_FMT" does not belong to service, but we'll accept it as the request to change it came from a privileged process.", new_main_pid);
+                                        r = 1;
+                                } else
+                                        log_unit_debug(u, "New main PID "PID_FMT" does not belong to service, refusing.", new_main_pid);
+                        }
+                        if (r > 0) {
+                                service_set_main_pid(s, new_main_pid);
+                                unit_watch_pid(UNIT(s), new_main_pid);
+                                notify_dbus = true;
+                        }
                 }
         }
 

src/core/unit.h

@@ -506,7 +506,7 @@ struct UnitVTable {
         void (*notify_cgroup_empty)(Unit *u);
 
         /* Called whenever a process of this unit sends us a message */
-        void (*notify_message)(Unit *u, pid_t pid, char **tags, FDSet *fds);
+        void (*notify_message)(Unit *u, const struct ucred *ucred, char **tags, FDSet *fds);
 
         /* Called whenever a name this Unit registered for comes or goes away. */
         void (*bus_name_owner_change)(Unit *u, const char *name, const char *old_owner, const char *new_owner);

src/libsystemd/sd-daemon/sd-daemon.c

@@ -456,7 +456,13 @@ _public_ int sd_is_mq(int fd, const char *path) {
         return 1;
 }
 
-_public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char *state, const int *fds, unsigned n_fds) {
+_public_ int sd_pid_notify_with_fds(
+                pid_t pid,
+                int unset_environment,
+                const char *state,
+                const int *fds,
+                unsigned n_fds) {
+
         union sockaddr_union sockaddr = {
                 .sa.sa_family = AF_UNIX,
         };
@@ -471,7 +477,7 @@ _public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char
         _cleanup_close_ int fd = -1;
         struct cmsghdr *cmsg = NULL;
         const char *e;
-        bool have_pid;
+        bool send_ucred;
         int r;
 
         if (!state) {
@@ -505,7 +511,7 @@ _public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char
                 goto finish;
         }
 
-        fd_inc_sndbuf(fd, SNDBUF_SIZE);
+        (void) fd_inc_sndbuf(fd, SNDBUF_SIZE);
 
         iovec.iov_len = strlen(state);
 
@@ -515,13 +521,16 @@ _public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char
 
         msghdr.msg_namelen = SOCKADDR_UN_LEN(sockaddr.un);
 
-        have_pid = pid != 0 && pid != getpid_cached();
+        send_ucred =
+                (pid != 0 && pid != getpid_cached()) ||
+                getuid() != geteuid() ||
+                getgid() != getegid();
 
-        if (n_fds > 0 || have_pid) {
+        if (n_fds > 0 || send_ucred) {
                 /* CMSG_SPACE(0) may return value different than zero, which results in miscalculated controllen. */
                 msghdr.msg_controllen =
                         (n_fds > 0 ? CMSG_SPACE(sizeof(int) * n_fds) : 0) +
-                        (have_pid ? CMSG_SPACE(sizeof(struct ucred)) : 0);
+                        (send_ucred ? CMSG_SPACE(sizeof(struct ucred)) : 0);
 
                 msghdr.msg_control = alloca0(msghdr.msg_controllen);
 
@@ -533,11 +542,11 @@ _public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char
 
                         memcpy(CMSG_DATA(cmsg), fds, sizeof(int) * n_fds);
 
-                        if (have_pid)
+                        if (send_ucred)
                                 assert_se(cmsg = CMSG_NXTHDR(&msghdr, cmsg));
                 }
 
-                if (have_pid) {
+                if (send_ucred) {
                         struct ucred *ucred;
 
                         cmsg->cmsg_level = SOL_SOCKET;
@@ -545,7 +554,7 @@ _public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char
                         cmsg->cmsg_len = CMSG_LEN(sizeof(struct ucred));
 
                         ucred = (struct ucred*) CMSG_DATA(cmsg);
-                        ucred->pid = pid;
+                        ucred->pid = pid != 0 ? pid : getpid_cached();
                         ucred->uid = getuid();
                         ucred->gid = getgid();
                 }
@@ -558,7 +567,7 @@ _public_ int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char
         }
 
         /* If that failed, try with our own ucred instead */
-        if (have_pid) {
+        if (send_ucred) {
                 msghdr.msg_controllen -= CMSG_SPACE(sizeof(struct ucred));
                 if (msghdr.msg_controllen == 0)
                         msghdr.msg_control = NULL;

src/notify/notify.c

@@ -33,12 +33,15 @@
 #include "parse-util.h"
 #include "string-util.h"
 #include "strv.h"
+#include "user-util.h"
 #include "util.h"
 
 static bool arg_ready = false;
 static pid_t arg_pid = 0;
 static const char *arg_status = NULL;
 static bool arg_booted = false;
+static uid_t arg_uid = UID_INVALID;
+static gid_t arg_gid = GID_INVALID;
 
 static void help(void) {
         printf("%s [OPTIONS...] [VARIABLE=VALUE...]\n\n"
@@ -46,7 +49,8 @@ static void help(void) {
                "  -h --help            Show this help\n"
                "     --version         Show package version\n"
                "     --ready           Inform the init system about service start-up completion\n"
-               "     --pid[=PID]       Set main pid of daemon\n"
+               "     --pid[=PID]       Set main PID of daemon\n"
+               "     --uid=USER        Set user to send from\n"
                "     --status=TEXT     Set status text\n"
                "     --booted          Check if the system was booted up with systemd\n",
                program_invocation_short_name);
@@ -60,6 +64,7 @@ static int parse_argv(int argc, char *argv[]) {
                 ARG_PID,
                 ARG_STATUS,
                 ARG_BOOTED,
+                ARG_UID,
         };
 
         static const struct option options[] = {
@@ -69,10 +74,11 @@ static int parse_argv(int argc, char *argv[]) {
                 { "pid",       optional_argument, NULL, ARG_PID       },
                 { "status",    required_argument, NULL, ARG_STATUS    },
                 { "booted",    no_argument,       NULL, ARG_BOOTED    },
+                { "uid",       required_argument, NULL, ARG_UID       },
                 {}
         };
 
-        int c;
+        int c, r;
 
         assert(argc >= 0);
         assert(argv);
@@ -112,6 +118,18 @@ static int parse_argv(int argc, char *argv[]) {
                         arg_booted = true;
                         break;
 
+                case ARG_UID: {
+                        const char *u = optarg;
+
+                        r = get_user_creds(&u, &arg_uid, &arg_gid, NULL, NULL);
+                        if (r == -ESRCH) /* If the user doesn't exist, then accept it anyway as numeric */
+                                r = parse_uid(u, &arg_uid);
+                        if (r < 0)
+                                return log_error_errno(r, "Can't resolve user %s: %m", optarg);
+
+                        break;
+                }
+
                 case '?':
                         return -EINVAL;
 
@@ -190,6 +208,22 @@ int main(int argc, char* argv[]) {
                 goto finish;
         }
 
+        /* If this is requested change to the requested UID/GID. Note thta we only change the real UID here, and leave
+           the effective UID in effect (which is 0 for this to work). That's because we want the privileges to fake the
+           ucred data, and sd_pid_notify() uses the real UID for filling in ucred. */
+
+        if (arg_gid != GID_INVALID)
+                if (setregid(arg_gid, (gid_t) -1) < 0) {
+                        r = log_error_errno(errno, "Failed to change GID: %m");
+                        goto finish;
+                }
+
+        if (arg_uid != UID_INVALID)
+                if (setreuid(arg_uid, (uid_t) -1) < 0) {
+                        r = log_error_errno(errno, "Failed to change UID: %m");
+                        goto finish;
+                }
+
         r = sd_pid_notify(arg_pid ? arg_pid : getppid(), false, n);
         if (r < 0) {
                 log_error_errno(r, "Failed to notify init system: %m");

src/test/test-fs-util.c

@@ -22,21 +22,25 @@
 
 #include "alloc-util.h"
 #include "fd-util.h"
+#include "fd-util.h"
 #include "fileio.h"
 #include "fs-util.h"
+#include "id128-util.h"
 #include "macro.h"
 #include "mkdir.h"
 #include "path-util.h"
 #include "rm-rf.h"
+#include "stdio-util.h"
 #include "string-util.h"
 #include "strv.h"
+#include "user-util.h"
 #include "util.h"
 
 static void test_chase_symlinks(void) {
         _cleanup_free_ char *result = NULL;
         char temp[] = "/tmp/test-chase.XXXXXX";
         const char *top, *p, *pslash, *q, *qslash;
-        int r;
+        int r, pfd;
 
         assert_se(mkdtemp(temp));
 
@@ -235,6 +239,55 @@ static void test_chase_symlinks(void) {
         r = chase_symlinks(p, NULL, 0, &result);
         assert_se(r == -ENOENT);
 
+        if (geteuid() == 0) {
+                p = strjoina(temp, "/priv1");
+                assert_se(mkdir(p, 0755) >= 0);
+
+                q = strjoina(p, "/priv2");
+                assert_se(mkdir(q, 0755) >= 0);
+
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
+
+                assert_se(chown(q, UID_NOBODY, GID_NOBODY) >= 0);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
+
+                assert_se(chown(p, UID_NOBODY, GID_NOBODY) >= 0);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
+
+                assert_se(chown(q, 0, 0) >= 0);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -EPERM);
+
+                assert_se(rmdir(q) >= 0);
+                assert_se(symlink("/etc/passwd", q) >= 0);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) == -EPERM);
+
+                assert_se(chown(p, 0, 0) >= 0);
+                assert_se(chase_symlinks(q, NULL, CHASE_SAFE, NULL) >= 0);
+        }
+
+        p = strjoina(temp, "/machine-id-test");
+        assert_se(symlink("/usr/../etc/./machine-id", p) >= 0);
+
+        pfd = chase_symlinks(p, NULL, CHASE_OPEN, NULL);
+        if (pfd != -ENOENT) {
+                char procfs[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(pfd) + 1];
+                _cleanup_close_ int fd = -1;
+                sd_id128_t a, b;
+
+                assert_se(pfd >= 0);
+
+                xsprintf(procfs, "/proc/self/fd/%i", pfd);
+
+                fd = open(procfs, O_RDONLY|O_CLOEXEC);
+                assert_se(fd >= 0);
+
+                safe_close(pfd);
+
+                assert_se(id128_read_fd(fd, ID128_PLAIN, &a) >= 0);
+                assert_se(sd_id128_get_machine(&b) >= 0);
+                assert_se(sd_id128_equal(a, b));
+        }
+
         assert_se(rm_rf(temp, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0);
 }
 

src/udev/net/ethtool-util.c

@@ -25,13 +25,13 @@
 
 #include "conf-parser.h"
 #include "ethtool-util.h"
-#include "log.h"
 #include "link-config.h"
+#include "log.h"
+#include "missing.h"
 #include "socket-util.h"
 #include "string-table.h"
 #include "strxcpyx.h"
 #include "util.h"
-#include "missing.h"
 
 static const char* const duplex_table[_DUP_MAX] = {
         [DUP_FULL] = "full",
@@ -83,6 +83,7 @@ int ethtool_connect(int *ret) {
         fd = socket_ioctl_fd();
         if (fd < 0)
                 return fd;
+
         *ret = fd;
 
         return 0;
@@ -265,7 +266,7 @@ int ethtool_set_wol(int *fd, const char *ifname, WakeOnLan wol) {
         return 0;
 }
 
-static int ethtool_get_stringset(int *fd, struct ifreq *ifr, int stringset_id, struct ethtool_gstrings **gstrings) {
+static int get_stringset(int fd, struct ifreq *ifr, int stringset_id, struct ethtool_gstrings **gstrings) {
         _cleanup_free_ struct ethtool_gstrings *strings = NULL;
         struct {
                 struct ethtool_sset_info info;
@@ -281,7 +282,7 @@ static int ethtool_get_stringset(int *fd, struct ifreq *ifr, int stringset_id, s
 
         ifr->ifr_data = (void *) &buffer.info;
 
-        r = ioctl(*fd, SIOCETHTOOL, ifr);
+        r = ioctl(fd, SIOCETHTOOL, ifr);
         if (r < 0)
                 return -errno;
 
@@ -300,7 +301,7 @@ static int ethtool_get_stringset(int *fd, struct ifreq *ifr, int stringset_id, s
 
         ifr->ifr_data = (void *) strings;
 
-        r = ioctl(*fd, SIOCETHTOOL, ifr);
+        r = ioctl(fd, SIOCETHTOOL, ifr);
         if (r < 0)
                 return -errno;
 
@@ -335,7 +336,7 @@ int ethtool_set_features(int *fd, const char *ifname, NetDevFeature *features) {
 
         strscpy(ifr.ifr_name, IFNAMSIZ, ifname);
 
-        r = ethtool_get_stringset(fd, &ifr, ETH_SS_FEATURES, &strings);
+        r = get_stringset(*fd, &ifr, ETH_SS_FEATURES, &strings);
         if (r < 0)
                 return log_warning_errno(r, "link_config: could not get ethtool features for %s", ifname);
 
@@ -374,7 +375,7 @@ int ethtool_set_features(int *fd, const char *ifname, NetDevFeature *features) {
         return 0;
 }
 
-static int get_glinksettings(int *fd, struct ifreq *ifr, struct ethtool_link_usettings **g) {
+static int get_glinksettings(int fd, struct ifreq *ifr, struct ethtool_link_usettings **g) {
         struct ecmd {
                 struct ethtool_link_settings req;
                 __u32 link_mode_data[3 * ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32];
@@ -395,29 +396,29 @@ static int get_glinksettings(int *fd, struct ifreq *ifr, struct ethtool_link_use
 
         ifr->ifr_data = (void *) &ecmd;
 
-        r = ioctl(*fd, SIOCETHTOOL, ifr);
+        r = ioctl(fd, SIOCETHTOOL, ifr);
         if (r < 0)
                 return -errno;
 
         if (ecmd.req.link_mode_masks_nwords >= 0 || ecmd.req.cmd != ETHTOOL_GLINKSETTINGS)
-                return -ENOTSUP;
+                return -EOPNOTSUPP;
 
         ecmd.req.link_mode_masks_nwords = -ecmd.req.link_mode_masks_nwords;
 
         ifr->ifr_data = (void *) &ecmd;
 
-        r = ioctl(*fd, SIOCETHTOOL, ifr);
+        r = ioctl(fd, SIOCETHTOOL, ifr);
         if (r < 0)
                 return -errno;
 
         if (ecmd.req.link_mode_masks_nwords <= 0 || ecmd.req.cmd != ETHTOOL_GLINKSETTINGS)
-                return -ENOTSUP;
+                return -EOPNOTSUPP;
 
         u = new0(struct ethtool_link_usettings , 1);
         if (!u)
                 return -ENOMEM;
 
-        memcpy(&u->base, &ecmd.req, sizeof(struct ethtool_link_settings));
+        ecmd.req = u->base;
 
         offset = 0;
         memcpy(u->link_modes.supported, &ecmd.link_mode_data[offset], 4 * ecmd.req.link_mode_masks_nwords);
@@ -433,7 +434,7 @@ static int get_glinksettings(int *fd, struct ifreq *ifr, struct ethtool_link_use
         return 0;
 }
 
-static int get_gset(int *fd, struct ifreq *ifr, struct ethtool_link_usettings **u) {
+static int get_gset(int fd, struct ifreq *ifr, struct ethtool_link_usettings **u) {
         struct ethtool_link_usettings *e;
         struct ethtool_cmd ecmd = {
                 .cmd = ETHTOOL_GSET,
@@ -442,7 +443,7 @@ static int get_gset(int *fd, struct ifreq *ifr, struct ethtool_link_usettings **
 
         ifr->ifr_data = (void *) &ecmd;
 
-        r = ioctl(*fd, SIOCETHTOOL, ifr);
+        r = ioctl(fd, SIOCETHTOOL, ifr);
         if (r < 0)
                 return -errno;
 
@@ -469,7 +470,7 @@ static int get_gset(int *fd, struct ifreq *ifr, struct ethtool_link_usettings **
         return 0;
 }
 
-static int set_slinksettings(int *fd, struct ifreq *ifr, const struct ethtool_link_usettings *u) {
+static int set_slinksettings(int fd, struct ifreq *ifr, const struct ethtool_link_usettings *u) {
         struct {
                 struct ethtool_link_settings req;
                 __u32 link_mode_data[3 * ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32];
@@ -480,7 +481,7 @@ static int set_slinksettings(int *fd, struct ifreq *ifr, const struct ethtool_li
         if (u->base.cmd != ETHTOOL_GLINKSETTINGS || u->base.link_mode_masks_nwords <= 0)
                 return -EINVAL;
 
-        memcpy(&ecmd.req, &u->base, sizeof(ecmd.req));
+        ecmd.req = u->base;
         ecmd.req.cmd = ETHTOOL_SLINKSETTINGS;
         offset = 0;
         memcpy(&ecmd.link_mode_data[offset], u->link_modes.supported, 4 * ecmd.req.link_mode_masks_nwords);
@@ -493,14 +494,14 @@ static int set_slinksettings(int *fd, struct ifreq *ifr, const struct ethtool_li
 
         ifr->ifr_data = (void *) &ecmd;
 
-        r = ioctl(*fd, SIOCETHTOOL, ifr);
+        r = ioctl(fd, SIOCETHTOOL, ifr);
         if (r < 0)
                 return -errno;
 
         return 0;
 }
 
-static int set_sset(int *fd, struct ifreq *ifr, const struct ethtool_link_usettings *u) {
+static int set_sset(int fd, struct ifreq *ifr, const struct ethtool_link_usettings *u) {
         struct ethtool_cmd ecmd = {
                 .cmd = ETHTOOL_SSET,
         };
@@ -523,7 +524,7 @@ static int set_sset(int *fd, struct ifreq *ifr, const struct ethtool_link_usetti
 
         ifr->ifr_data = (void *) &ecmd;
 
-        r = ioctl(*fd, SIOCETHTOOL, ifr);
+        r = ioctl(fd, SIOCETHTOOL, ifr);
         if (r < 0)
                 return -errno;
 
@@ -555,10 +556,9 @@ int ethtool_set_glinksettings(int *fd, const char *ifname, struct link_config *l
 
         strscpy(ifr.ifr_name, IFNAMSIZ, ifname);
 
-        r = get_glinksettings(fd, &ifr, &u);
+        r = get_glinksettings(*fd, &ifr, &u);
         if (r < 0) {
-
-                r = get_gset(fd, &ifr, &u);
+                r = get_gset(*fd, &ifr, &u);
                 if (r < 0)
                         return log_warning_errno(r, "link_config: Cannot get device settings for %s : %m", ifname);
         }
@@ -570,15 +570,14 @@ int ethtool_set_glinksettings(int *fd, const char *ifname, struct link_config *l
                 u->base.duplex = link->duplex;
 
         if (link->port != _NET_DEV_PORT_INVALID)
-              u->base.port = link->port;
+                u->base.port = link->port;
 
         u->base.autoneg = link->autonegotiation;
 
         if (u->base.cmd == ETHTOOL_GLINKSETTINGS)
-                r = set_slinksettings(fd, &ifr, u);
+                r = set_slinksettings(*fd, &ifr, u);
         else
-                r = set_sset(fd, &ifr, u);
-
+                r = set_sset(*fd, &ifr, u);
         if (r < 0)
                 return log_warning_errno(r, "link_config: Cannot set device settings for %s : %m", ifname);
 

test/TEST-20-MAINPIDGAMES/Makefile

@@ -0,0 +1,4 @@
+BUILD_DIR=$(shell ../../tools/find-build-dir.sh)
+
+all setup clean run:
+	@basedir=../.. TEST_BASE_DIR=../ BUILD_DIR=$(BUILD_DIR) ./test.sh --$@

test/TEST-20-MAINPIDGAMES/test.sh

@@ -0,0 +1,42 @@
+#!/bin/bash
+# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*-
+# ex: ts=8 sw=4 sts=4 et filetype=sh
+set -e
+TEST_DESCRIPTION="test changing main PID"
+
+. $TEST_BASE_DIR/test-functions
+
+test_setup() {
+    create_empty_image
+    mkdir -p $TESTDIR/root
+    mount ${LOOPDEV}p1 $TESTDIR/root
+
+    (
+        LOG_LEVEL=5
+        eval $(udevadm info --export --query=env --name=${LOOPDEV}p2)
+
+        setup_basic_environment
+
+        # setup the testsuite service
+        cat >$initdir/etc/systemd/system/testsuite.service <<EOF
+[Unit]
+Description=Testsuite service
+
+[Service]
+ExecStart=/bin/bash -x /testsuite.sh
+Type=oneshot
+StandardOutput=tty
+StandardError=tty
+NotifyAccess=all
+EOF
+        cp testsuite.sh $initdir/
+
+        setup_testsuite
+    ) || return 1
+    setup_nspawn_root
+
+    ddebug "umount $TESTDIR/root"
+    umount $TESTDIR/root
+}
+
+do_test "$@"

test/TEST-20-MAINPIDGAMES/testsuite.sh

@@ -0,0 +1,141 @@
+#!/bin/bash
+# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*-
+# ex: ts=8 sw=4 sts=4 et filetype=sh
+set -ex
+set -o pipefail
+
+systemd-analyze set-log-level debug
+systemd-analyze set-log-target console
+
+test `systemctl show -p MainPID --value testsuite.service` -eq $$
+
+# Start a test process inside of our own cgroup
+sleep infinity &
+INTERNALPID=$!
+disown
+
+# Start a test process outside of our own cgroup
+systemd-run -p DynamicUser=1 --unit=sleep.service /bin/sleep infinity
+EXTERNALPID=`systemctl show -p MainPID --value sleep.service`
+
+# Update our own main PID to the external test PID, this should work
+systemd-notify MAINPID=$EXTERNALPID
+test `systemctl show -p MainPID --value testsuite.service` -eq $EXTERNALPID
+
+# Update our own main PID to the internal test PID, this should work, too
+systemd-notify MAINPID=$INTERNALPID
+test `systemctl show -p MainPID --value testsuite.service` -eq $INTERNALPID
+
+# Update it back to our own PID, this should also work
+systemd-notify MAINPID=$$
+test `systemctl show -p MainPID --value testsuite.service` -eq $$
+
+# Try to set it to PID 1, which it should ignore, because that's the manager
+systemd-notify MAINPID=1
+test `systemctl show -p MainPID --value testsuite.service` -eq $$
+
+# Try to set it to PID 0, which is invalid and should be ignored
+systemd-notify MAINPID=0
+test `systemctl show -p MainPID --value testsuite.service` -eq $$
+
+# Try to set it to a valid but non-existing PID, which should be ignored. (Note
+# that we set the PID to a value well above any known /proc/sys/kernel/pid_max,
+# which means we can be pretty sure it doesn't exist by coincidence)
+systemd-notify MAINPID=1073741824
+test `systemctl show -p MainPID --value testsuite.service` -eq $$
+
+# Change it again to the external PID, without priviliges this time. This should be ignored, because the PID is from outside of our cgroup and we lack privileges.
+systemd-notify --uid=1000 MAINPID=$EXTERNALPID
+test `systemctl show -p MainPID --value testsuite.service` -eq $$
+
+# Change it again to the internal PID, without priviliges this time. This should work, as the process is on our cgroup, and that's enough even if we lack privileges.
+systemd-notify --uid=1000 MAINPID=$INTERNALPID
+test `systemctl show -p MainPID --value testsuite.service` -eq $INTERNALPID
+
+# Update it back to our own PID, this should also work
+systemd-notify --uid=1000 MAINPID=$$
+test `systemctl show -p MainPID --value testsuite.service` -eq $$
+
+cat >/tmp/mainpid.sh <<EOF
+#!/bin/bash
+
+set -eux
+set -o pipefail
+
+# Create a number of children, and make one the main one
+sleep infinity &
+disown
+
+sleep infinity &
+MAINPID=\$!
+disown
+
+sleep infinity &
+disown
+
+echo \$MAINPID > /run/mainpidsh/pid
+EOF
+chmod +x /tmp/mainpid.sh
+
+systemd-run --unit=mainpidsh.service -p StandardOutput=tty -p StandardError=tty -p Type=forking -p RuntimeDirectory=mainpidsh -p PIDFile=/run/mainpidsh/pid /tmp/mainpid.sh
+test `systemctl show -p MainPID --value mainpidsh.service` -eq `cat /run/mainpidsh/pid`
+
+cat >/tmp/mainpid2.sh <<EOF
+#!/bin/bash
+
+set -eux
+set -o pipefail
+
+# Create a number of children, and make one the main one
+sleep infinity &
+disown
+
+sleep infinity &
+MAINPID=\$!
+disown
+
+sleep infinity &
+disown
+
+echo \$MAINPID > /run/mainpidsh2/pid
+chown 1001:1001 /run/mainpidsh2/pid
+EOF
+chmod +x /tmp/mainpid2.sh
+
+systemd-run --unit=mainpidsh2.service -p StandardOutput=tty -p StandardError=tty -p Type=forking -p RuntimeDirectory=mainpidsh2 -p PIDFile=/run/mainpidsh2/pid /tmp/mainpid2.sh
+test `systemctl show -p MainPID --value mainpidsh2.service` -eq `cat /run/mainpidsh2/pid`
+
+cat >/dev/shm/mainpid3.sh <<EOF
+#!/bin/bash
+
+set -eux
+set -o pipefail
+
+sleep infinity &
+disown
+
+sleep infinity &
+disown
+
+sleep infinity &
+disown
+
+# Let's try to play games, and link up a privileged PID file
+ln -s ../mainpidsh/pid /run/mainpidsh3/pid
+
+# Quick assertion that the link isn't dead
+test -f /run/mainpidsh3/pid
+EOF
+chmod 755 /dev/shm/mainpid3.sh
+
+# This has to fail, as we shouldn't accept the dangerous PID file, and then inotify-wait on it to be corrected which we never do
+! systemd-run --unit=mainpidsh3.service -p StandardOutput=tty -p StandardError=tty -p Type=forking -p RuntimeDirectory=mainpidsh3 -p PIDFile=/run/mainpidsh3/pid -p DynamicUser=1 -p TimeoutStartSec=2s /dev/shm/mainpid3.sh
+
+# Test that this failed due to timeout, and not some other error
+test `systemctl show -p Result --value mainpidsh3.service` = timeout
+
+systemd-analyze set-log-level info
+
+echo OK > /testok
+
+exit 0

test/test-functions

@@ -21,7 +21,7 @@ if ! ROOTLIBDIR=$(pkg-config --variable=systemdutildir systemd); then
     ROOTLIBDIR=/usr/lib/systemd
 fi
 
-BASICTOOLS="test sh bash setsid loadkeys setfont login sulogin gzip sleep echo mount umount cryptsetup date dmsetup modprobe sed cmp tee rm true false"
+BASICTOOLS="test sh bash setsid loadkeys setfont login sulogin gzip sleep echo mount umount cryptsetup date dmsetup modprobe sed cmp tee rm true false chmod chown ln"
 DEBUGTOOLS="df free ls stty cat ps ln ip route dmesg dhclient mkdir cp ping dhclient strace less grep id tty touch du sort hostname find"
 
 STATEDIR="${BUILD_DIR:-.}/test/$(basename $(dirname $(realpath $0)))"