From: Alan T. DeKok <aland@freeradius.org>
Date: Fri, 21 Jul 2023 17:41:54 +0000 (-0400)
Subject: define and use fr_vasprintf_secure()
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=91df57172140e7236b79ed12c69079078f5d2e10;p=thirdparty%2Ffreeradius-server.git

define and use fr_vasprintf_secure()

which escapes / omits the contents of any value-box which is
marked "secret".

Note that we _cannot_ do this omission in fr_value_box_print(),
as that function may be used multiple times internally.  For example,
converting a clear-text password to CHAP via an MD5 xlat.  Those
values should be passed through unchanged.

As a result, we can only omit secrets in a new function, which can
then be used in debug / log functions which are known to be sent
to the admin.

There are still some pieces missing.

%pM prints a value-box list, and the fr_value_box_list_aprint()
function does not take a flag for printing secrets (or not).

%pP prints a pair, and the fr_pair_aprint() function also does not
take a flag for printing secrets.

The configuration files store data in CONF_PAIRs, which don't use
value-boxes.  So any shared secret is still printed in debug mode.
---

diff --git a/src/lib/util/print.c b/src/lib/util/print.c
index 1b80e719f79..b5dab8cfcc8 100644
--- a/src/lib/util/print.c
+++ b/src/lib/util/print.c
@@ -467,11 +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.
  * @return
  *	- The result of string interpolation.
  *	- NULL if OOM.
  */
-char *fr_vasprintf(TALLOC_CTX *ctx, char const *fmt, va_list ap)
+static char *fr_vasprintf_internal(TALLOC_CTX *ctx, char const *fmt, va_list ap, fr_sbuff_escape_rules_t const *secret_rules)
 {
 	char const	*p = fmt, *end = p + strlen(fmt), *fmt_p = p, *fmt_q = p;
 	char		*out = NULL, *out_tmp;
@@ -663,7 +664,17 @@ char *fr_vasprintf(TALLOC_CTX *ctx, char const *fmt, va_list ap)
 				fr_value_box_t const *in = va_arg(ap_q, fr_value_box_t const *);
 				fr_sbuff_escape_rules_t const *e_rules = NULL;
 
-				if (*(p + 1) == 'V') e_rules = &fr_value_escape_double;
+				/*
+				 *	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.
+				 */
+				if (*(p + 1) == 'V') {
+					e_rules = &fr_value_escape_double;
+					if (in->secret) e_rules = secret_rules;
+				}
 
 				/*
 				 *	Allocations that are not part of the output
@@ -831,6 +842,18 @@ char *fr_vasprintf(TALLOC_CTX *ctx, char const *fmt, va_list ap)
 
 	return out;
 }
+
+char *fr_vasprintf(TALLOC_CTX *ctx, char const *fmt, va_list ap)
+{
+	return fr_vasprintf_internal(ctx, fmt, ap, &fr_value_escape_double);
+}
+
+char *fr_vasprintf_secure(TALLOC_CTX *ctx, char const *fmt, va_list ap)
+{
+	return fr_vasprintf_internal(ctx, fmt, ap, &fr_value_escape_secret);
+}
+
+
 DIAG_ON(format-nonliteral)
 
 /** Special version of asprintf which implements custom format specifiers
diff --git a/src/lib/util/print.h b/src/lib/util/print.h
index 0e4577f6a71..37f51ebc623 100644
--- a/src/lib/util/print.h
+++ b/src/lib/util/print.h
@@ -41,6 +41,7 @@ size_t		fr_snprint(char *out, size_t outlen, char const *in, ssize_t inlen, char
 size_t		fr_snprint_len(char const *in, ssize_t inlen, char quote);
 char		*fr_asprint(TALLOC_CTX *ctx, char const *in, ssize_t inlen, char quote);
 char		*fr_vasprintf(TALLOC_CTX *ctx, char const *fmt, va_list ap);
+char		*fr_vasprintf_secure(TALLOC_CTX *ctx, char const *fmt, va_list ap);
 char		*fr_asprintf(TALLOC_CTX *ctx, char const *fmt, ...) CC_HINT(format (printf, 2, 3));
 ssize_t 	fr_fprintf(FILE *stream, char const *fmt, ...) CC_HINT(format (printf, 2, 3));