json: add builder/dispatcher for PidRef → JSON and back

So far, at the one place we sent a PID over Varlink we did so as a
simple numeric pid_t value. That's of course is racy, since classic PIDs
are recycled too eagerly.

Let's address that, by passing around JSON objects distantly resembling our
PidRef structure. Note that this JSON object does *not* contain the
pidfd, however, but just the pidfd inode number if known.

I originally planned to include the pidfd in some direct form, but I
figured that's not really the best idea, since we always need a
side-channel of some form for that (i.e. AF_UNIX/SCM_RIGHTS), but we
should be able to report about PIDs even without that.

Moreover, while sending the pid number and pidfd id around should always
be OK to do, it's a lot more problematic to always send a pidfd around,
since that implies that fd passing is on and it is OK to install fds
remotely in some IPC peers fd table. For example, when doing a wild dump
of service manager service state we really shouldn't end up with a bunch
of fds installed in our client's fd table.

Hence, all in all I think it is cleaner to define a structure carrying
pid number and pidfd inode id, wich is passed directly as JSON. And then
optionally, in a separate field also pass around a pidfd where it makes
sense.

Note that sending around pidfds is not that beneficial anymore if we
have the pidfd inode id, because we can always securely and reliably get
a pidfd back from a pair of pid + inode id: first we do pidfd_open() on
the pid, and then we check if it is really the right one by comparing
.st_ino after fstat().

This logic is implemented gracefully: if for some reason pidfd/pidfd
inode nrs are not available (too old kernel), we'll fall back to plain
PID numbers.

The dispatching logic knows two distinct levels of validation of the
provided PID data: if SD_JSON_STRICT is specified we'll acquire a pidfd
for the PID, thus verifying it currently exists and failing if it
doesn't. If the flag is not set, well just store the provided info
as-is, will try to acquire a pidfd for it, but not fail if we cannot.
Both modes are important in different contexts.

Also note that in addition to the pidfd inode nr we always store the
current boot ID of the system in the JSON object, since only the
combination of pidfd inode nr and boot ID of the system really is a
world-wide unique reference to a process.

When dispatching a JSON pid field we operate somewhat gracefully: we
either support the triplet structure of pid, pid inode nr, boot id, or
we accept a simple classic UNIX pid.

diff --git a/src/libsystemd/sd-json/json-util.c b/src/libsystemd/sd-json/json-util.c
index c8d67c8d55da788aef1f6a53dc69d6b25b7d4e19..2ff4587565407e6c7ca14e4034b9f5d9e003d553 100644 (file)
--- a/src/libsystemd/sd-json/json-util.c
+++ b/src/libsystemd/sd-json/json-util.c
@@ -5,7 +5,9 @@
 #include "in-addr-util.h"
 #include "iovec-util.h"
 #include "json-util.h"
+#include "parse-util.h"
 #include "path-util.h"
+#include "process-util.h"
 #include "string-util.h"
 #include "user-util.h"
 
@@ -144,3 +146,138 @@ int json_dispatch_path(const char *name, sd_json_variant *variant, sd_json_dispa
 
         return 0;
 }
+
+int json_variant_new_pidref(sd_json_variant **ret, PidRef *pidref) {
+        sd_id128_t boot_id;
+        int r;
+
+        /* Turns a PidRef into a triplet of PID, pidfd inode nr, and the boot ID. The triplet should uniquely
+         * identify the process globally, and be good enough to turn back into a pidfd + PidRef */
+
+        if (!pidref_is_set(pidref))
+                return sd_json_variant_new_null(ret);
+
+        r = pidref_acquire_pidfd_id(pidref);
+        if (r < 0 && !ERRNO_IS_NEG_NOT_SUPPORTED(r) && r != -ENOMEDIUM)
+                return r;
+
+        if (pidref->fd_id > 0) {
+                /* If we have the pidfd inode number, also acquire the boot ID, to make things universally unique */
+                r = sd_id128_get_boot(&boot_id);
+                if (r < 0)
+                        return r;
+        }
+
+        return sd_json_buildo(
+                        ret,
+                        SD_JSON_BUILD_PAIR_INTEGER("pid", pidref->pid),
+                        SD_JSON_BUILD_PAIR_CONDITION(pidref->fd_id > 0, "pidfdId", SD_JSON_BUILD_INTEGER(pidref->fd_id)),
+                        SD_JSON_BUILD_PAIR_CONDITION(pidref->fd_id > 0, "bootId", SD_JSON_BUILD_ID128(boot_id)));
+}
+
+int json_dispatch_pidref(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata) {
+        PidRef *p = ASSERT_PTR(userdata);
+        int r;
+
+        assert(variant);
+
+        /* Turns a JSON PID triplet back into a PidRef, i.e. the reverse of json_variant_new_pidref()
+         * above. If SD_JSON_STRICT is set this will acquire a pidfd for the process, and validate that the
+         * auxiliary fields match it. Otherwise, this will just store the pid and the pidfd inode number (the
+         * latter not if the provided boot id differs from the local one), and not attempt to get a pidfd for
+         * it, or authenticate it. */
+
+        if (sd_json_variant_is_null(variant)) {
+                pidref_done(p);
+                return 0;
+        }
+
+        struct {
+                uint64_t pid, fd_id;
+                sd_id128_t boot_id;
+        } data = {};
+
+        if (sd_json_variant_is_integer(variant))
+                /* Permit a simple classic integer based format */
+                data.pid = sd_json_variant_integer(variant);
+        else if (sd_json_variant_is_string(variant)) {
+                /* As usual, allow integers be encoded as strings too */
+                r = safe_atou64(sd_json_variant_string(variant), &data.pid);
+                if (r < 0)
+                        return json_log(variant, flags, r, "JSON field '%s' is not a numeric PID.", strna(name));
+        } else if (sd_json_variant_is_object(variant)) {
+
+                static const sd_json_dispatch_field dispatch_table[] = {
+                        { "pid",     _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, voffsetof(data, pid),     SD_JSON_MANDATORY },
+                        { "pidfdId", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, voffsetof(data, fd_id),   0                 },
+                        { "bootId",  SD_JSON_VARIANT_STRING,        sd_json_dispatch_id128,  voffsetof(data, boot_id), 0                 },
+                        {}
+                };
+
+                r = sd_json_dispatch(variant, dispatch_table, flags, &data);
+                if (r < 0)
+                        return r;
+        } else
+                return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' is neither a numeric PID nor a PID object.", strna(name));
+
+        /* Before casting the 64bit data.pid field to pid_t, let's ensure it fits the pid_t range. */
+        if (data.pid > PID_T_MAX || !pid_is_valid(data.pid))
+                return json_log(variant, flags, SYNTHETIC_ERRNO(EINVAL), "JSON field '%s' does not contain a valid PID.", strna(name));
+
+        int local_boot_id = -1; /* tristate */
+        if (!sd_id128_is_null(data.boot_id)) {
+                sd_id128_t my_boot_id;
+
+                r = sd_id128_get_boot(&my_boot_id);
+                if (r < 0) {
+                        json_log(variant, flags | (FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : SD_JSON_DEBUG), r, "Unable to get local boot ID to validate JSON field '%s': %m", strna(name));
+                        if (FLAGS_SET(flags, SD_JSON_STRICT))
+                                return r;
+                } else {
+                        local_boot_id = sd_id128_equal(data.boot_id, my_boot_id);
+                        if (!local_boot_id) {
+                                json_log(variant, flags | (FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : SD_JSON_DEBUG), 0, "JSON field '%s' refers to non-local PID.", strna(name));
+                                if (FLAGS_SET(flags, SD_JSON_STRICT))
+                                        return -ESRCH;
+                        }
+                }
+        }
+
+        _cleanup_(pidref_done) PidRef np = PIDREF_NULL;
+        if (local_boot_id != 0) {
+                /* Try to acquire a pidfd – unless this is definitely not a local PID */
+                r = pidref_set_pid(&np, data.pid);
+                if (r < 0) {
+                        json_log(variant, flags | (FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : SD_JSON_DEBUG), r, "Unable to get fd for PID in JSON field '%s': %m", strna(name));
+                        if (FLAGS_SET(flags, SD_JSON_STRICT))
+                                return r;
+                }
+        }
+
+        /* If the the PID is dead or we otherwise can't get a pidfd of it, then store at least the PID number */
+        if (!pidref_is_set(&np))
+                np = PIDREF_MAKE_FROM_PID(data.pid);
+
+        /* If the pidfd inode nr is specified, validate it or at least state */
+        if (data.fd_id > 0) {
+                if (np.fd >= 0) {
+                        r = pidref_acquire_pidfd_id(&np);
+                        if (r < 0 && !ERRNO_IS_NOT_SUPPORTED(r))
+                                return json_log(variant, flags, r, "Unable to get pidfd ID to validate JSON field '%s': %m", strna(name));
+
+                        if (data.fd_id != np.fd_id) {
+                                json_log(variant, flags | (FLAGS_SET(flags, SD_JSON_STRICT) ? 0 : SD_JSON_DEBUG), 0, "JSON field '%s' references PID with non-matching inode number.", strna(name));
+                                if (FLAGS_SET(flags, SD_JSON_STRICT))
+                                        return -ESRCH;
+                        }
+                } else if (local_boot_id != 0) {
+                        json_log(variant, flags|SD_JSON_DEBUG, 0, "Not validating PID inode number on JSON field '%s', because operating without pidfd.", strna(name));
+                        np.fd_id = data.fd_id;
+                }
+        }
+
+        pidref_done(p);
+        *p = TAKE_PIDREF(np);
+
+        return 0;
+}

diff --git a/src/libsystemd/sd-json/json-util.h b/src/libsystemd/sd-json/json-util.h
index 3821e451d4b65a2290d24f16e973162420cdfced..6b72d0d93d2cf8bba8f563fa025c6bc1ae021645 100644 (file)
--- a/src/libsystemd/sd-json/json-util.h
+++ b/src/libsystemd/sd-json/json-util.h
@@ -6,6 +6,7 @@
 #include "sd-json.h"
 
 #include "macro.h"
+#include "pidref.h"
 
 #define JSON_VARIANT_REPLACE(v, q)        \
         do {                              \
@@ -112,6 +113,7 @@ int json_dispatch_user_group_name(const char *name, sd_json_variant *variant, sd
 int json_dispatch_const_user_group_name(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata);
 int json_dispatch_in_addr(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata);
 int json_dispatch_path(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata);
+int json_dispatch_pidref(const char *name, sd_json_variant *variant, sd_json_dispatch_flags_t flags, void *userdata);
 
 static inline int json_variant_unbase64_iovec(sd_json_variant *v, struct iovec *ret) {
         return sd_json_variant_unbase64(v, ret ? &ret->iov_base : NULL, ret ? &ret->iov_len : NULL);
@@ -144,6 +146,7 @@ enum {
         _JSON_BUILD_DUAL_TIMESTAMP,
         _JSON_BUILD_RATELIMIT,
         _JSON_BUILD_TRISTATE,
+        _JSON_BUILD_PIDREF,
 
         _JSON_BUILD_PAIR_INTEGER_NON_ZERO,
         _JSON_BUILD_PAIR_INTEGER_NON_NEGATIVE,
@@ -168,6 +171,7 @@ enum {
         _JSON_BUILD_PAIR_HEX_NON_EMPTY,
         _JSON_BUILD_PAIR_OCTESCAPE_NON_EMPTY,
         _JSON_BUILD_PAIR_TRISTATE_NON_NULL,
+        _JSON_BUILD_PAIR_PIDREF_NON_NULL,
 
         _SD_JSON_BUILD_REALLYMAX,
 };
@@ -187,6 +191,7 @@ enum {
 #define JSON_BUILD_DUAL_TIMESTAMP(t) _JSON_BUILD_DUAL_TIMESTAMP, (dual_timestamp*) { t }
 #define JSON_BUILD_RATELIMIT(rl) _JSON_BUILD_RATELIMIT, (const RateLimit*) { rl }
 #define JSON_BUILD_TRISTATE(i) _JSON_BUILD_TRISTATE, (int) { i }
+#define JSON_BUILD_PIDREF(p) _JSON_BUILD_PIDREF, (const PidRef*) { p }
 
 #define JSON_BUILD_PAIR_INTEGER_NON_ZERO(name, i) _JSON_BUILD_PAIR_INTEGER_NON_ZERO, (const char*) { name }, (int64_t) { i }
 #define JSON_BUILD_PAIR_INTEGER_NON_NEGATIVE(name, i) _JSON_BUILD_PAIR_INTEGER_NON_NEGATIVE, (const char*) { name }, (int64_t) { i }
@@ -210,6 +215,7 @@ enum {
 #define JSON_BUILD_PAIR_HEX_NON_EMPTY(name, v, n) _JSON_BUILD_PAIR_HEX_NON_EMPTY, (const char*) { name }, (const void*) { v }, (size_t) { n }
 #define JSON_BUILD_PAIR_OCTESCAPE_NON_EMPTY(name, v, n) _JSON_BUILD_PAIR_HEX_NON_EMPTY, (const char*) { name }, (const void*) { v }, (size_t) { n }
 #define JSON_BUILD_PAIR_TRISTATE_NON_NULL(name, i) _JSON_BUILD_PAIR_TRISTATE_NON_NULL, (const char*) { name }, (int) { i }
+#define JSON_BUILD_PAIR_PIDREF_NON_NULL(name, p) _JSON_BUILD_PAIR_PIDREF_NON_NULL, (const char*) { name }, (const PidRef*) { p }
 
 #define JSON_BUILD_PAIR_IOVEC_BASE64(name, iov) SD_JSON_BUILD_PAIR(name, JSON_BUILD_IOVEC_BASE64(iov))
 #define JSON_BUILD_PAIR_IOVEC_HEX(name, iov) SD_JSON_BUILD_PAIR(name, JSON_BUILD_IOVEC_HEX(iov))
@@ -223,3 +229,6 @@ enum {
 #define JSON_BUILD_PAIR_DUAL_TIMESTAMP(name, t) SD_JSON_BUILD_PAIR(name, JSON_BUILD_DUAL_TIMESTAMP(t))
 #define JSON_BUILD_PAIR_RATELIMIT(name, rl) SD_JSON_BUILD_PAIR(name, JSON_BUILD_RATELIMIT(rl))
 #define JSON_BUILD_PAIR_TRISTATE(name, i) SD_JSON_BUILD_PAIR(name, JSON_BUILD_TRISTATE(i))
+#define JSON_BUILD_PAIR_PIDREF(name, p) SD_JSON_BUILD_PAIR(name, JSON_BUILD_PIDREF(p))
+
+int json_variant_new_pidref(sd_json_variant **ret, PidRef *pidref);

diff --git a/src/libsystemd/sd-json/sd-json.c b/src/libsystemd/sd-json/sd-json.c
index 33ccdcac5aad0f4e5ac5dbfabd5a8e881637f65d..03ef3ef0a86f698839d09b8c2946aef44472ca6b 100644 (file)
--- a/src/libsystemd/sd-json/sd-json.c
+++ b/src/libsystemd/sd-json/sd-json.c
@@ -4194,6 +4194,34 @@ _public_ int sd_json_buildv(sd_json_variant **ret, va_list ap) {
                         break;
                 }
 
+                case _JSON_BUILD_PIDREF: {
+                        PidRef *pidref;
+
+                        if (!IN_SET(current->expect, EXPECT_TOPLEVEL, EXPECT_OBJECT_VALUE, EXPECT_ARRAY_ELEMENT)) {
+                                r = -EINVAL;
+                                goto finish;
+                        }
+
+                        pidref = va_arg(ap, PidRef*);
+
+                        if (current->n_suppress == 0) {
+                                r = json_variant_new_pidref(&add, pidref);
+                                if (r < 0)
+                                        goto finish;
+                        }
+
+                        n_subtract = 1;
+
+                        if (current->expect == EXPECT_TOPLEVEL)
+                                current->expect = EXPECT_END;
+                        else if (current->expect == EXPECT_OBJECT_VALUE)
+                                current->expect = EXPECT_OBJECT_KEY;
+                        else
+                                assert(current->expect == EXPECT_ARRAY_ELEMENT);
+
+                        break;
+                }
+
                 case _JSON_BUILD_TRISTATE: {
                         int tristate;
 
@@ -4769,6 +4797,34 @@ _public_ int sd_json_buildv(sd_json_variant **ret, va_list ap) {
                         break;
                 }
 
+                case _JSON_BUILD_PAIR_PIDREF_NON_NULL: {
+                        PidRef *p;
+                        const char *n;
+
+                        if (current->expect != EXPECT_OBJECT_KEY) {
+                                r = -EINVAL;
+                                goto finish;
+                        }
+
+                        n = va_arg(ap, const char*);
+                        p = va_arg(ap, PidRef*);
+
+                        if (pidref_is_set(p) && current->n_suppress == 0) {
+                                r = sd_json_variant_new_string(&add, n);
+                                if (r < 0)
+                                        goto finish;
+
+                                r = json_variant_new_pidref(&add_more, p);
+                                if (r < 0)
+                                        goto finish;
+                        }
+
+                        n_subtract = 2; /* we generated two items */
+
+                        current->expect = EXPECT_OBJECT_KEY;
+                        break;
+                }
+
                 case _JSON_BUILD_PAIR_CALLBACK_NON_NULL: {
                         sd_json_build_callback_t cb;
                         void *userdata;

diff --git a/src/test/test-json.c b/src/test/test-json.c
index 378a3d0d8de69d24fad320b5c7d65f4f73183a43..32cd2858412bac8c09cea62e1fa042ad681e049e 100644 (file)
--- a/src/test/test-json.c
+++ b/src/test/test-json.c
@@ -1264,4 +1264,47 @@ TEST(parse_continue) {
         assert_se(sd_json_parse_with_source_continue(&p, "piff", /* flags= */ 0, &x, &line, &column) == -EINVAL);
 }
 
+TEST(pidref) {
+        _cleanup_(pidref_done) PidRef myself = PIDREF_NULL, pid1 = PIDREF_NULL;
+
+        assert_se(pidref_set_pid(&myself, 0) >= 0);
+        assert_se(pidref_set_pid(&pid1, 1) >= 0);
+
+        _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL;
+        assert_se(sd_json_buildo(&v,
+                                 JSON_BUILD_PAIR_PIDREF("myself", &myself),
+                                 JSON_BUILD_PAIR_PIDREF("pid1", &pid1)) >= 0);
+
+        sd_json_variant_dump(v, SD_JSON_FORMAT_COLOR|SD_JSON_FORMAT_PRETTY, NULL, NULL);
+
+        struct {
+                PidRef myself, pid1;
+        } data = {
+                .myself = PIDREF_NULL,
+                .pid1 = PIDREF_NULL,
+        };
+
+        assert_se(sd_json_dispatch(
+                                  v,
+                                  (const sd_json_dispatch_field[]) {
+                                          { "myself", _SD_JSON_VARIANT_TYPE_INVALID, json_dispatch_pidref, voffsetof(data, myself), 0 },
+                                          { "pid1", _SD_JSON_VARIANT_TYPE_INVALID,   json_dispatch_pidref, voffsetof(data, pid1),   0 },
+                                          {},
+                                  },
+                                  /* flags= */ 0,
+                                  &data) >= 0);
+
+        assert_se(pidref_equal(&myself, &data.myself));
+        assert_se(pidref_equal(&pid1, &data.pid1));
+
+        assert_se(!pidref_equal(&myself, &data.pid1));
+        assert_se(!pidref_equal(&pid1, &data.myself));
+
+        assert_se((myself.fd_id > 0) == (data.myself.fd_id > 0));
+        assert_se((pid1.fd_id > 0) == (data.pid1.fd_id > 0));
+
+        pidref_done(&data.myself);
+        pidref_done(&data.pid1);
+}
+
 DEFINE_TEST_MAIN(LOG_DEBUG);