Redact ACL username information and mark *-key-file-pass configs as sensitive (#12860)

In #11489, we consider acl username to be sensitive information,
and consider the ACL GETUSER a sensitive command and remove it
from redis-cli historyfile.

This PR redact username information in ACL GETUSER and ACL DELUSER
from SLOWLOG, and also remove ACL DELUSER from redis-cli historyfile.

This PR also mark tls-key-file-pass and tls-client-key-file-pass
as sensitive config, will redact it from SLOWLOG and also
remove them from redis-cli historyfile.
This commit is contained in:
Binbin 2023-12-13 21:28:13 +08:00 committed by GitHub
parent f9cc25c1dd
commit 3c0fd25201
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 33 additions and 10 deletions

View File

@ -2837,6 +2837,10 @@ void aclCommand(client *c) {
}
return;
} else if (!strcasecmp(sub,"deluser") && c->argc >= 3) {
/* Initially redact all the arguments to not leak any information
* about the users. */
for (int j = 2; j < c->argc; j++) redactClientCommandArgument(c, j);
int deleted = 0;
for (int j = 2; j < c->argc; j++) {
sds username = c->argv[j]->ptr;
@ -2859,6 +2863,9 @@ void aclCommand(client *c) {
}
addReplyLongLong(c,deleted);
} else if (!strcasecmp(sub,"getuser") && c->argc == 3) {
/* Redact the username to not leak any information about the user. */
redactClientCommandArgument(c, 2);
user *u = ACLGetUserByName(c->argv[2]->ptr,sdslen(c->argv[2]->ptr));
if (u == NULL) {
addReplyNull(c);

View File

@ -3244,10 +3244,10 @@ standardConfig static_configs[] = {
createBoolConfig("tls-session-caching", NULL, MODIFIABLE_CONFIG, server.tls_ctx_config.session_caching, 1, NULL, applyTlsCfg),
createStringConfig("tls-cert-file", NULL, VOLATILE_CONFIG | MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.cert_file, NULL, NULL, applyTlsCfg),
createStringConfig("tls-key-file", NULL, VOLATILE_CONFIG | MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.key_file, NULL, NULL, applyTlsCfg),
createStringConfig("tls-key-file-pass", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.key_file_pass, NULL, NULL, applyTlsCfg),
createStringConfig("tls-key-file-pass", NULL, MODIFIABLE_CONFIG | SENSITIVE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.key_file_pass, NULL, NULL, applyTlsCfg),
createStringConfig("tls-client-cert-file", NULL, VOLATILE_CONFIG | MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.client_cert_file, NULL, NULL, applyTlsCfg),
createStringConfig("tls-client-key-file", NULL, VOLATILE_CONFIG | MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.client_key_file, NULL, NULL, applyTlsCfg),
createStringConfig("tls-client-key-file-pass", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.client_key_file_pass, NULL, NULL, applyTlsCfg),
createStringConfig("tls-client-key-file-pass", NULL, MODIFIABLE_CONFIG | SENSITIVE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.client_key_file_pass, NULL, NULL, applyTlsCfg),
createStringConfig("tls-dh-params-file", NULL, VOLATILE_CONFIG | MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.dh_params_file, NULL, NULL, applyTlsCfg),
createStringConfig("tls-ca-cert-file", NULL, VOLATILE_CONFIG | MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.ca_cert_file, NULL, NULL, applyTlsCfg),
createStringConfig("tls-ca-cert-dir", NULL, VOLATILE_CONFIG | MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.ca_cert_dir, NULL, NULL, applyTlsCfg),

View File

@ -3266,8 +3266,8 @@ void cliLoadPreferences(void) {
/* Some commands can include sensitive information and shouldn't be put in the
* history file. Currently these commands are include:
* - AUTH
* - ACL SETUSER, ACL GETUSER
* - CONFIG SET masterauth/masteruser/requirepass
* - ACL DELUSER, ACL SETUSER, ACL GETUSER
* - CONFIG SET masterauth/masteruser/tls-key-file-pass/tls-client-key-file-pass/requirepass
* - HELLO with [AUTH username password]
* - MIGRATE with [AUTH password] or [AUTH2 username password]
* - SENTINEL CONFIG SET sentinel-pass password, SENTINEL CONFIG SET sentinel-user username
@ -3277,6 +3277,7 @@ static int isSensitiveCommand(int argc, char **argv) {
return 1;
} else if (argc > 1 &&
!strcasecmp(argv[0],"acl") && (
!strcasecmp(argv[1],"deluser") ||
!strcasecmp(argv[1],"setuser") ||
!strcasecmp(argv[1],"getuser")))
{
@ -3287,6 +3288,8 @@ static int isSensitiveCommand(int argc, char **argv) {
for (int j = 2; j < argc; j = j+2) {
if (!strcasecmp(argv[j],"masterauth") ||
!strcasecmp(argv[j],"masteruser") ||
!strcasecmp(argv[j],"tls-key-file-pass") ||
!strcasecmp(argv[j],"tls-client-key-file-pass") ||
!strcasecmp(argv[j],"requirepass")) {
return 1;
}

View File

@ -24,7 +24,7 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} {
} {10}
test {SLOWLOG - GET optional argument to limit output len works} {
assert_equal 5 [llength [r slowlog get 5]]
assert_equal 10 [llength [r slowlog get -1]]
assert_equal 10 [llength [r slowlog get 20]]
@ -50,22 +50,35 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} {
} {} {needs:debug}
test {SLOWLOG - Certain commands are omitted that contain sensitive information} {
r config set slowlog-max-len 100
r config set slowlog-log-slower-than 0
r slowlog reset
catch {r acl setuser "slowlog test user" +get +set} _
r config set masteruser ""
r config set masterauth ""
r config set requirepass ""
r config set tls-key-file-pass ""
r config set tls-client-key-file-pass ""
r acl setuser slowlog-test-user +get +set
r acl getuser slowlog-test-user
r acl deluser slowlog-test-user non-existing-user
r config set slowlog-log-slower-than 0
r config set slowlog-log-slower-than -1
set slowlog_resp [r slowlog get]
set slowlog_resp [r slowlog get -1]
# Make sure normal configs work, but the two sensitive
# commands are omitted or redacted
assert_equal 5 [llength $slowlog_resp]
assert_equal {slowlog reset} [lindex [lindex $slowlog_resp 4] 3]
assert_equal 11 [llength $slowlog_resp]
assert_equal {slowlog reset} [lindex [lindex $slowlog_resp 10] 3]
assert_equal {acl setuser (redacted) (redacted) (redacted)} [lindex [lindex $slowlog_resp 9] 3]
assert_equal {config set masteruser (redacted)} [lindex [lindex $slowlog_resp 8] 3]
assert_equal {config set masterauth (redacted)} [lindex [lindex $slowlog_resp 7] 3]
assert_equal {config set requirepass (redacted)} [lindex [lindex $slowlog_resp 6] 3]
assert_equal {config set tls-key-file-pass (redacted)} [lindex [lindex $slowlog_resp 5] 3]
assert_equal {config set tls-client-key-file-pass (redacted)} [lindex [lindex $slowlog_resp 4] 3]
assert_equal {acl setuser (redacted) (redacted) (redacted)} [lindex [lindex $slowlog_resp 3] 3]
assert_equal {config set masterauth (redacted)} [lindex [lindex $slowlog_resp 2] 3]
assert_equal {acl setuser (redacted) (redacted) (redacted)} [lindex [lindex $slowlog_resp 1] 3]
assert_equal {acl getuser (redacted)} [lindex [lindex $slowlog_resp 2] 3]
assert_equal {acl deluser (redacted) (redacted)} [lindex [lindex $slowlog_resp 1] 3]
assert_equal {config set slowlog-log-slower-than 0} [lindex [lindex $slowlog_resp 0] 3]
} {} {needs:repl}