You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by fr...@apache.org on 2026/08/17 22:10:22 UTC
(logging-log4net) 05/19: redact the password when reporting a failed database connection
This is an automated email from the ASF dual-hosted git repository.
FreeAndNil pushed a commit to branch Feature/security-audit-hardening
in repository https://gitbox.apache.org/repos/asf/logging-log4net.git
commit e80b3810acb3a2c9cb9788dab1ef1b9e4aedcf42
Author: Jan Friedrich <fr...@apache.org>
AuthorDate: Mon Aug 17 21:54:18 2026 +0200
redact the password when reporting a failed database connection
InitializeDatabaseConnection named the resolved connection string in full
when it could not open the connection, and the documented examples embed
Password=... The message goes through the ErrorHandler, so it is what an
operator sees while diagnosing exactly this failure.
Password-bearing keywords are now replaced with *****. The rest of the
connection string is kept, so the message stays useful for spotting a typo
in the server name or catalog. If the string cannot be parsed - likely,
given that it just failed to connect - all of it is redacted.
This matters more since appender errors became visible without
log4net.Internal.Debug: the password would otherwise have reached stderr in
a default configuration.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
...redact-password-in-connection-string-errors.xml | 13 +++++
.../Appender/AdoNet/Log4NetConnection.cs | 14 +++++-
src/log4net.Tests/Appender/AdoNetAppenderTest.cs | 40 ++++++++++++++++
src/log4net/Appender/AdoNetAppender.cs | 56 +++++++++++++++++++++-
4 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/src/changelog/3.4.0/309-redact-password-in-connection-string-errors.xml b/src/changelog/3.4.0/309-redact-password-in-connection-string-errors.xml
new file mode 100644
index 00000000..bcc07ebb
--- /dev/null
+++ b/src/changelog/3.4.0/309-redact-password-in-connection-string-errors.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="https://logging.apache.org/xml/ns"
+ xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+ type="fixed">
+ <issue id="309" link="https://github.com/apache/logging-log4net/pull/309"/>
+ <description format="asciidoc">
+ stop `AdoNetAppender` from repeating the password when it reports a connection it could not
+ open. The message named the resolved connection string in full, and the documented examples embed
+ `Password=...` (CWE-532). Password-bearing keywords are now replaced with `*****`, while the rest
+ of the connection string is kept so the message stays useful (audit 1231d72-f018)
+ </description>
+</entry>
diff --git a/src/log4net.Tests/Appender/AdoNet/Log4NetConnection.cs b/src/log4net.Tests/Appender/AdoNet/Log4NetConnection.cs
index e11411b0..43364c51 100644
--- a/src/log4net.Tests/Appender/AdoNet/Log4NetConnection.cs
+++ b/src/log4net.Tests/Appender/AdoNet/Log4NetConnection.cs
@@ -44,7 +44,19 @@ internal sealed class Log4NetConnection : IDbConnection
public IDbCommand CreateCommand() => new Log4NetCommand();
- public void Open() => _open = true;
+ public void Open()
+ {
+ if (FailOnOpen)
+ {
+ throw new InvalidOperationException("Simulated failure to open the connection");
+ }
+ _open = true;
+ }
+
+ /// <summary>
+ /// When set, <see cref="Open"/> throws, simulating a connection that cannot be established.
+ /// </summary>
+ public static bool FailOnOpen { get; set; }
public static Log4NetConnection? MostRecentInstance { get; private set; }
diff --git a/src/log4net.Tests/Appender/AdoNetAppenderTest.cs b/src/log4net.Tests/Appender/AdoNetAppenderTest.cs
index a07744c8..3e391586 100644
--- a/src/log4net.Tests/Appender/AdoNetAppenderTest.cs
+++ b/src/log4net.Tests/Appender/AdoNetAppenderTest.cs
@@ -265,6 +265,46 @@ public void BufferingWebsiteExample()
Assert.That(param.Value, Is.Empty);
}
+ /// <summary>
+ /// The message reporting a failed connection must not repeat the password from the connection
+ /// string. The appender reports the failure through its ErrorHandler, so this message is what
+ /// an operator sees on stderr in a default configuration.
+ /// </summary>
+ [Test]
+ [NonParallelizable]
+ public void FailedConnectionDoesNotReportThePassword()
+ {
+ const string password = "H0rseBatteryStaple";
+ List<LogLog> messages = [];
+ try
+ {
+ Log4NetConnection.FailOnOpen = true;
+ LogLog.ExecuteWithoutEmittingInternalMessages(() =>
+ {
+ using LogLog.LogReceivedAdapter _ = new(messages);
+ AdoNetAppender adoNetAppender = new()
+ {
+ BufferSize = -1,
+ ConnectionType = typeof(Log4NetConnection).AssemblyQualifiedName!,
+ ConnectionString = $"data source=someserver;initial catalog=somedb;User ID=someuser;Password={password}",
+ CommandText = "INSERT INTO Log ([Message]) VALUES (@message)"
+ };
+ adoNetAppender.ActivateOptions();
+ });
+
+ string reported = string.Join(Environment.NewLine, messages.ConvertAll(m => m.Message));
+
+ Assert.That(reported, Does.Not.Contain(password));
+ Assert.That(reported, Does.Contain("Could not open database connection"));
+ // The rest of the connection string survives, so the message stays useful for diagnosis.
+ Assert.That(reported, Does.Contain("someserver"));
+ }
+ finally
+ {
+ Log4NetConnection.FailOnOpen = false;
+ }
+ }
+
/// <summary>
/// Without CommandText the rendered Layout is executed as the SQL statement, which is open
/// to SQL injection from logged content. Activation has to say so.
diff --git a/src/log4net/Appender/AdoNetAppender.cs b/src/log4net/Appender/AdoNetAppender.cs
index 0faf995e..b4cd2209 100644
--- a/src/log4net/Appender/AdoNetAppender.cs
+++ b/src/log4net/Appender/AdoNetAppender.cs
@@ -21,6 +21,7 @@
using System.Collections.Generic;
using System.Configuration;
using System.Data;
+using System.Data.Common;
using System.IO;
using log4net.Util;
@@ -788,12 +789,65 @@ private void InitializeDatabaseConnection()
catch (Exception e) when (!e.IsFatal())
{
// Sadly, your connection string is bad.
- ErrorHandler.Error($"Could not open database connection [{resolvedConnectionString}]. Connection string context [{connectionStringContext}].", e);
+ ErrorHandler.Error($"Could not open database connection [{RedactConnectionString(resolvedConnectionString)}]. Connection string context [{connectionStringContext}].", e);
Connection = null;
}
}
+ /// <summary>
+ /// Replaces the values of password-bearing keywords in a connection string with
+ /// <see cref="RedactedValue"/>, so that it can be named in a diagnostic message.
+ /// </summary>
+ /// <param name="connectionString">The connection string to redact.</param>
+ /// <returns>
+ /// The connection string with every password value replaced, or <see cref="RedactedValue"/> if it
+ /// could not be parsed.
+ /// </returns>
+ private static string RedactConnectionString(string connectionString)
+ {
+ if (string.IsNullOrEmpty(connectionString))
+ {
+ return connectionString;
+ }
+
+ try
+ {
+ DbConnectionStringBuilder builder = new() { ConnectionString = connectionString };
+
+ List<string> keys = [];
+ foreach (string key in builder.Keys)
+ {
+ keys.Add(key);
+ }
+
+ foreach (string key in keys)
+ {
+ // Providers spell the secret differently - Password, PWD, User Password - so match on
+ // the keyword rather than on a fixed list.
+ if (key.IndexOf("password", StringComparison.OrdinalIgnoreCase) >= 0
+ || key.Equals("pwd", StringComparison.OrdinalIgnoreCase))
+ {
+ builder[key] = RedactedValue;
+ }
+ }
+
+ return builder.ConnectionString;
+ }
+ catch (Exception e) when (!e.IsFatal())
+ {
+ // The connection string could not be parsed - which is likely, given that it just failed
+ // to connect - so redact all of it rather than risk echoing a password.
+ LogLog.Debug(_declaringType, "Could not parse the connection string in order to redact it", e);
+ return RedactedValue;
+ }
+ }
+
+ /// <summary>
+ /// Stands in for a password in diagnostic messages.
+ /// </summary>
+ private const string RedactedValue = "*****";
+
/// <summary>
/// Cleanup the existing connection.
/// </summary>