From: Alan T. DeKok Date: Fri, 21 Jul 2023 18:40:14 +0000 (-0400) Subject: add and support suppress_secrets X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=328193eae3e0dac599ac7ac0ae0fdbeb100def1c;p=thirdparty%2Ffreeradius-server.git add and support suppress_secrets the default is to not suppress secrets. Printing secrets is suppressed if it's running as "-X" AND the configuration file says to suppress them. Otherwise the secrets are printed. this behavior means that for most configurations, administrators can see (and compare) the user passwords. However, if the admins want to suppress secrets, they can do so in their local configuration. --- diff --git a/raddb/radiusd.conf.in b/raddb/radiusd.conf.in index f01a96df284..4fad44caf83 100644 --- a/raddb/radiusd.conf.in +++ b/raddb/radiusd.conf.in @@ -302,6 +302,23 @@ log { # don't want to change this. # syslog_facility = daemon + + # Suppress "secret" values when printing them in debug mode. + # + # + # Setting this to "yes" means that the server prints a series + # of dots: + # + # ....... + # + # instead of the value, for attributes which contain secret + # information. e.g. User-Name, Tunnel-Password, etc. + # + # This configuration is disabled by default. It is extremely + # important for administrators to be able to debug user logins + # by seeing what is actually being sent. + # +# suppress_secrets = no } # diff --git a/src/bin/radiusd.c b/src/bin/radiusd.c index 54ea908891d..a06c9175263 100644 --- a/src/bin/radiusd.c +++ b/src/bin/radiusd.c @@ -346,6 +346,7 @@ int main(int argc, char *argv[]) default_log.dst = L_DST_NULL; default_log.fd = -1; default_log.print_level = true; + default_log.suppress_secrets = true; /* * Set the panic action and enable other debugging facilities @@ -450,6 +451,8 @@ int main(int argc, char *argv[]) config->spawn_workers = false; config->daemonize = false; fr_debug_lvl += 2; + if (fr_debug_lvl > 2) default_log.suppress_secrets = false; + do_stdout: default_log.dst = L_DST_STDOUT; default_log.fd = STDOUT_FILENO; @@ -457,6 +460,7 @@ int main(int argc, char *argv[]) case 'x': fr_debug_lvl++; + if (fr_debug_lvl > 2) default_log.suppress_secrets = false; break; default: @@ -597,6 +601,8 @@ int main(int argc, char *argv[]) */ if (main_config_init(config) < 0) EXIT_WITH_FAILURE; + if (!config->suppress_secrets) default_log.suppress_secrets = false; + /* * Check we're the only process using this config. */ diff --git a/src/lib/server/log.c b/src/lib/server/log.c index 0cc750339a6..b17645b4ef6 100644 --- a/src/lib/server/log.c +++ b/src/lib/server/log.c @@ -458,7 +458,11 @@ print_fmt: * running unit tests which generate errors under CI. */ va_copy(aq, ap); - fmt_exp = fr_vasprintf(pool, fmt, aq); + if (!log_dst->suppress_secrets) { + fmt_exp = fr_vasprintf(pool, fmt, aq); + } else { + fmt_exp = fr_vasprintf_secure(pool, fmt, aq); + } va_end(aq); /* diff --git a/src/lib/server/main_config.c b/src/lib/server/main_config.c index d15013028b3..43a701fd910 100644 --- a/src/lib/server/main_config.c +++ b/src/lib/server/main_config.c @@ -115,6 +115,7 @@ static const CONF_PARSER initial_log_config[] = { { FR_CONF_OFFSET("local_state_dir", FR_TYPE_STRING, main_config_t, local_state_dir), .dflt = "${prefix}/var"}, { FR_CONF_OFFSET("logdir", FR_TYPE_STRING, main_config_t, log_dir), .dflt = "${local_state_dir}/log"}, { FR_CONF_OFFSET("file", FR_TYPE_STRING, main_config_t, log_file), .dflt = "${logdir}/radius.log" }, + { FR_CONF_OFFSET("suppress_secrets", FR_TYPE_BOOL, main_config_t, suppress_secrets), .dflt = "no" }, CONF_PARSER_TERMINATOR }; diff --git a/src/lib/server/main_config.h b/src/lib/server/main_config.h index 916b125a80d..5be14f0891d 100644 --- a/src/lib/server/main_config.h +++ b/src/lib/server/main_config.h @@ -62,6 +62,7 @@ struct main_config_s { //!< timing out. bool drop_requests; //!< Administratively disable request processing. + bool suppress_secrets; //!< suppress secrets (or not) char const *log_dir; char const *local_state_dir; diff --git a/src/lib/util/log.h b/src/lib/util/log.h index 5be480d5b42..66821360c07 100644 --- a/src/lib/util/log.h +++ b/src/lib/util/log.h @@ -103,6 +103,8 @@ typedef struct { bool print_level; //!< sometimes we don't want log levels printed + bool suppress_secrets; //!< suppress secrets when printing to this destination + fr_log_timestamp_t timestamp; //!< Prefix log messages with timestamps. int fd; //!< File descriptor to write messages to. diff --git a/src/lib/util/print.c b/src/lib/util/print.c index b5dab8cfcc8..2680d1e3ec5 100644 --- a/src/lib/util/print.c +++ b/src/lib/util/print.c @@ -467,12 +467,12 @@ DIAG_OFF(format-nonliteral) * @param[in] ctx to allocate buffer in. * @param[in] fmt string. * @param[in] ap variadic argument list. - * @param[in] secret_rules rules for escaping value-boxes with a "secret" flag set. + * @param[in] suppress_secrets as described * @return * - The result of string interpolation. * - NULL if OOM. */ -static char *fr_vasprintf_internal(TALLOC_CTX *ctx, char const *fmt, va_list ap, fr_sbuff_escape_rules_t const *secret_rules) +static char *fr_vasprintf_internal(TALLOC_CTX *ctx, char const *fmt, va_list ap, bool suppress_secrets) { char const *p = fmt, *end = p + strlen(fmt), *fmt_p = p, *fmt_q = p; char *out = NULL, *out_tmp; @@ -668,12 +668,11 @@ static char *fr_vasprintf_internal(TALLOC_CTX *ctx, char const *fmt, va_list ap, * Value boxes get escaped as double-quoted strings, unless the value-box * in question is secret, AND we've been asked to hide secrets. * - * Note that the secret_rules only hides secrets of data type "string" - * and "octets", which should be good enough for most purposes. + * Note that the secret_rules only hides secrets of data type "string", + * which should be good enough for most purposes. */ if (*(p + 1) == 'V') { e_rules = &fr_value_escape_double; - if (in->secret) e_rules = secret_rules; } /* @@ -681,7 +680,10 @@ static char *fr_vasprintf_internal(TALLOC_CTX *ctx, char const *fmt, va_list ap, * string need to occur in the NULL ctx so we don't fragment * any pool associated with it. */ - if (in) { + if (in->secret && suppress_secrets) { + subst = talloc_typed_strdup(NULL, "<<< secret >>>"); + + } else if (in) { fr_value_box_aprint(NULL, &subst, in, e_rules); if (!subst) { talloc_free(out); @@ -845,12 +847,12 @@ static char *fr_vasprintf_internal(TALLOC_CTX *ctx, char const *fmt, va_list ap, char *fr_vasprintf(TALLOC_CTX *ctx, char const *fmt, va_list ap) { - return fr_vasprintf_internal(ctx, fmt, ap, &fr_value_escape_double); + return fr_vasprintf_internal(ctx, fmt, ap, false); } char *fr_vasprintf_secure(TALLOC_CTX *ctx, char const *fmt, va_list ap) { - return fr_vasprintf_internal(ctx, fmt, ap, &fr_value_escape_secret); + return fr_vasprintf_internal(ctx, fmt, ap, true); }