From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Tue, 4 Oct 2022 01:26:22 +0000 (-0400)
Subject: Just use the printf functions to produce tls errors
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e6b712d3a7294f60e13796885f272d0affd39fec;p=thirdparty%2Ffreeradius-server.git

Just use the printf functions to produce tls errors
---

diff --git a/src/lib/server/log.c b/src/lib/server/log.c
index 29cb0474444..8da13c585ff 100644
--- a/src/lib/server/log.c
+++ b/src/lib/server/log.c
@@ -233,7 +233,7 @@ void log_always(fr_log_t const *log, fr_log_type_t type,
  */
 inline bool log_rdebug_enabled(fr_log_lvl_t lvl, request_t const *request)
 {
-	if (!request->log.dst) return false;
+	if (!request || !request->log.dst) return false;
 
 	if (lvl <= request->log.lvl) return true;
 
diff --git a/src/lib/tls/log.c b/src/lib/tls/log.c
index 62fa35e765a..0e016c20c75 100644
--- a/src/lib/tls/log.c
+++ b/src/lib/tls/log.c
@@ -226,24 +226,23 @@ void _fr_tls_log_x509_objects(char const *file, int line,
 		}
 	}
 }
-DIAG_ON(used-but-marked-unused)
-DIAG_ON(DIAG_UNKNOWN_PRAGMAS)
 
 DIAG_OFF(format-nonliteral)
 /** Print errors in the TLS thread local error stack
  *
- * Drains the thread local OpenSSL error queue, and prints out errors.
+ * Drains the thread local OpenSSL error queue, and prints out the first error
+ * storing it in libfreeradius's error buffer.
  *
- * @param[in] request	The current request (may be NULL).
  * @param[in] msg	Error message describing the operation being attempted.
  * @param[in] ap	Arguments for msg.
+ * @parma[in] debug	If true, line numbers for errors will also be printed.
  * @return the number of errors drained from the stack.
  */
-static int fr_tls_log_error_va(request_t *request, char const *msg, va_list ap)
+static int tls_strerror_vasprintf(char const *msg, va_list ap, bool debug)
 {
 	unsigned long	error;
-	char		*p;
-	int		in_stack = 0;
+	char		*p = NULL;
+	int		drained = 0;
 	char		buffer[256];
 
 	int		line;
@@ -261,56 +260,58 @@ static int fr_tls_log_error_va(request_t *request, char const *msg, va_list ap)
 	if (!(flags & ERR_TXT_STRING)) data = NULL;
 
 	if (msg) {
-		p = talloc_vasprintf(request, msg, ap);
-
 		/*
-		 *	Single line mode (there's only one error)
+		 *	Print the error we were passed, and
+		 *	OpenSSL's error.
 		 */
-		if (error && !ERR_peek_error()) {
+		p = talloc_vasprintf(NULL, msg, ap);
+		if (error) {
 			ERR_error_string_n(error, buffer, sizeof(buffer));
 
-			/* Extra verbose */
-			if ((request && RDEBUG_ENABLED3) || DEBUG_ENABLED3) {
-				ROPTIONAL(REDEBUG, ERROR, "%s: %s[%i]:%s%c%s", p, file, line, buffer,
-					  data ? ':' : '\0', data ? data : "");
+			if (debug) {
+				fr_strerror_printf("%s: %s[%i]:%s%c%s",
+						   p, file, line, buffer, data ? ':' : '\0', data ? data : "");
 			} else {
-				ROPTIONAL(REDEBUG, ERROR, "%s: %s%c%s", p, buffer,
-					  data ? ':' : '\0', data ? data : "");
+				fr_strerror_printf("%s: %s%c%s", p, buffer, data ? ':' : '\0', data ? data : "");
 			}
-
 			talloc_free(p);
-
-			return 1;
-		}
-
+			drained++;
 		/*
 		 *	Print the error we were given, irrespective
 		 *	of whether there were any OpenSSL errors.
 		 */
-		ROPTIONAL(RERROR, ERROR, "%s", p);
-		talloc_free(p);
+		} else {
+			fr_strerror_printf("%s", p);
+			talloc_free(p);
+		}
+	} else if (error) {
+		ERR_error_string_n(error, buffer, sizeof(buffer));
+
+		if (debug) {
+			fr_strerror_printf("%s[%i]:%s%c%s", file, line, buffer, data ? ':' : '\0', data ? data : "");
+		} else {
+			fr_strerror_printf("%s%c%s", buffer, data ? ':' : '\0', data ? data : "");
+		}
+		drained++;
+	} else {
+		return 0;
 	}
 
-	/*
-	 *	Stack mode (there are multiple errors)
-	 */
-	if (!error) return 0;
-	do {
+	while ((error = ERR_get_error_all(&file, &line, &func, &data, &flags))) {
 		if (!(flags & ERR_TXT_STRING)) data = NULL;
 
 		ERR_error_string_n(error, buffer, sizeof(buffer));
-		/* Extra verbose */
-		if ((request && RDEBUG_ENABLED3) || DEBUG_ENABLED3) {
-			ROPTIONAL(REDEBUG, ERROR, "%s[%i]:%s%c%s", file, line, buffer,
-				  data ? ':' : '\0', data ? data : "");
+
+		if (debug) {
+			fr_strerror_printf_push("%s[%i]:%s%c%s",
+						file, line, buffer, data ? ':' : '\0', data ? data : "");
 		} else {
-			ROPTIONAL(REDEBUG, ERROR, "%s%c%s", buffer,
-				  data ? ':' : '\0', data ? data : "");
+			fr_strerror_printf_push("%s%c%s", buffer, data ? ':' : '\0', data ? data : "");
 		}
-		in_stack++;
-	} while ((error = ERR_get_error_all(&file, &line, &func, &data, &flags)));
+		drained++;
+	}
 
-	return in_stack;
+	return drained;
 }
 DIAG_ON(format-nonliteral)
 
@@ -407,8 +408,10 @@ int fr_tls_log_io_error(request_t *request, int err, char const *fmt, ...)
 	 */
 	case SSL_ERROR_SSL:
 		va_start(ap, fmt);
-		fr_tls_log_error_va(request, fmt, ap);
+		(void)tls_strerror_vasprintf(fmt, ap, RDEBUG_ENABLED3);
 		va_end(ap);
+
+		ROPTIONAL(RPERROR, PERROR, "");
 		return -1;
 
 	/*
@@ -433,114 +436,46 @@ int fr_tls_log_io_error(request_t *request, int err, char const *fmt, ...)
 	return 0;
 }
 
-/** Print errors in the TLS thread local error stack
+/** Wrapper around fr_strerror_printf to log error messages for library functions calling libssl
  *
- * Drains the thread local OpenSSL error queue, and prints out errors.
+ * @note Will only drain the first error.
  *
- * @param[in] request	The current request (may be NULL).
  * @param[in] msg	Error message describing the operation being attempted.
  * @param[in] ...	Arguments for msg.
  * @return the number of errors drained from the stack.
  */
-int fr_tls_log_error(request_t *request, char const *msg, ...)
+int fr_tls_log_strerror_printf(char const *msg, ...)
 {
 	va_list ap;
 	int ret;
 
 	va_start(ap, msg);
-	ret = fr_tls_log_error_va(request, msg, ap);
+	ret = tls_strerror_vasprintf(msg, ap, false);
 	va_end(ap);
 
 	return ret;
 }
 
-DIAG_OFF(format-nonliteral)
 /** Print errors in the TLS thread local error stack
  *
- * Drains the thread local OpenSSL error queue, and prints out the first error
- * storing it in libfreeradius's error buffer.
- *
- * @param[in] msg	Error message describing the operation being attempted.
- * @param[in] ap	Arguments for msg.
- * @return the number of errors drained from the stack.
- */
-static int tls_strerror_vasprintf(char const *msg, va_list ap)
-{
-	unsigned long	error;
-	char		*p = NULL;
-	int		drained = 0;
-	char		buffer[256];
-
-	int		line;
-	char const	*file;
-	char const	*func;
-	char const	*data;
-	int		flags = 0;
-
-	/*
-	 *	Pop the first error, so ERR_peek_error()
-	 *	can be used to determine if there are
-	 *	multiple errors.
-	 */
-	error = ERR_get_error_all(&file, &line, &func, &data, &flags);
-	if (!(flags & ERR_TXT_STRING)) data = NULL;
-
-	if (msg) {
-		/*
-		 *	Print the error we were passed, and
-		 *	OpenSSL's error.
-		 */
-		p = talloc_vasprintf(NULL, msg, ap);
-		if (error) {
-			ERR_error_string_n(error, buffer, sizeof(buffer));
-			fr_strerror_printf("%s: %s%c%s", p, buffer, data ? ':' : '\0', data ? data : "");
-			talloc_free(p);
-			drained++;
-		/*
-		 *	Print the error we were given, irrespective
-		 *	of whether there were any OpenSSL errors.
-		 */
-		} else {
-			fr_strerror_printf("%s", p);
-			talloc_free(p);
-		}
-	} else if (error) {
-		ERR_error_string_n(error, buffer, sizeof(buffer));
-		fr_strerror_printf("%s%c%s", buffer, data ? ':' : '\0', data ? data : "");
-		drained++;
-	} else {
-		return 0;
-	}
-
-	while ((error = ERR_get_error_all(&file, &line, &func, &data, &flags))) {
-		if (!(flags & ERR_TXT_STRING)) data = NULL;
-
-		ERR_error_string_n(error, buffer, sizeof(buffer));
-		fr_strerror_printf_push("%s%c%s", buffer, data ? ':' : '\0', data ? data : "");
-		drained++;
-	}
-
-	return drained;
-}
-DIAG_ON(format-nonliteral)
-
-/** Wrapper around fr_strerror_printf to log error messages for library functions calling libssl
- *
- * @note Will only drain the first error.
+ * Drains the thread local OpenSSL error queue, and prints out errors.
  *
+ * @param[in] request	The current request (may be NULL).
  * @param[in] msg	Error message describing the operation being attempted.
  * @param[in] ...	Arguments for msg.
  * @return the number of errors drained from the stack.
  */
-int fr_tls_log_strerror_printf(char const *msg, ...)
+int fr_tls_log_error(request_t *request, char const *msg, ...)
 {
 	va_list ap;
 	int ret;
 
 	va_start(ap, msg);
-	ret = tls_strerror_vasprintf(msg, ap);
+	ret = tls_strerror_vasprintf(msg, ap, RDEBUG_ENABLED3);
 	va_end(ap);
 
+	ROPTIONAL(RPERROR, PERROR, "");
+
 	return ret;
 }
 
diff --git a/src/lib/tls/log.h b/src/lib/tls/log.h
index d0064ef3763..040b31db692 100644
--- a/src/lib/tls/log.h
+++ b/src/lib/tls/log.h
@@ -56,10 +56,10 @@ void		_fr_tls_log_x509_objects(char const *file, int line,
 int		fr_tls_log_io_error(request_t *request, int err, char const *msg, ...)
 				    CC_HINT(format (printf, 3, 4));
 
-int		fr_tls_log_error(request_t *request, char const *msg, ...) CC_HINT(format (printf, 2, 3));
-
 int		fr_tls_log_strerror_printf(char const *msg, ...) CC_HINT(format (printf, 1, 2));
 
+int		fr_tls_log_error(request_t *request, char const *msg, ...)  CC_HINT(format (printf, 2, 3));
+
 void		tls_log_clear(void);
 
 /** Return a BIO that writes to the log of the specified request