diff --git a/src/creds/creds.c b/src/creds/creds.c index c9d1a6e8d9..bbc705c006 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 94b2ea5181..b1d0c51120 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/json.c b/src/shared/json.c index 0259196e4e..7ec6368e1e 100644 --- a/src/shared/json.c +++ b/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("\"\"", 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; diff --git a/src/shared/json.h b/src/shared/json.h index 3c20f94b58..9c8448f728 100644 --- a/src/shared/json.h +++ b/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 "" */ } JsonFormatFlags; int json_variant_format(JsonVariant *v, JsonFormatFlags flags, char **ret); diff --git a/src/shared/varlink.c b/src/shared/varlink.c index 699d21745f..84497c1a06 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,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 r; + int sz, r; assert(v); assert(m); - r = json_variant_format(m, JSON_FORMAT_REFUSE_SENSITIVE, &text); - if (r == -EPERM) { - sensitive = true; - r = json_variant_format(m, /* flags= */ 0, &text); - } - if (r < 0) - return r; - assert(text[r] == '\0'); + sz = json_variant_format(m, /* flags= */ 0, &text); + if (sz < 0) + return sz; + assert(text[sz] == '\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 ? "" : 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); - v->output_buffer_size += r + 1; + memcpy(v->output_buffer + v->output_buffer_size, text, sz + 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) { diff --git a/src/shared/varlink.h b/src/shared/varlink.h index a971762a51..418ba49d1a 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); diff --git a/src/test/test-json.c b/src/test/test-json.c index 333fbe6cf2..cb0adc2643 100644 --- a/src/test/test-json.c +++ b/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); - assert_se(r == -EPERM); - assert_se(!s); + r = json_variant_format(w, JSON_FORMAT_CENSOR_SENSITIVE, &s); + assert_se(s); + assert_se(streq_ptr(s, "\"\"")); 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(!s); + assert_se(json_variant_format(a, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0); + assert_se(streq_ptr(s, "\"\"")); + 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(!s); + assert_se(json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0); + assert_se(streq_ptr(s, "{\"b\":[\"foo\",\"bar\",\"baz\",\"qux\"],\"a\":\"\",\"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(!s); + assert_se(json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0); + assert_se(streq_ptr(s, "{\"b\":[\"foo\",\"bar\",\"baz\",\"qux\"],\"c\":-9223372036854775808,\"a\":\"\",\"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(!s); + assert_se(json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0); + assert_se(streq_ptr(s, "{\"b\":[\"foo\",\"bar\",\"baz\",\"qux\"],\"c\":-9223372036854775808,\"d\":\"-9223372036854775808\",\"a\":\"\",\"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(!s); + assert_se(json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0); + assert_se(streq_ptr(s, "{\"b\":[\"foo\",\"bar\",\"baz\",\"qux\"],\"c\":-9223372036854775808,\"d\":\"-9223372036854775808\",\"e\":{},\"a\":\"\"}")); } TEST(json_iovec) {