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 f01a96df28432a5a488804b24a27e0b6c987bfcb..4fad44caf83a262f442966f8099123192942ffa3 100644 (file)
--- 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 54ea908891d13916164dcb25be5a7d429851b9dc..a06c917526346b36a4acc9378df78883611f4b35 100644 (file)
--- 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 0cc750339a6ad415b9efb3406eefac519f49c5d1..b17645b4ef67d9c441d7b96309d2e84ae6a621a2 100644 (file)
--- 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 d15013028b3f7a24decf658e6bc0d7989158c1a5..43a701fd91031e1267134e142b774ac616850dfd 100644 (file)
--- 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 916b125a80d89c8565195970060c2d0a00ad60cb..5be14f0891d19d904328875ff0727238dfb46b63 100644 (file)
--- 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 5be480d5b42d29fdcfed693d3ca4664d9f6620e6..66821360c07a1e95cc9cf1a19f5ced4861e2a9e2 100644 (file)
--- 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 b5dab8cfcc870da33337438285d3943941ec8f47..2680d1e3ec5a3369c9e051528dba6d1c6b0275c0 100644 (file)
--- 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);
 }