json: replace JSON_FORMAT_REFUSE_SENSITIVE with JSON_FORMAT_CENSOR_SENSITIVE

Previously, the flag would completely refuse formatting a JSON object if
any field of it was marked sensitive. With this change we'll simply
replace the subobject with the string "<sensitive data>", and show
everything else.

This is tremendously useful when debugging, since it means that we can
again trace varlink calls through the stack: we can show all the message
metadata and just suppress the actually sensitive parameters.

The ability to debug this matters, and we should not hide more
information that we can get away with, to keep things debuggable and
maintainable.

diff --git a/src/shared/json.c b/src/shared/json.c
index 270aef41051d30093fbfa5460cc5824067b3452f..7ec6368e1eccbc1e36f1cea920514b490aafc24b 100644 (file)
--- a/src/shared/json.c
+++ b/src/shared/json.c
@@ -1612,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: {
@@ -1818,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 156f8e731e4e6fb7f4f64fd55f67269886e58016..9c8448f728b82e450de8034c1c2c7d5e18cf832c 100644 (file)
--- a/src/shared/json.h
+++ b/src/shared/json.h
@@ -197,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);

diff --git a/src/shared/varlink.c b/src/shared/varlink.c
index 96dfd8de92ac40c60413d60291c89853c83deab5..7c37211cdda1bac8a7bf76eb642aa4273fd82e2c 100644 (file)
--- a/src/shared/varlink.c
+++ b/src/shared/varlink.c
@@ -1860,56 +1860,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 ? "<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);
-                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 */

diff --git a/src/test/test-json.c b/src/test/test-json.c
index 333fbe6cf2a30c320f13178dba1243b2c3598f17..cb0adc2643774d90640ff6bdb5168661bacbd765 100644 (file)
--- 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, "\"<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(!s);
+        assert_se(json_variant_format(a, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0);
+        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(!s);
+        assert_se(json_variant_format(v, JSON_FORMAT_CENSOR_SENSITIVE, &s) >= 0);
+        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(!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\":\"<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(!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\":\"<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(!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\":\"<sensitive data>\"}"));
 }
 
 TEST(json_iovec) {