Merge pull request #30962 from poettering/varlink-json-sensitive-rework

varlink/json: rework "sensitive" logic

src/creds/creds.c

@@ -996,8 +996,6 @@ static int vl_method_encrypt(Varlink *link, JsonVariant *parameters, VarlinkMeth
 
         assert(link);
 
-        json_variant_sensitive(parameters);
-
         r = varlink_dispatch(link, parameters, dispatch_table, &p);
         if (r != 0)
                 return r;
@@ -1079,9 +1077,6 @@ static int vl_method_decrypt(Varlink *link, JsonVariant *parameters, VarlinkMeth
 
         assert(link);
 
-        /* Let's also mark the (theoretically encrypted) input as sensitive, in case the NULL encryption scheme was used. */
-        json_variant_sensitive(parameters);
-
         r = varlink_dispatch(link, parameters, dispatch_table, &p);
         if (r != 0)
                 return r;
@@ -1144,7 +1139,7 @@ static int run(int argc, char *argv[]) {
 
                 /* Invocation as Varlink service */
 
-                r = varlink_server_new(&varlink_server, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA);
+                r = varlink_server_new(&varlink_server, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA|VARLINK_SERVER_INPUT_SENSITIVE);
                 if (r < 0)
                         return log_error_errno(r, "Failed to allocate Varlink server: %m");
 

src/home/homed-manager.c

@@ -998,7 +998,7 @@ static int manager_bind_varlink(Manager *m) {
         assert(m);
         assert(!m->varlink_server);
 
-        r = varlink_server_new(&m->varlink_server, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA);
+        r = varlink_server_new(&m->varlink_server, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA|VARLINK_SERVER_INPUT_SENSITIVE);
         if (r < 0)
                 return log_error_errno(r, "Failed to allocate varlink server object: %m");
 

src/shared/json.c

@@ -89,6 +89,9 @@ struct JsonVariant {
         /* Erase from memory when freeing */
         bool sensitive:1;
 
+        /* True if we know that any referenced json object is marked sensitive */
+        bool recursive_sensitive:1;
+
         /* If this is an object the fields are strictly ordered by name */
         bool sorted:1;
 
@@ -1453,6 +1456,33 @@ bool json_variant_is_sensitive(JsonVariant *v) {
         return v->sensitive;
 }
 
+bool json_variant_is_sensitive_recursive(JsonVariant *v) {
+        if (!v)
+                return false;
+        if (json_variant_is_sensitive(v))
+                return true;
+        if (!json_variant_is_regular(v))
+                return false;
+        if (v->recursive_sensitive) /* Already checked this before */
+                return true;
+        if (!IN_SET(v->type, JSON_VARIANT_ARRAY, JSON_VARIANT_OBJECT))
+                return false;
+        if (v->is_reference) {
+                if (!json_variant_is_sensitive_recursive(v->reference))
+                        return false;
+
+                return (v->recursive_sensitive = true);
+        }
+
+        for (size_t i = 0; i < json_variant_elements(v); i++)
+                if (json_variant_is_sensitive_recursive(json_variant_by_index(v, i)))
+                        return (v->recursive_sensitive = true);
+
+        /* Note: we only cache the result here in case true, since we allow all elements down the tree to
+         * have their sensitive flag toggled later on (but never off) */
+        return false;
+}
+
 static void json_variant_propagate_sensitive(JsonVariant *from, JsonVariant *to) {
         if (json_variant_is_sensitive(from))
                 json_variant_sensitive(to);
@@ -1582,6 +1612,15 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha
         assert(f);
         assert(v);
 
+        if (FLAGS_SET(flags, JSON_FORMAT_CENSOR_SENSITIVE) && json_variant_is_sensitive(v)) {
+                if (flags & JSON_FORMAT_COLOR)
+                        fputs(ansi_red(), f);
+                fputs("\"<sensitive data>\"", f);
+                if (flags & JSON_FORMAT_COLOR)
+                        fputs(ANSI_NORMAL, f);
+                return 0;
+        }
+
         switch (json_variant_type(v)) {
 
         case JSON_VARIANT_REAL: {
@@ -1773,25 +1812,6 @@ static int json_format(FILE *f, JsonVariant *v, JsonFormatFlags flags, const cha
         return 0;
 }
 
-static bool json_variant_is_sensitive_recursive(JsonVariant *v) {
-        if (!v)
-                return false;
-        if (json_variant_is_sensitive(v))
-                return true;
-        if (!json_variant_is_regular(v))
-                return false;
-        if (!IN_SET(v->type, JSON_VARIANT_ARRAY, JSON_VARIANT_OBJECT))
-                return false;
-        if (v->is_reference)
-                return json_variant_is_sensitive_recursive(v->reference);
-
-        for (size_t i = 0; i < json_variant_elements(v); i++)
-                if (json_variant_is_sensitive_recursive(json_variant_by_index(v, i)))
-                        return true;
-
-        return false;
-}
-
 int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret) {
         _cleanup_(memstream_done) MemStream m = {};
         size_t sz;
@@ -1807,10 +1827,6 @@ int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret) {
         if (flags & JSON_FORMAT_OFF)
                 return -ENOEXEC;
 
-        if ((flags & JSON_FORMAT_REFUSE_SENSITIVE))
-                if (json_variant_is_sensitive_recursive(v))
-                        return -EPERM;
-
         f = memstream_init(&m);
         if (!f)
                 return -ENOMEM;

src/shared/json.h

@@ -155,6 +155,7 @@ bool json_variant_equal(JsonVariant *a, JsonVariant *b);
 
 void json_variant_sensitive(JsonVariant *v);
 bool json_variant_is_sensitive(JsonVariant *v);
+bool json_variant_is_sensitive_recursive(JsonVariant *v);
 
 struct json_variant_foreach_state {
         JsonVariant *variant;
@@ -196,7 +197,7 @@ typedef enum JsonFormatFlags {
         JSON_FORMAT_FLUSH            = 1 << 8, /* call fflush() after dumping JSON */
         JSON_FORMAT_EMPTY_ARRAY      = 1 << 9, /* output "[]" for empty input */
         JSON_FORMAT_OFF              = 1 << 10, /* make json_variant_format() fail with -ENOEXEC */
-        JSON_FORMAT_REFUSE_SENSITIVE = 1 << 11, /* return EPERM if any node in the tree is marked as senstitive */
+        JSON_FORMAT_CENSOR_SENSITIVE = 1 << 11, /* Replace all sensitive elements with the string "<sensitive data>" */
 } JsonFormatFlags;
 
 int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret);

src/shared/varlink.c

@@ -184,6 +184,7 @@ struct Varlink {
         bool allow_fd_passing_output:1;
 
         bool output_buffer_sensitive:1; /* whether to erase the output buffer after writing it to the socket */
+        bool input_sensitive:1; /* Whether incoming messages might be sensitive */
 
         int af; /* address family if socket; AF_UNSPEC if not socket; negative if not known */
 
@@ -703,7 +704,7 @@ static void varlink_clear(Varlink *v) {
 
         varlink_clear_current(v);
 
-        v->input_buffer = mfree(v->input_buffer);
+        v->input_buffer = v->input_sensitive ? erase_and_free(v->input_buffer) : mfree(v->input_buffer);
         v->output_buffer = v->output_buffer_sensitive ? erase_and_free(v->output_buffer) : mfree(v->output_buffer);
 
         v->input_control_buffer = mfree(v->input_control_buffer);
@@ -1022,7 +1023,8 @@ static int varlink_read(Varlink *v) {
 }
 
 static int varlink_parse_message(Varlink *v) {
-        const char *e, *begin;
+        const char *e;
+        char *begin;
         size_t sz;
         int r;
 
@@ -1047,6 +1049,8 @@ static int varlink_parse_message(Varlink *v) {
         sz = e - begin + 1;
 
         r = json_parse(begin, 0, &v->current, NULL, NULL);
+        if (v->input_sensitive)
+                explicit_bzero_safe(begin, sz);
         if (r < 0) {
                 /* If we encounter a parse failure flush all data. We cannot possibly recover from this,
                  * hence drop all buffered data now. */
@@ -1054,6 +1058,24 @@ static int varlink_parse_message(Varlink *v) {
                 return varlink_log_errno(v, r, "Failed to parse JSON: %m");
         }
 
+        if (v->input_sensitive) {
+                /* Mark the parameters subfield as sensitive right-away, if that's requested */
+                JsonVariant *parameters = json_variant_by_key(v->current, "parameters");
+                if (parameters)
+                        json_variant_sensitive(parameters);
+        }
+
+        if (DEBUG_LOGGING) {
+                _cleanup_(erase_and_freep) char *censored_text = NULL;
+
+                /* Suppress sensitive fields in the debug output */
+                r = json_variant_format(v->current, /* flags= */ JSON_FORMAT_CENSOR_SENSITIVE, &censored_text);
+                if (r < 0)
+                        return r;
+
+                varlink_log(v, "Received message: %s", censored_text);
+        }
+
         v->input_buffer_size -= sz;
 
         if (v->input_buffer_size == 0)
@@ -1849,56 +1871,60 @@ Varlink* varlink_flush_close_unref(Varlink *v) {
 
 static int varlink_format_json(Varlink *v, JsonVariant *m) {
         _cleanup_(erase_and_freep) char *text = NULL;
-        bool sensitive = false;
+        int sz, r;
-        int r;
 
         assert(v);
         assert(m);
 
-        r = json_variant_format(m, JSON_FORMAT_REFUSE_SENSITIVE, &text);
+        sz = json_variant_format(m, /* flags= */ 0, &text);
-        if (r == -EPERM) {
+        if (sz < 0)
-                sensitive = true;
+                return sz;
-                r = json_variant_format(m, /* flags= */ 0, &text);
+        assert(text[sz] == '\0');
-        }
-        if (r < 0)
-                return r;
-        assert(text[r] == '\0');
 
-        if (v->output_buffer_size + r + 1 > VARLINK_BUFFER_MAX)
+        if (v->output_buffer_size + sz + 1 > VARLINK_BUFFER_MAX)
                 return -ENOBUFS;
 
-        varlink_log(v, "Sending message: %s", sensitive ? "<sensitive data>" : text);
+        if (DEBUG_LOGGING) {
+                _cleanup_(erase_and_freep) char *censored_text = NULL;
+
+                /* Suppress sensitive fields in the debug output */
+                r = json_variant_format(m, /* flags= */ JSON_FORMAT_CENSOR_SENSITIVE, &censored_text);
+                if (r < 0)
+                        return r;
+
+                varlink_log(v, "Sending message: %s", censored_text);
+        }
 
         if (v->output_buffer_size == 0) {
 
                 free_and_replace(v->output_buffer, text);
 
-                v->output_buffer_size = r + 1;
+                v->output_buffer_size = sz + 1;
                 v->output_buffer_index = 0;
 
         } else if (v->output_buffer_index == 0) {
 
-                if (!GREEDY_REALLOC(v->output_buffer, v->output_buffer_size + r + 1))
+                if (!GREEDY_REALLOC(v->output_buffer, v->output_buffer_size + sz + 1))
                         return -ENOMEM;
 
-                memcpy(v->output_buffer + v->output_buffer_size, text, r + 1);
+                memcpy(v->output_buffer + v->output_buffer_size, text, sz + 1);
-                v->output_buffer_size += r + 1;
+                v->output_buffer_size += sz + 1;
         } else {
                 char *n;
-                const size_t new_size = v->output_buffer_size + r + 1;
+                const size_t new_size = v->output_buffer_size + sz + 1;
 
                 n = new(char, new_size);
                 if (!n)
                         return -ENOMEM;
 
-                memcpy(mempcpy(n, v->output_buffer + v->output_buffer_index, v->output_buffer_size), text, r + 1);
+                memcpy(mempcpy(n, v->output_buffer + v->output_buffer_index, v->output_buffer_size), text, sz + 1);
 
                 free_and_replace(v->output_buffer, n);
                 v->output_buffer_size = new_size;
                 v->output_buffer_index = 0;
         }
 
-        if (sensitive)
+        if (json_variant_is_sensitive_recursive(m))
                 v->output_buffer_sensitive = true; /* Propagate sensitive flag */
         else
                 text = mfree(text); /* No point in the erase_and_free() destructor declared above */
@@ -3097,6 +3123,13 @@ int varlink_set_allow_fd_passing_output(Varlink *v, bool b) {
         return 0;
 }
 
+int varlink_set_input_sensitive(Varlink *v) {
+        assert_return(v, -EINVAL);
+
+        v->input_sensitive = true;
+        return 0;
+}
+
 int varlink_server_new(VarlinkServer **ret, VarlinkServerFlags flags) {
         _cleanup_(varlink_server_unrefp) VarlinkServer *s = NULL;
         int r;
@@ -3325,6 +3358,9 @@ static int connect_callback(sd_event_source *source, int fd, uint32_t revents, v
 
         TAKE_FD(cfd);
 
+        if (FLAGS_SET(ss->server->flags, VARLINK_SERVER_INPUT_SENSITIVE))
+                varlink_set_input_sensitive(v);
+
         if (ss->server->connect_callback) {
                 r = ss->server->connect_callback(ss->server, v, ss->server->userdata);
                 if (r < 0) {

src/shared/varlink.h

@@ -47,7 +47,8 @@ typedef enum VarlinkServerFlags {
         VARLINK_SERVER_MYSELF_ONLY      = 1 << 1, /* Only accessible by our own UID */
         VARLINK_SERVER_ACCOUNT_UID      = 1 << 2, /* Do per user accounting */
         VARLINK_SERVER_INHERIT_USERDATA = 1 << 3, /* Initialize Varlink connection userdata from VarlinkServer userdata */
-        _VARLINK_SERVER_FLAGS_ALL = (1 << 4) - 1,
+        VARLINK_SERVER_INPUT_SENSITIVE  = 1 << 4, /* Automatically mark al connection input as sensitive */
+        _VARLINK_SERVER_FLAGS_ALL = (1 << 5) - 1,
 } VarlinkServerFlags;
 
 typedef int (*VarlinkMethod)(Varlink *link, JsonVariant *parameters, VarlinkMethodFlags flags, void *userdata);
@@ -154,6 +155,9 @@ VarlinkServer* varlink_get_server(Varlink *v);
 
 int varlink_set_description(Varlink *v, const char *d);
 
+/* Automatically mark the parameters part of incoming messages as security sensitive */
+int varlink_set_input_sensitive(Varlink *v);
+
 /* Create a varlink server */
 int varlink_server_new(VarlinkServer **ret, VarlinkServerFlags flags);
 VarlinkServer *varlink_server_ref(VarlinkServer *s);

src/test/test-json.c

@@ -107,9 +107,9 @@ static void test_variant_one(const char *data, Test test) {
         assert_se(json_variant_equal(v, w));
 
         s = mfree(s);
-        r = json_variant_format(w, JSON_FORMAT_REFUSE_SENSITIVE, &s);
+        r = json_variant_format(w, JSON_FORMAT_CENSOR_SENSITIVE, &s);
-        assert_se(r == -EPERM);
+        assert_se(s);
-        assert_se(!s);
+        assert_se(streq_ptr(s, "\"<sensitive data>\""));
 
         s = mfree(s);
         r = json_variant_format(w, JSON_FORMAT_PRETTY, &s);
@@ -886,10 +886,11 @@ TEST(json_sensitive) {
 
         json_variant_sensitive(a);
 
-        assert_se(json_variant_format(a, JSON_FORMAT_REFUSE_SENSITIVE, &s) == -EPERM);
+        assert_se(json_variant_format(a, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0);
-        assert_se(!s);
+        assert_se(streq_ptr(s, "\"<sensitive data>\""));
+        s = mfree(s);
 
-        r = json_variant_format(b, JSON_FORMAT_REFUSE_SENSITIVE, &s);
+        r = json_variant_format(b, JSON_FORMAT_CENSOR_SENSITIVE, &s);
         assert_se(r >= 0);
         assert_se(s);
         assert_se((size_t) r == strlen(s));
@@ -901,7 +902,7 @@ TEST(json_sensitive) {
                                              JSON_BUILD_PAIR("e", JSON_BUILD_EMPTY_OBJECT))) >= 0);
         json_variant_dump(v, JSON_FORMAT_COLOR|JSON_FORMAT_PRETTY, NULL, NULL);
 
-        r = json_variant_format(v, JSON_FORMAT_REFUSE_SENSITIVE, &s);
+        r = json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s);
         assert_se(r >= 0);
         assert_se(s);
         assert_se((size_t) r == strlen(s));
@@ -915,7 +916,7 @@ TEST(json_sensitive) {
                                              JSON_BUILD_PAIR("e", JSON_BUILD_EMPTY_OBJECT))) >= 0);
         json_variant_dump(v, JSON_FORMAT_COLOR|JSON_FORMAT_PRETTY, NULL, NULL);
 
-        r = json_variant_format(v, JSON_FORMAT_REFUSE_SENSITIVE, &s);
+        r = json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s);
         assert_se(r >= 0);
         assert_se(s);
         assert_se((size_t) r == strlen(s));
@@ -930,8 +931,9 @@ TEST(json_sensitive) {
                                              JSON_BUILD_PAIR("e", JSON_BUILD_EMPTY_OBJECT))) >= 0);
         json_variant_dump(v, JSON_FORMAT_COLOR|JSON_FORMAT_PRETTY, NULL, NULL);
 
-        assert_se(json_variant_format(v, JSON_FORMAT_REFUSE_SENSITIVE, &s) == -EPERM);
+        assert_se(json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0);
-        assert_se(!s);
+        assert_se(streq_ptr(s, "{\"b\":[\"foo\",\"bar\",\"baz\",\"qux\"],\"a\":\"<sensitive data>\",\"c\":-9223372036854775808,\"d\":\"-9223372036854775808\",\"e\":{}}"));
+        s = mfree(s);
         v = json_variant_unref(v);
 
         assert_se(json_build(&v, JSON_BUILD_OBJECT(
@@ -942,8 +944,9 @@ TEST(json_sensitive) {
                                              JSON_BUILD_PAIR("e", JSON_BUILD_EMPTY_OBJECT))) >= 0);
         json_variant_dump(v, JSON_FORMAT_COLOR|JSON_FORMAT_PRETTY, NULL, NULL);
 
-        assert_se(json_variant_format(v, JSON_FORMAT_REFUSE_SENSITIVE, &s) == -EPERM);
+        assert_se(json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0);
-        assert_se(!s);
+        assert_se(streq_ptr(s, "{\"b\":[\"foo\",\"bar\",\"baz\",\"qux\"],\"c\":-9223372036854775808,\"a\":\"<sensitive data>\",\"d\":\"-9223372036854775808\",\"e\":{}}"));
+        s = mfree(s);
         v = json_variant_unref(v);
 
         assert_se(json_build(&v, JSON_BUILD_OBJECT(
@@ -954,8 +957,9 @@ TEST(json_sensitive) {
                                              JSON_BUILD_PAIR("e", JSON_BUILD_EMPTY_OBJECT))) >= 0);
         json_variant_dump(v, JSON_FORMAT_COLOR|JSON_FORMAT_PRETTY, NULL, NULL);
 
-        assert_se(json_variant_format(v, JSON_FORMAT_REFUSE_SENSITIVE, &s) == -EPERM);
+        assert_se(json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0);
-        assert_se(!s);
+        assert_se(streq_ptr(s, "{\"b\":[\"foo\",\"bar\",\"baz\",\"qux\"],\"c\":-9223372036854775808,\"d\":\"-9223372036854775808\",\"a\":\"<sensitive data>\",\"e\":{}}"));
+        s = mfree(s);
         v = json_variant_unref(v);
 
         assert_se(json_build(&v, JSON_BUILD_OBJECT(
@@ -966,8 +970,8 @@ TEST(json_sensitive) {
                                              JSON_BUILD_PAIR_VARIANT("a", a))) >= 0);
         json_variant_dump(v, JSON_FORMAT_COLOR|JSON_FORMAT_PRETTY, NULL, NULL);
 
-        assert_se(json_variant_format(v, JSON_FORMAT_REFUSE_SENSITIVE, &s) == -EPERM);
+        assert_se(json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0);
-        assert_se(!s);
+        assert_se(streq_ptr(s, "{\"b\":[\"foo\",\"bar\",\"baz\",\"qux\"],\"c\":-9223372036854775808,\"d\":\"-9223372036854775808\",\"e\":{},\"a\":\"<sensitive data>\"}"));
 }
 
 TEST(json_iovec) {