Redact slowlog entries for config with sensitive data. (#8584)
Redact config set requirepass/masterauth/masteruser from slowlog in addition to showing ACL commands without sensitive values.
This commit is contained in:
Madelyn Olson
GitHub
parent
12991353b6
commit
139181e9eb
@ -1873,6 +1873,10 @@ void addACLLogEntry(client *c, int reason, int argpos, sds username) {
|
||||
void aclCommand(client *c) {
|
||||
char *sub = c->argv[1]->ptr;
|
||||
if (!strcasecmp(sub,"setuser") && c->argc >= 3) {
|
||||
/* Consider information about passwords or permissions
|
||||
* to be sensitive, which will be the arguments for this
|
||||
* subcommand. */
|
||||
preventCommandLogging(c);
|
||||
sds username = c->argv[2]->ptr;
|
||||
/* Check username validity. */
|
||||
if (ACLStringHasSpaces(username,sdslen(username))) {
|
||||
|
||||
@ -106,12 +106,20 @@ void blockClient(client *c, int btype) {
|
||||
void updateStatsOnUnblock(client *c, long blocked_us, long reply_us){
|
||||
const ustime_t total_cmd_duration = c->duration + blocked_us + reply_us;
|
||||
c->lastcmd->microseconds += total_cmd_duration;
|
||||
|
||||
/* Log the command into the Slow log if needed. */
|
||||
if (!(c->lastcmd->flags & CMD_SKIP_SLOWLOG)) {
|
||||
if (!(c->lastcmd->flags & CMD_SKIP_SLOWLOG) &&
|
||||
!(c->flags & CLIENT_PREVENT_LOGGING))
|
||||
{
|
||||
slowlogPushEntryIfNeeded(c,c->argv,c->argc,total_cmd_duration);
|
||||
/* Log the reply duration event. */
|
||||
latencyAddSampleIfNeeded("command-unblocking",reply_us/1000);
|
||||
}
|
||||
|
||||
/* Always clear the prevent logging field now. */
|
||||
if (c->flags & CLIENT_PREVENT_LOGGING) {
|
||||
c->flags &= ~CLIENT_PREVENT_LOGGING;
|
||||
}
|
||||
}
|
||||
|
||||
/* This function is called in the beforeSleep() function of the event loop
|
||||
|
||||
86
src/config.c
86
src/config.c
@ -241,11 +241,16 @@ typedef struct typeInterface {
|
||||
typedef struct standardConfig {
|
||||
const char *name; /* The user visible name of this config */
|
||||
const char *alias; /* An alias that can also be used for this config */
|
||||
const int modifiable; /* Can this value be updated by CONFIG SET? */
|
||||
const unsigned int flags; /* Flags for this specific config */
|
||||
typeInterface interface; /* The function pointers that define the type interface */
|
||||
typeData data; /* The type specific data exposed used by the interface */
|
||||
} standardConfig;
|
||||
|
||||
#define MODIFIABLE_CONFIG 0 /* This is the implied default for a standard
|
||||
* config, which is mutable. */
|
||||
#define IMMUTABLE_CONFIG (1ULL<<0) /* Can this value only be set at startup? */
|
||||
#define SENSITIVE_CONFIG (1ULL<<1) /* Does this value contain sensitive information */
|
||||
|
||||
standardConfig configs[];
|
||||
|
||||
/*-----------------------------------------------------------------------------
|
||||
@ -712,9 +717,13 @@ void configSetCommand(client *c) {
|
||||
|
||||
/* Iterate the configs that are standard */
|
||||
for (standardConfig *config = configs; config->name != NULL; config++) {
|
||||
if(config->modifiable && (!strcasecmp(c->argv[2]->ptr,config->name) ||
|
||||
if (!(config->flags & IMMUTABLE_CONFIG) &&
|
||||
(!strcasecmp(c->argv[2]->ptr,config->name) ||
|
||||
(config->alias && !strcasecmp(c->argv[2]->ptr,config->alias))))
|
||||
{
|
||||
if (config->flags & SENSITIVE_CONFIG) {
|
||||
preventCommandLogging(c);
|
||||
}
|
||||
if (!config->interface.set(config->data,o->ptr,1,&errstr)) {
|
||||
goto badfmt;
|
||||
}
|
||||
@ -1709,13 +1718,10 @@ int rewriteConfig(char *path, int force_all) {
|
||||
#define LOADBUF_SIZE 256
|
||||
static char loadbuf[LOADBUF_SIZE];
|
||||
|
||||
#define MODIFIABLE_CONFIG 1
|
||||
#define IMMUTABLE_CONFIG 0
|
||||
|
||||
#define embedCommonConfig(config_name, config_alias, is_modifiable) \
|
||||
#define embedCommonConfig(config_name, config_alias, config_flags) \
|
||||
.name = (config_name), \
|
||||
.alias = (config_alias), \
|
||||
.modifiable = (is_modifiable),
|
||||
.flags = (config_flags),
|
||||
|
||||
#define embedConfigInterface(initfn, setfn, getfn, rewritefn) .interface = { \
|
||||
.init = (initfn), \
|
||||
@ -1766,8 +1772,8 @@ static void boolConfigRewrite(typeData data, const char *name, struct rewriteCon
|
||||
rewriteConfigYesNoOption(state, name,*(data.yesno.config), data.yesno.default_value);
|
||||
}
|
||||
|
||||
#define createBoolConfig(name, alias, modifiable, config_addr, default, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, modifiable) \
|
||||
#define createBoolConfig(name, alias, flags, config_addr, default, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, flags) \
|
||||
embedConfigInterface(boolConfigInit, boolConfigSet, boolConfigGet, boolConfigRewrite) \
|
||||
.data.yesno = { \
|
||||
.config = &(config_addr), \
|
||||
@ -1839,8 +1845,8 @@ static void sdsConfigRewrite(typeData data, const char *name, struct rewriteConf
|
||||
#define ALLOW_EMPTY_STRING 0
|
||||
#define EMPTY_STRING_IS_NULL 1
|
||||
|
||||
#define createStringConfig(name, alias, modifiable, empty_to_null, config_addr, default, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, modifiable) \
|
||||
#define createStringConfig(name, alias, flags, empty_to_null, config_addr, default, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, flags) \
|
||||
embedConfigInterface(stringConfigInit, stringConfigSet, stringConfigGet, stringConfigRewrite) \
|
||||
.data.string = { \
|
||||
.config = &(config_addr), \
|
||||
@ -1851,8 +1857,8 @@ static void sdsConfigRewrite(typeData data, const char *name, struct rewriteConf
|
||||
} \
|
||||
}
|
||||
|
||||
#define createSDSConfig(name, alias, modifiable, empty_to_null, config_addr, default, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, modifiable) \
|
||||
#define createSDSConfig(name, alias, flags, empty_to_null, config_addr, default, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, flags) \
|
||||
embedConfigInterface(sdsConfigInit, sdsConfigSet, sdsConfigGet, sdsConfigRewrite) \
|
||||
.data.sds = { \
|
||||
.config = &(config_addr), \
|
||||
@ -1907,8 +1913,8 @@ static void enumConfigRewrite(typeData data, const char *name, struct rewriteCon
|
||||
rewriteConfigEnumOption(state, name,*(data.enumd.config), data.enumd.enum_value, data.enumd.default_value);
|
||||
}
|
||||
|
||||
#define createEnumConfig(name, alias, modifiable, enum, config_addr, default, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, modifiable) \
|
||||
#define createEnumConfig(name, alias, flags, enum, config_addr, default, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, flags) \
|
||||
embedConfigInterface(enumConfigInit, enumConfigSet, enumConfigGet, enumConfigRewrite) \
|
||||
.data.enumd = { \
|
||||
.config = &(config_addr), \
|
||||
@ -2061,8 +2067,8 @@ static void numericConfigRewrite(typeData data, const char *name, struct rewrite
|
||||
#define INTEGER_CONFIG 0
|
||||
#define MEMORY_CONFIG 1
|
||||
|
||||
#define embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, modifiable) \
|
||||
#define embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) { \
|
||||
embedCommonConfig(name, alias, flags) \
|
||||
embedConfigInterface(numericConfigInit, numericConfigSet, numericConfigGet, numericConfigRewrite) \
|
||||
.data.numeric = { \
|
||||
.lower_bound = (lower), \
|
||||
@ -2072,71 +2078,71 @@ static void numericConfigRewrite(typeData data, const char *name, struct rewrite
|
||||
.update_fn = (update), \
|
||||
.is_memory = (memory),
|
||||
|
||||
#define createIntConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createIntConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_INT, \
|
||||
.config.i = &(config_addr) \
|
||||
} \
|
||||
}
|
||||
|
||||
#define createUIntConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createUIntConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_UINT, \
|
||||
.config.ui = &(config_addr) \
|
||||
} \
|
||||
}
|
||||
|
||||
#define createLongConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createLongConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_LONG, \
|
||||
.config.l = &(config_addr) \
|
||||
} \
|
||||
}
|
||||
|
||||
#define createULongConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createULongConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_ULONG, \
|
||||
.config.ul = &(config_addr) \
|
||||
} \
|
||||
}
|
||||
|
||||
#define createLongLongConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createLongLongConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_LONG_LONG, \
|
||||
.config.ll = &(config_addr) \
|
||||
} \
|
||||
}
|
||||
|
||||
#define createULongLongConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createULongLongConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_ULONG_LONG, \
|
||||
.config.ull = &(config_addr) \
|
||||
} \
|
||||
}
|
||||
|
||||
#define createSizeTConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createSizeTConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_SIZE_T, \
|
||||
.config.st = &(config_addr) \
|
||||
} \
|
||||
}
|
||||
|
||||
#define createSSizeTConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createSSizeTConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_SSIZE_T, \
|
||||
.config.sst = &(config_addr) \
|
||||
} \
|
||||
}
|
||||
|
||||
#define createTimeTConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createTimeTConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_TIME_T, \
|
||||
.config.tt = &(config_addr) \
|
||||
} \
|
||||
}
|
||||
|
||||
#define createOffTConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, modifiable, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
#define createOffTConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
embedCommonNumericalConfig(name, alias, flags, lower, upper, config_addr, default, memory, is_valid, update) \
|
||||
.numeric_type = NUMERIC_TYPE_OFF_T, \
|
||||
.config.ot = &(config_addr) \
|
||||
} \
|
||||
@ -2429,7 +2435,7 @@ standardConfig configs[] = {
|
||||
createStringConfig("unixsocket", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.unixsocket, NULL, NULL, NULL),
|
||||
createStringConfig("pidfile", NULL, IMMUTABLE_CONFIG, EMPTY_STRING_IS_NULL, server.pidfile, NULL, NULL, NULL),
|
||||
createStringConfig("replica-announce-ip", "slave-announce-ip", MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.slave_announce_ip, NULL, NULL, NULL),
|
||||
createStringConfig("masteruser", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.masteruser, NULL, NULL, NULL),
|
||||
createStringConfig("masteruser", NULL, MODIFIABLE_CONFIG | SENSITIVE_CONFIG, EMPTY_STRING_IS_NULL, server.masteruser, NULL, NULL, NULL),
|
||||
createStringConfig("cluster-announce-ip", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_ip, NULL, NULL, NULL),
|
||||
createStringConfig("syslog-ident", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.syslog_ident, "redis", NULL, NULL),
|
||||
createStringConfig("dbfilename", NULL, MODIFIABLE_CONFIG, ALLOW_EMPTY_STRING, server.rdb_filename, "dump.rdb", isValidDBfilename, NULL),
|
||||
@ -2442,8 +2448,8 @@ standardConfig configs[] = {
|
||||
createStringConfig("proc-title-template", NULL, MODIFIABLE_CONFIG, ALLOW_EMPTY_STRING, server.proc_title_template, CONFIG_DEFAULT_PROC_TITLE_TEMPLATE, isValidProcTitleTemplate, updateProcTitleTemplate),
|
||||
|
||||
/* SDS Configs */
|
||||
createSDSConfig("masterauth", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.masterauth, NULL, NULL, NULL),
|
||||
createSDSConfig("requirepass", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.requirepass, NULL, NULL, updateRequirePass),
|
||||
createSDSConfig("masterauth", NULL, MODIFIABLE_CONFIG | SENSITIVE_CONFIG, EMPTY_STRING_IS_NULL, server.masterauth, NULL, NULL, NULL),
|
||||
createSDSConfig("requirepass", NULL, MODIFIABLE_CONFIG | SENSITIVE_CONFIG, EMPTY_STRING_IS_NULL, server.requirepass, NULL, NULL, updateRequirePass),
|
||||
|
||||
/* Enum Configs */
|
||||
createEnumConfig("supervised", NULL, IMMUTABLE_CONFIG, supervised_mode_enum, server.supervised_mode, SUPERVISED_NONE, NULL, NULL),
|
||||
|
||||
@ -2941,6 +2941,7 @@ void helloCommand(client *c) {
|
||||
int moreargs = (c->argc-1) - j;
|
||||
const char *opt = c->argv[j]->ptr;
|
||||
if (!strcasecmp(opt,"AUTH") && moreargs >= 2) {
|
||||
preventCommandLogging(c);
|
||||
if (ACLAuthenticateUser(c, c->argv[j+1], c->argv[j+2]) == C_ERR) {
|
||||
addReplyError(c,"-WRONGPASS invalid username-password pair or user is disabled.");
|
||||
return;
|
||||
|
||||
17
src/server.c
17
src/server.c
@ -901,7 +901,7 @@ struct redisCommand redisCommandTable[] = {
|
||||
0,NULL,0,0,0,0,0,0},
|
||||
|
||||
{"hello",helloCommand,-1,
|
||||
"no-auth no-script fast no-monitor ok-loading ok-stale no-slowlog @connection",
|
||||
"no-auth no-script fast no-monitor ok-loading ok-stale @connection",
|
||||
0,NULL,0,0,0,0,0,0},
|
||||
|
||||
/* EVAL can modify the dataset, however it is not flagged as a write
|
||||
@ -1091,7 +1091,7 @@ struct redisCommand redisCommandTable[] = {
|
||||
0,NULL,0,0,0,0,0,0},
|
||||
|
||||
{"acl",aclCommand,-2,
|
||||
"admin no-script no-slowlog ok-loading ok-stale",
|
||||
"admin no-script ok-loading ok-stale",
|
||||
0,NULL,0,0,0,0,0,0},
|
||||
|
||||
{"stralgo",stralgoCommand,-2,
|
||||
@ -3619,6 +3619,12 @@ void preventCommandPropagation(client *c) {
|
||||
c->flags |= CLIENT_PREVENT_PROP;
|
||||
}
|
||||
|
||||
/* Avoid logging any information about this client's arguments
|
||||
* since they contain sensitive information. */
|
||||
void preventCommandLogging(client *c) {
|
||||
c->flags |= CLIENT_PREVENT_LOGGING;
|
||||
}
|
||||
|
||||
/* AOF specific version of preventCommandPropagation(). */
|
||||
void preventCommandAOF(client *c) {
|
||||
c->flags |= CLIENT_PREVENT_AOF_PROP;
|
||||
@ -3731,6 +3737,13 @@ void call(client *c, int flags) {
|
||||
server.lua_caller->flags |= CLIENT_FORCE_AOF;
|
||||
}
|
||||
|
||||
/* Some commands may contain sensitive data that should
|
||||
* not be available in the slowlog. */
|
||||
if ((c->flags & CLIENT_PREVENT_LOGGING) && !(c->flags & CLIENT_BLOCKED)) {
|
||||
c->flags &= ~CLIENT_PREVENT_LOGGING;
|
||||
flags &= ~CMD_CALL_SLOWLOG;
|
||||
}
|
||||
|
||||
/* Log the command into the Slow log if needed, and populate the
|
||||
* per-command statistics that we show in INFO commandstats. */
|
||||
if (flags & CMD_CALL_SLOWLOG && !(c->cmd->flags & CMD_SKIP_SLOWLOG)) {
|
||||
|
||||
@ -274,6 +274,7 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT];
|
||||
and AOF client */
|
||||
#define CLIENT_REPL_RDBONLY (1ULL<<42) /* This client is a replica that only wants
|
||||
RDB without replication buffer. */
|
||||
#define CLIENT_PREVENT_LOGGING (1ULL<<43) /* Prevent logging of command to slowlog */
|
||||
|
||||
/* Client block type (btype field in client structure)
|
||||
* if CLIENT_BLOCKED flag is set. */
|
||||
@ -2196,6 +2197,7 @@ void redisOpArrayInit(redisOpArray *oa);
|
||||
void redisOpArrayFree(redisOpArray *oa);
|
||||
void forceCommandPropagation(client *c, int flags);
|
||||
void preventCommandPropagation(client *c);
|
||||
void preventCommandLogging(client *c);
|
||||
void preventCommandAOF(client *c);
|
||||
void preventCommandReplication(client *c);
|
||||
int prepareForShutdown(int flags);
|
||||
|
||||
@ -41,6 +41,22 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} {
|
||||
assert_equal {foobar} [lindex $e 5]
|
||||
}
|
||||
|
||||
test {SLOWLOG - Certain commands are omitted that contain sensitive information} {
|
||||
r config set slowlog-log-slower-than 0
|
||||
r slowlog reset
|
||||
r config set masterauth ""
|
||||
r acl setuser slowlog-test-user
|
||||
r config set slowlog-log-slower-than 0
|
||||
r config set slowlog-log-slower-than 10000
|
||||
set slowlog_resp [r slowlog get]
|
||||
|
||||
# Make sure normal configs work, but the two sensitive
|
||||
# commands are omitted
|
||||
assert_equal 2 [llength $slowlog_resp]
|
||||
assert_equal {slowlog reset} [lindex [lindex [r slowlog get] 1] 3]
|
||||
assert_equal {config set slowlog-log-slower-than 0} [lindex [lindex [r slowlog get] 0] 3]
|
||||
}
|
||||
|
||||
test {SLOWLOG - Rewritten commands are logged as their original command} {
|
||||
r config set slowlog-log-slower-than 0
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user