From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 16 Jan 2024 10:27:40 +0000 (+0100)
Subject: varlink: optionally, mark all incoming message's "parameters" field as sensitive
X-Git-Tag: v256-rc1~1134^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a570877c12760def86fcf20b151685b6a736cdda;p=thirdparty%2Fsystemd.git

varlink: optionally, mark all incoming message's "parameters" field as sensitive

So far the varlink logic honoured the "sensitive" flag of output
messages. Let's add something similar for input messages. Since we don't
really know incoming messages, the flag simply controls whether the
"parmaeters" field of all incoming messages should be marked as
sensitive.

Then, turn this on in the credentials logic and in homed, since both
deal with credentials.
---

diff --git a/src/creds/creds.c b/src/creds/creds.c
index c9d1a6e8d94..bbc705c0069 100644
--- a/src/creds/creds.c
+++ b/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");
 
diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c
index 94b2ea5181a..b1d0c511203 100644
--- a/src/home/homed-manager.c
+++ b/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");
 
diff --git a/src/shared/varlink.c b/src/shared/varlink.c
index 699d21745f1..96dfd8de92a 100644
--- a/src/shared/varlink.c
+++ b/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,13 @@ 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);
+        }
+
         v->input_buffer_size -= sz;
 
         if (v->input_buffer_size == 0)
@@ -3097,6 +3108,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 +3343,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) {
diff --git a/src/shared/varlink.h b/src/shared/varlink.h
index a971762a511..418ba49d1a0 100644
--- a/src/shared/varlink.h
+++ b/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);