From 73d5aba7b8906ef44eea10ed1255007540a2cf9a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 24 Jun 2020 18:00:41 -0700 Subject: [PATCH] libct/nsenter: add json msg escaping Since the previous commit, some strings logged by write_log() contain a literal newline, which leads to errors like this one: > # time="2020-06-07T15:41:37Z" level=error msg="failed to decode \"{\\\"level\\\":\\\"debug\\\", \\\"msg\\\": \\\"nsexec-0[2265]: update /proc/2266/uid_map to '0 1000 1\\n\" to json: invalid character '\\n' in string literal" The fix is to escape such characters. Add a simple (as much as it can be) routine which implements JSON string escaping as required by RFC4627, section 2.5, plus escaping of DEL (0x7f) character (not required, but allowed by the standard, and usually done by tools such as jq). As much as I hate to code something like this, I was not able to find a ready to consume and decent C implementation (not using glib). Added a test case (and some additional asserts in C code, conditionally enabled by the test case) to make sure the implementation is correct. The test case have to live in a separate directory so we can use different C flags to compile the test, and use C from go test. [v2: try to simplify the code, add more tests] Signed-off-by: Kir Kolyshkin --- libcontainer/nsenter/escape.c | 138 +++++++++++++++++++++++ libcontainer/nsenter/nsexec.c | 4 + libcontainer/nsenter/test/escape.c | 1 + libcontainer/nsenter/test/escape.go | 53 +++++++++ libcontainer/nsenter/test/escape_test.go | 11 ++ 5 files changed, 207 insertions(+) create mode 100644 libcontainer/nsenter/escape.c create mode 120000 libcontainer/nsenter/test/escape.c create mode 100644 libcontainer/nsenter/test/escape.go create mode 100644 libcontainer/nsenter/test/escape_test.go diff --git a/libcontainer/nsenter/escape.c b/libcontainer/nsenter/escape.c new file mode 100644 index 00000000000..6164b4241f9 --- /dev/null +++ b/libcontainer/nsenter/escape.c @@ -0,0 +1,138 @@ +#include + +#ifdef ESCAPE_TEST +# include +# define test_assert(arg) assert(arg) +#else +# define test_assert(arg) +#endif + +#define DEL '\x7f' + +/* + * Poor man version of itoa with base=16 and input number from 0 to 15, + * represented by a char. Converts it to a single hex digit ('0' to 'f'). + */ +static char hex(char i) +{ + test_assert(i >= 0 && i < 16); + + if (i >= 0 && i < 10) { + return '0' + i; + } + if (i >= 10 && i < 16) { + return 'a' + i - 10; + } + return '?'; +} + +/* + * Given the character, tells how many _extra_ characters are needed + * to JSON-escape it. If 0 is returned, the character does not need to + * be escaped. + */ +static int need_escape(char c) +{ + switch (c) { + case '\\': + case '"': + case '\b': + case '\n': + case '\r': + case '\t': + case '\f': + return 1; + case DEL: // -> \u007f + return 5; + default: + if (c > 0 && c < ' ') { + // ASCII decimal 01 to 31 -> \u00xx + return 5; + } + return 0; + } +} + +/* + * Escape the string so it can be used as a JSON string (per RFC4627, + * section 2.5 minimal requirements, plus the DEL (0x7f) character). + * + * It is expected that the argument is a string allocated via malloc. + * In case no escaping is needed, the original string is returned as is; + * otherwise, the original string is free'd, and the newly allocated + * escaped string is returned. Thus, in any case, the value returned + * need to be free'd by the caller. + */ +char *escape_json_string(char *s) +{ + int i, j, len; + char *c, *out; + + /* + * First, check if escaping is at all needed -- if not, we can avoid + * malloc and return the argument as is. While at it, count how much + * extra space is required. + * + * XXX: the counting code must be in sync with the escaping code + * (checked by test_assert()s below). + */ + for (i = j = 0; s[i] != '\0'; i++) { + j += need_escape(s[i]); + } + if (j == 0) { + // nothing to escape + return s; + } + + len = i + j + 1; + out = malloc(len); + if (!out) { + exit(1); + } + for (c = s, j = 0; *c != '\0'; c++) { + switch (*c) { + case '"': + case '\\': + test_assert(need_escape(*c) == 1); + out[j++] = '\\'; + out[j++] = *c; + continue; + } + if ((*c < 0 || *c >= ' ') && (*c != DEL)) { + // no escape needed + test_assert(need_escape(*c) == 0); + out[j++] = *c; + continue; + } + out[j++] = '\\'; + switch (*c) { + case '\b': + out[j++] = 'b'; + break; + case '\n': + out[j++] = 'n'; + break; + case '\r': + out[j++] = 'r'; + break; + case '\t': + out[j++] = 't'; + break; + case '\f': + out[j++] = 'f'; + break; + default: + test_assert(need_escape(*c) == 5); + out[j++] = 'u'; + out[j++] = '0'; + out[j++] = '0'; + out[j++] = hex(*c >> 4); + out[j++] = hex(*c & 0x0f); + } + } + test_assert(j + 1 == len); + out[j] = '\0'; + + free(s); + return out; +} diff --git a/libcontainer/nsenter/nsexec.c b/libcontainer/nsenter/nsexec.c index 6896b769edc..5503b81f8f3 100644 --- a/libcontainer/nsenter/nsexec.c +++ b/libcontainer/nsenter/nsexec.c @@ -29,6 +29,8 @@ /* Get all of the CLONE_NEW* flags. */ #include "namespace.h" +extern char *escape_json_string(char *str); + /* Synchronisation values. */ enum sync_t { SYNC_USERMAP_PLS = 0x40, /* Request parent to map our users. */ @@ -153,6 +155,8 @@ static void write_log(const char *level, const char *format, ...) if (ret < 0) goto out; + message = escape_json_string(message); + if (current_stage == STAGE_SETUP) stage = strdup("nsexec"); else diff --git a/libcontainer/nsenter/test/escape.c b/libcontainer/nsenter/test/escape.c new file mode 120000 index 00000000000..c53e316a22c --- /dev/null +++ b/libcontainer/nsenter/test/escape.c @@ -0,0 +1 @@ +../escape.c \ No newline at end of file diff --git a/libcontainer/nsenter/test/escape.go b/libcontainer/nsenter/test/escape.go new file mode 100644 index 00000000000..4accf967a47 --- /dev/null +++ b/libcontainer/nsenter/test/escape.go @@ -0,0 +1,53 @@ +package escapetest + +// This file is part of escape_json_string unit test. +// It is in a separate package so cgo can be used together +// with go test. + +// #include +// extern char *escape_json_string(char *str); +// #cgo CFLAGS: -DESCAPE_TEST=1 +import "C" + +import ( + "testing" + "unsafe" +) + +func testEscapeJsonString(t *testing.T, input, want string) { + in := C.CString(input) + out := C.escape_json_string(in) + got := C.GoString(out) + C.free(unsafe.Pointer(out)) + t.Logf("input: %q, output: %q", input, got) + if got != want { + t.Errorf("Failed on input: %q, want %q, got %q", input, want, got) + } +} + +func testEscapeJson(t *testing.T) { + testCases := []struct { + input, output string + }{ + {"", ""}, + {"abcdef", "abcdef"}, + {`\\\\\\`, `\\\\\\\\\\\\`}, + {`with"quote`, `with\"quote`}, + {"\n\r\b\t\f\\", `\n\r\b\t\f\\`}, + {"\007", "\\u0007"}, + {"\017 \020 \037", "\\u000f \\u0010 \\u001f"}, + {"\033", "\\u001b"}, + {`<->`, `<->`}, + {"\176\177\200", "~\\u007f\200"}, + {"\000", ""}, + {"a\x7fxc", "a\\u007fxc"}, + {"a\033xc", "a\\u001bxc"}, + {"a\nxc", "a\\nxc"}, + {"a\\xc", "a\\\\xc"}, + {"Barney B\303\244r", "Barney B\303\244r"}, + } + + for _, tc := range testCases { + testEscapeJsonString(t, tc.input, tc.output) + } +} diff --git a/libcontainer/nsenter/test/escape_test.go b/libcontainer/nsenter/test/escape_test.go new file mode 100644 index 00000000000..eefd5ecf11a --- /dev/null +++ b/libcontainer/nsenter/test/escape_test.go @@ -0,0 +1,11 @@ +package escapetest + +import "testing" + +// The actual test function is in escape.go +// so that it can use cgo (import "C"). +// This wrapper is here for gotest to find. + +func TestEscapeJson(t *testing.T) { + testEscapeJson(t) +}