Skip to content

Commit 7cdab8b

Browse files
Redact protocol error log when hide-user-data-from-log enabled (#1889)
In this code logic: https://github.com/valkey-io/valkey/blob/unstable/src/networking.c#L2767-L2773, `c->querybuf + c->qb_pos` may also include user data. Update the log message when config `hide-user-data-from-log` is enabled. --------- Signed-off-by: VanessaTang <[email protected]> Signed-off-by: Ran Shidlansik <[email protected]> Co-authored-by: Ran Shidlansik <[email protected]>
1 parent 3dd1f18 commit 7cdab8b

File tree

2 files changed

+38
-14
lines changed

2 files changed

+38
-14
lines changed

src/networking.c

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2764,24 +2764,27 @@ static void setProtocolError(const char *errstr, client *c) {
27642764
/* Sample some protocol to given an idea about what was inside. */
27652765
char buf[256];
27662766
buf[0] = '\0';
2767-
if (c->querybuf && sdslen(c->querybuf) - c->qb_pos < PROTO_DUMP_LEN) {
2768-
snprintf(buf, sizeof(buf), "Query buffer during protocol error: '%s'", c->querybuf + c->qb_pos);
2769-
} else if (c->querybuf) {
2770-
snprintf(buf, sizeof(buf), "Query buffer during protocol error: '%.*s' (... more %zu bytes ...) '%.*s'",
2771-
PROTO_DUMP_LEN / 2, c->querybuf + c->qb_pos, sdslen(c->querybuf) - c->qb_pos - PROTO_DUMP_LEN,
2772-
PROTO_DUMP_LEN / 2, c->querybuf + sdslen(c->querybuf) - PROTO_DUMP_LEN / 2);
2773-
}
2767+
if (server.hide_user_data_from_log) {
2768+
snprintf(buf, sizeof(buf), "*redacted*");
2769+
} else {
2770+
if (c->querybuf && sdslen(c->querybuf) - c->qb_pos < PROTO_DUMP_LEN) {
2771+
snprintf(buf, sizeof(buf), "'%s'", c->querybuf + c->qb_pos);
2772+
} else if (c->querybuf) {
2773+
snprintf(buf, sizeof(buf), "'%.*s' (... more %zu bytes ...) '%.*s'",
2774+
PROTO_DUMP_LEN / 2, c->querybuf + c->qb_pos, sdslen(c->querybuf) - c->qb_pos - PROTO_DUMP_LEN,
2775+
PROTO_DUMP_LEN / 2, c->querybuf + sdslen(c->querybuf) - PROTO_DUMP_LEN / 2);
2776+
}
27742777

2775-
/* Remove non printable chars. */
2776-
char *p = buf;
2777-
while (*p != '\0') {
2778-
if (!isprint(*p)) *p = '.';
2779-
p++;
2778+
/* Remove non printable chars. */
2779+
char *p = buf;
2780+
while (*p != '\0') {
2781+
if (!isprint(*p)) *p = '.';
2782+
p++;
2783+
}
27802784
}
2781-
27822785
/* Log all the client and protocol info. */
27832786
int loglevel = (c->flag.primary) ? LL_WARNING : LL_VERBOSE;
2784-
serverLog(loglevel, "Protocol error (%s) from client: %s. %s", errstr, client, buf);
2787+
serverLog(loglevel, "Protocol error (%s) from client: %s. Query buffer: %s", errstr, client, buf);
27852788
sdsfree(client);
27862789
}
27872790
c->flag.close_after_reply = 1;

tests/integration/logging.tcl

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,27 @@ if {!$::valgrind} {
109109
assert_equal "PONG" [r ping]
110110
}
111111
}
112+
113+
# test hide-user-data-from-log
114+
set server_path [tmpdir server5.log]
115+
start_server [list overrides [list dir $server_path crash-memcheck-enabled no hide-user-data-from-log no]] {
116+
test "Config hide-user-data-from-log is off" {
117+
r write "*3\r\n\$3\r\nSET\r\n\$1\r\nx\r\n\$blabla\r\n"
118+
r flush
119+
catch {r debug segfault}
120+
wait_for_log_messages 0 {"*Query buffer: *\$blabla*"} 0 10 1000
121+
}
122+
}
123+
124+
set server_path [tmpdir server6.log]
125+
start_server [list overrides [list dir $server_path crash-memcheck-enabled no hide-user-data-from-log yes]] {
126+
test "Config hide-user-data-from-log is on" {
127+
r write "*3\r\n\$3\r\nSET\r\n\$1\r\nx\r\n\$blabla\r\n"
128+
r flush
129+
catch {r debug segfault}
130+
wait_for_log_messages 0 {"*Query buffer: *redacted*"} 0 10 1000
131+
}
132+
}
112133
}
113134

114135
# test DEBUG ASSERT

0 commit comments

Comments
 (0)