core: pin notify sender through pidfd (potentially SCM_PIDFD)

diff --git a/TODO b/TODO
index e375dd1f4f86fb8e566bec44a27735fa6b78e017..2d37bc671fcf8b41f17bae7f2789ca895ca20363 100644 (file)
--- a/TODO
+++ b/TODO
@@ -579,7 +579,6 @@ Features:
   - cg_pid_get_xyz()
   - pid_from_same_root_fs()
   - get_ctty_devnr()
-  - pid1: sd_notify() receiver should use SCM_PIDFD to authenticate client
   - actually wait for POLLIN on pidref's pidfd in service logic
   - openpt_allocate_in_namespace()
   - unit_attach_pid_to_cgroup_via_bus()

diff --git a/src/core/manager.c b/src/core/manager.c
index b8de50c2c04dd4892096b123f5ec5d46b384f8af..39fbbfa4709eff85dca0d6a9f16fae1a44490057 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1104,6 +1104,11 @@ static int manager_setup_notify(Manager *m) {
                 if (r < 0)
                         return log_error_errno(r, "Failed to enable SO_PASSCRED for notify socket: %m");
 
+                r = setsockopt_int(fd, SOL_SOCKET, SO_PASSPIDFD, true);
+                if (r < 0 && r != -ENOPROTOOPT)
+                        log_warning_errno(r, "Failed to enable SO_PASSPIDFD for notify socket, ignoring: %m");
+                // TODO: maybe enforce SO_PASSPIDFD?
+
                 m->notify_fd = TAKE_FD(fd);
 
                 log_debug("Using notification socket %s", m->notify_socket);
@@ -2638,13 +2643,16 @@ static bool manager_process_barrier_fd(char * const *tags, FDSet *fds) {
 static void manager_invoke_notify_message(
                 Manager *m,
                 Unit *u,
+                PidRef *pidref,
                 const struct ucred *ucred,
                 char * const *tags,
                 FDSet *fds) {
 
         assert(m);
         assert(u);
+        assert(pidref_is_set(pidref));
         assert(ucred);
+        assert(pidref->pid == ucred->pid);
         assert(tags);
 
         if (u->notifygen == m->notifygen) /* Already invoked on this same unit in this same iteration? */
@@ -2652,7 +2660,7 @@ static void manager_invoke_notify_message(
         u->notifygen = m->notifygen;
 
         if (UNIT_VTABLE(u)->notify_message)
-                UNIT_VTABLE(u)->notify_message(u, ucred, tags, fds);
+                UNIT_VTABLE(u)->notify_message(u, pidref, ucred, tags, fds);
 
         else if (DEBUG_LOGGING) {
                 _cleanup_free_ char *joined = strv_join(tags, ", ");
@@ -2723,7 +2731,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
                 .iov_base = buf,
                 .iov_len = sizeof(buf)-1,
         };
-        CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) +
+        CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct ucred)) + CMSG_SPACE(sizeof(int)) /* SCM_PIDFD */ +
                          CMSG_SPACE(sizeof(int) * NOTIFY_FD_MAX)) control;
         struct msghdr msghdr = {
                 .msg_iov = &iovec,
@@ -2755,6 +2763,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
                  * socket. */
                 return log_error_errno(n, "Failed to receive notification message: %m");
 
+        _cleanup_close_ int pidfd = -EBADF;
         struct ucred *ucred = NULL;
         int *fd_array = NULL;
         size_t n_fds = 0;
@@ -2773,6 +2782,9 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
 
                         assert(!ucred);
                         ucred = CMSG_TYPED_DATA(cmsg, struct ucred);
+                } else if (cmsg->cmsg_type == SCM_PIDFD) {
+                        assert(pidfd < 0);
+                        pidfd = *CMSG_TYPED_DATA(cmsg, int);
                 }
         }
 
@@ -2794,6 +2806,28 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
                 return 0;
         }
 
+        _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
+
+        if (pidfd >= 0)
+                r = pidref_set_pidfd_consume(&pidref, TAKE_FD(pidfd));
+        else
+                r = pidref_set_pid(&pidref, ucred->pid);
+        if (r < 0) {
+                if (r == -ESRCH)
+                        log_debug_errno(r, "Notify sender died before message is processed. Ignoring.");
+                else
+                        log_warning_errno(r, "Failed to pin notify sender, ignoring message: %m");
+                return 0;
+        }
+
+        if (pidref.pid != ucred->pid) {
+                assert(pidref.fd >= 0);
+
+                log_warning("Got SCM_PIDFD for process " PID_FMT " but SCM_CREDENTIALS for process " PID_FMT ". Ignoring.",
+                            pidref.pid, ucred->pid);
+                return 0;
+        }
+
         if ((size_t) n >= sizeof(buf) || (msghdr.msg_flags & MSG_TRUNC)) {
                 log_warning("Received notify message exceeded maximum size. Ignoring.");
                 return 0;
@@ -2824,9 +2858,6 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
         /* Increase the generation counter used for filtering out duplicate unit invocations. */
         m->notifygen++;
 
-        /* Generate lookup key from the PID (we have no pidfd here, after all) */
-        PidRef pidref = PIDREF_MAKE_FROM_PID(ucred->pid);
-
         /* Notify every unit that might be interested, which might be multiple. */
         _cleanup_free_ Unit **array = NULL;
 
@@ -2841,7 +2872,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
                 /* And now invoke the per-unit callbacks. Note that manager_invoke_notify_message() will handle
                  * duplicate units – making sure we only invoke each unit's handler once. */
                 FOREACH_ARRAY(u, array, n_array)
-                        manager_invoke_notify_message(m, *u, ucred, tags, fds);
+                        manager_invoke_notify_message(m, *u, &pidref, ucred, tags, fds);
 
         if (!fdset_isempty(fds))
                 log_warning("Got extra auxiliary fds with notification message, closing them.");

diff --git a/src/core/service.c b/src/core/service.c
index b4d283686c8e42e4cf167faafb35a980d3688485..4403b209bdeab81acbef243837fb6893afde8c39 100644 (file)
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -4341,44 +4341,44 @@ static void service_force_watchdog(Service *s) {
         service_enter_signal(s, SERVICE_STOP_WATCHDOG, SERVICE_FAILURE_WATCHDOG);
 }
 
-static bool service_notify_message_authorized(Service *s, pid_t pid) {
+static bool service_notify_message_authorized(Service *s, PidRef *pid) {
         assert(s);
-        assert(pid_is_valid(pid));
+        assert(pidref_is_set(pid));
 
         switch (service_get_notify_access(s)) {
 
         case NOTIFY_NONE:
                 /* Warn level only if no notifications are expected */
-                log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception is disabled", pid);
+                log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception is disabled", pid->pid);
                 return false;
 
         case NOTIFY_ALL:
                 return true;
 
         case NOTIFY_MAIN:
-                if (pid == s->main_pid.pid)
+                if (pidref_equal(pid, &s->main_pid))
                         return true;
 
                 if (pidref_is_set(&s->main_pid))
-                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid.pid);
+                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid->pid, s->main_pid.pid);
                 else
-                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid);
+                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid->pid);
 
                 return false;
 
         case NOTIFY_EXEC:
-                if (pid == s->main_pid.pid || pid == s->control_pid.pid)
+                if (pidref_equal(pid, &s->main_pid) || pidref_equal(pid, &s->control_pid))
                         return true;
 
                 if (pidref_is_set(&s->main_pid) && pidref_is_set(&s->control_pid))
                         log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT" and control PID "PID_FMT,
-                                       pid, s->main_pid.pid, s->control_pid.pid);
+                                       pid->pid, s->main_pid.pid, s->control_pid.pid);
                 else if (pidref_is_set(&s->main_pid))
-                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid.pid);
+                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid->pid, s->main_pid.pid);
                 else if (pidref_is_set(&s->control_pid))
-                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for control PID "PID_FMT, pid, s->control_pid.pid);
+                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for control PID "PID_FMT, pid->pid, s->control_pid.pid);
                 else
-                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID and control PID which are currently not known", pid);
+                        log_unit_debug(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID and control PID which are currently not known", pid->pid);
 
                 return false;
 
@@ -4389,6 +4389,7 @@ static bool service_notify_message_authorized(Service *s, pid_t pid) {
 
 static void service_notify_message(
                 Unit *u,
+                PidRef *pidref,
                 const struct ucred *ucred,
                 char * const *tags,
                 FDSet *fds) {
@@ -4396,14 +4397,15 @@ static void service_notify_message(
         Service *s = ASSERT_PTR(SERVICE(u));
         int r;
 
+        assert(pidref_is_set(pidref));
         assert(ucred);
 
-        if (!service_notify_message_authorized(s, ucred->pid))
+        if (!service_notify_message_authorized(s, pidref))
                 return;
 
         if (DEBUG_LOGGING) {
                 _cleanup_free_ char *cc = strv_join(tags, ", ");
-                log_unit_debug(u, "Got notification message from PID "PID_FMT": %s", ucred->pid, empty_to_na(cc));
+                log_unit_debug(u, "Got notification message from PID "PID_FMT": %s", pidref->pid, empty_to_na(cc));
         }
 
         usec_t monotonic_usec = USEC_INFINITY;

diff --git a/src/core/unit.h b/src/core/unit.h
index 31a1a13df12ca262272fa933d4c6478449061eb6..4c900430b40bf068fa4a8cd366f080ed94282050 100644 (file)
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -630,7 +630,7 @@ typedef struct UnitVTable {
         void (*notify_cgroup_oom)(Unit *u, bool managed_oom);
 
         /* Called whenever a process of this unit sends us a message */
-        void (*notify_message)(Unit *u, const struct ucred *ucred, char * const *tags, FDSet *fds);
+        void (*notify_message)(Unit *u, PidRef *pidref, const struct ucred *ucred, char * const *tags, FDSet *fds);
 
         /* Called whenever we learn a handoff timestamp */
         void (*notify_handoff_timestamp)(Unit *u, const struct ucred *ucred, const dual_timestamp *ts);