This is an automated email from the ASF dual-hosted git repository.
swebb2066 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
The following commit(s) were added to refs/heads/master by this push:
new 123953f3 Change SMTP and Telnet appenders to be secure-by-default
(#734)
123953f3 is described below
commit 123953f3d274fe98cbf779fb693f640051548800
Author: Stephen Webb <[email protected]>
AuthorDate: Sat Aug 22 10:35:07 2026 +1000
Change SMTP and Telnet appenders to be secure-by-default (#734)
---
src/main/cpp/propertysetter.cpp | 14 +++++-
src/main/cpp/smtpappender.cpp | 25 +++++++++-
src/main/cpp/telnetappender.cpp | 4 +-
src/main/include/log4cxx/net/smtpappender.h | 1 +
src/main/include/log4cxx/net/telnetappender.h | 9 +++-
src/test/cpp/net/smtpappendertestcase.cpp | 54 ++++++++++++++++++----
src/test/resources/input/xml/smtpAppenderValid.xml | 5 +-
7 files changed, 96 insertions(+), 16 deletions(-)
diff --git a/src/main/cpp/propertysetter.cpp b/src/main/cpp/propertysetter.cpp
index 7147ded4..1b0a3a09 100644
--- a/src/main/cpp/propertysetter.cpp
+++ b/src/main/cpp/propertysetter.cpp
@@ -20,6 +20,7 @@
#include <log4cxx/helpers/optionconverter.h>
#include <log4cxx/spi/optionhandler.h>
#include <log4cxx/helpers/properties.h>
+#include <log4cxx/helpers/stringhelper.h>
#include <log4cxx/appender.h>
@@ -81,8 +82,19 @@ void PropertySetter::setProperty(const LogString& option,
const LogString& value
{
if (LogLog::isDebugEnabled())
{
+ // Do not echo secret-bearing option values (e.g. the
ODBCAppender
+ // "Password", SMTPAppender "SMTPPassword" or DBAppender
+ // "DriverParams" options) into diagnostic output:
internal debug
+ // output routinely flows into console/log pipelines
with weaker
+ // access control than the configuration file itself.
+ LogString
lowerOption(StringHelper::toLowerCase(option));
+ bool sensitive =
+ lowerOption.find(LOG4CXX_STR("password")) !=
LogString::npos ||
+ lowerOption.find(LOG4CXX_STR("driverparams"))
!= LogString::npos;
LogLog::debug(LOG4CXX_STR("Setting option name=[") +
- option + LOG4CXX_STR("], value=[") + value +
LOG4CXX_STR("]"));
+ option + LOG4CXX_STR("], value=[") +
+ (sensitive ? LogString(LOG4CXX_STR("****")) :
value) +
+ LOG4CXX_STR("]"));
}
OptionHandlerPtr handler = LOG4CXX_NS::cast<OptionHandler>(obj);
handler->setOption(option, value);
diff --git a/src/main/cpp/smtpappender.cpp b/src/main/cpp/smtpappender.cpp
index 02b5a2e0..5b39a620 100644
--- a/src/main/cpp/smtpappender.cpp
+++ b/src/main/cpp/smtpappender.cpp
@@ -101,6 +101,7 @@ class SMTPSession
int smtpPort,
const LogString& smtpUsername,
const LogString& smtpPassword,
+ bool allowPlainTextAuth,
Pool& p) : session(0), authctx(0),
user(toAscii(smtpUsername, p)),
pwd(toAscii(smtpPassword, p))
@@ -124,6 +125,20 @@ class SMTPSession
if (*user || *pwd)
{
+ // Secure by default: never send AUTH
credentials over an
+ // unencrypted connection. Require STARTTLS
before
+ // authenticating unless the operator
explicitly opted in
+ // to plain-text authentication via the
+ // AllowPlainTextAuthentication option.
+ if (!allowPlainTextAuth &&
!smtp_starttls_enable(session, Starttls_REQUIRED))
+ {
+ // The destructor does not run when a
constructor throws
+ smtp_destroy_session(session);
+ auth_destroy_context(authctx);
+ throw Exception("SMTPAppender: STARTTLS
is unavailable in this libESMTP build;"
+ " refusing to send SMTP
credentials in clear text."
+ " Set
AllowPlainTextAuthentication=true to override.");
+ }
smtp_auth_set_context(session, authctx);
}
}
@@ -447,6 +462,8 @@ struct SMTPAppender::SMTPPriv : public
AppenderSkeletonPrivate
bool locationInfo;
helpers::CyclicBuffer cb;
spi::TriggeringEventEvaluatorPtr evaluator;
+ // Whether AUTH credentials may be sent without STARTTLS (see setOption)
+ bool allowPlainTextAuth{false};
};
#define _priv static_cast<SMTPPriv*>(m_priv.get())
@@ -605,6 +622,12 @@ void SMTPAppender::setOption(const LogString& option,
{
setSMTPPort(OptionConverter::toInt(value, 25));
}
+ else if (StringHelper::equalsIgnoreCase(option,
LOG4CXX_STR("ALLOWPLAINTEXTAUTHENTICATION"),
LOG4CXX_STR("allowplaintextauthentication")))
+ {
+ // Explicit opt-out from the STARTTLS-before-AUTH requirement;
+ // only for servers that cannot offer TLS on a trusted network.
+ _priv->allowPlainTextAuth = OptionConverter::toBoolean(value,
false);
+ }
else
{
AppenderSkeleton::setOption(option, value);
@@ -783,7 +806,7 @@ void SMTPAppender::sendBuffer(Pool& p)
_priv->layout->appendFooter(sbuf);
- SMTPSession session(_priv->smtpHost, _priv->smtpPort,
_priv->smtpUsername, _priv->smtpPassword, p);
+ SMTPSession session(_priv->smtpHost, _priv->smtpPort,
_priv->smtpUsername, _priv->smtpPassword, _priv->allowPlainTextAuth, p);
SMTPMessage message(session, _priv->from, _priv->to, _priv->cc,
_priv->bcc, _priv->subject, sbuf, p);
diff --git a/src/main/cpp/telnetappender.cpp b/src/main/cpp/telnetappender.cpp
index 9dd071ba..c8ab07d8 100644
--- a/src/main/cpp/telnetappender.cpp
+++ b/src/main/cpp/telnetappender.cpp
@@ -71,7 +71,9 @@ struct TelnetAppender::TelnetAppenderPriv : public
AppenderSkeletonPrivate
int port;
LogString hostname;
bool reuseAddress = false;
- bool nonBlocking = false;
+ // Secure default: never let an unauthenticated peer that stops reading
+ // block the logging pipeline (see setNonBlocking for the opt-out).
+ bool nonBlocking = true;
ConnectionList connections;
LogString encoding;
LOG4CXX_NS::helpers::CharsetEncoderPtr encoder;
diff --git a/src/main/include/log4cxx/net/smtpappender.h
b/src/main/include/log4cxx/net/smtpappender.h
index 55894982..5b6ebebf 100644
--- a/src/main/include/log4cxx/net/smtpappender.h
+++ b/src/main/include/log4cxx/net/smtpappender.h
@@ -130,6 +130,7 @@ class LOG4CXX_EXPORT SMTPAppender : public AppenderSkeleton
*/
void setOption(const LogString& option, const LogString& value)
override;
+ using spi::OptionHandler::activateOptions;
/**
\copybrief AppenderSkeleton::activateOptions()
diff --git a/src/main/include/log4cxx/net/telnetappender.h
b/src/main/include/log4cxx/net/telnetappender.h
index 4f1fc705..e8b690ef 100644
--- a/src/main/include/log4cxx/net/telnetappender.h
+++ b/src/main/include/log4cxx/net/telnetappender.h
@@ -103,7 +103,7 @@ class LOG4CXX_EXPORT TelnetAppender : public
AppenderSkeleton
MaxConnections | {int} | 20 |
Encoding |
C,UTF-8,UTF-16,UTF-16BE,UTF-16LE,646,US-ASCII,ISO646-US,ANSI_X3.4-1968,ISO-8859-1,ISO-LATIN-1
| UTF-8 |
ReuseAddress | True,False | False |
- NonBlocking | True,False | False |
+ NonBlocking | True,False | True |
\sa AppenderSkeleton::setOption()
*/
@@ -156,7 +156,12 @@ class LOG4CXX_EXPORT TelnetAppender : public
AppenderSkeleton
/**
Use \c newValue for the behaviour when the TCP send buffer (on
an accepted socket connection) is full.
- When true, the socket connection is closed if the write would
block.
+ When true (the default), the socket connection is closed if the
write would block.
+
+ Setting \c newValue to \c false is an explicit opt-out that
must only be
+ used when every telnet client is trusted to read promptly: a
blocking
+ connection lets a client that stops reading stall the send
indefinitely,
+ blocking every thread that logs through this appender.
\sa setOption
*/
diff --git a/src/test/cpp/net/smtpappendertestcase.cpp
b/src/test/cpp/net/smtpappendertestcase.cpp
index 5ff10c40..5fd89ccf 100644
--- a/src/test/cpp/net/smtpappendertestcase.cpp
+++ b/src/test/cpp/net/smtpappendertestcase.cpp
@@ -22,8 +22,15 @@
#include "../appenderskeletontestcase.h"
#include <log4cxx/xml/domconfigurator.h>
#include <log4cxx/logmanager.h>
+#include <log4cxx/file.h>
#include <log4cxx/simplelayout.h>
+#include <log4cxx/spi/configurator.h>
+#include <log4cxx/helpers/fileinputstream.h>
+#include <log4cxx/helpers/loglog.h>
+#include <log4cxx/helpers/properties.h>
#include <log4cxx/helpers/onlyonceerrorhandler.h>
+#include <log4cxx/helpers/system.h>
+#include <fstream>
namespace LOG4CXX_NS
{
@@ -79,13 +86,7 @@ class SMTPAppenderTestCase : public AppenderSkeletonTestCase
LOGUNIT_TEST(testSubjectStripsCRLF);
LOGUNIT_TEST(testAddressFieldsStripCRLF);
LOGUNIT_TEST(testCleanFieldsArePreserved);
-//#define LOG4CXX_TEST_EMAIL_AND_SMTP_HOST_ARE_IN_ENVIRONMENT_VARIABLES
-#ifdef LOG4CXX_TEST_EMAIL_AND_SMTP_HOST_ARE_IN_ENVIRONMENT_VARIABLES
- // This test requires the following environment variables:
- // LOG4CXX_TEST_EMAIL_RECIPIENT - where the email is sent
- // LOG4CXX_TEST_SMTP_HOST_NAME - the email server
- LOGUNIT_TEST(testValid);
-#endif
+ LOGUNIT_TEST(testWithSMTPServer);
LOGUNIT_TEST_SUITE_END();
@@ -126,8 +127,7 @@ class SMTPAppenderTestCase : public AppenderSkeletonTestCase
appender->setTo(LOG4CXX_STR("[email protected]"));
appender->setFrom(LOG4CXX_STR("[email protected]"));
appender->setLayout(std::make_shared<SimpleLayout>());
- Pool p;
- appender->activateOptions(p);
+ appender->activateOptions();
auto root = Logger::getRootLogger();
root->addAppender(appender);
LOG4CXX_INFO(root, "Hello, World.");
@@ -212,8 +212,42 @@ class SMTPAppenderTestCase : public
AppenderSkeletonTestCase
appender.getTo());
}
- void testValid()
+ void testWithSMTPServer()
{
+ LogString credentialsFileNameVar =
LOG4CXX_STR("SMTP_TEST_CREDENTIALS_FILE_PATH");
+ auto credentialsFileName =
helpers::System::getProperty(credentialsFileNameVar);
+ if (credentialsFileName.empty())
+ {
+ helpers::LogLog::warn(
+ LOG4CXX_STR("Set the " +
credentialsFileNameVar + " environment variable"
+ " and re-run testWithSMTPServer to
initialise it with the required values")
+ );
+ return;
+ }
+ File credentialsFile(credentialsFileName);
+ if (!credentialsFile.exists())
+ {
+ std::ofstream f(credentialsFileName);
+ const char credentials[] =
+ "LOG4CXX_TEST_EMAIL_RECIPIENT=\n"
+ "LOG4CXX_TEST_SMTP_HOST_NAME=\n"
+ "LOG4CXX_TEST_SMTP_HOST_PORT=587\n"
+ "LOG4CXX_TEST_SMTP_HOST_ACCOUNT=\n"
+
"LOG4CXX_TEST_SMTP_HOST_ACCOUNT_PASSWORD=\n"
+
"LOG4CXX_TEST_SMTP_HOST_PLAIN_TEXT_CREDENTIALS=false\n"
+ ;
+ f.write(credentials, sizeof (credentials));
+ f.close();
+ LOGUNIT_ASSERT(credentialsFile.exists());
+ helpers::LogLog::warn(LOG4CXX_STR("Enter
credentials into '") + credentialsFileName + LOG4CXX_STR("' and re-run
testWithSMTPServer"));
+ return;
+ }
+ auto credentialsStream =
std::make_shared<helpers::FileInputStream>(credentialsFile);
+ helpers::Properties& credentials =
spi::Configurator::properties();
+ credentials.load(credentialsStream);
+
LOGUNIT_ASSERT(!credentials.getProperty(LOG4CXX_STR("LOG4CXX_TEST_SMTP_HOST_NAME")).empty());
+
LOGUNIT_ASSERT(!credentials.getProperty(LOG4CXX_STR("LOG4CXX_TEST_SMTP_HOST_PORT")).empty());
+
LOGUNIT_ASSERT(!credentials.getProperty(LOG4CXX_STR("LOG4CXX_TEST_EMAIL_RECIPIENT")).empty());
auto status =
xml::DOMConfigurator::configure("input/xml/smtpAppenderValid.xml");
LOGUNIT_ASSERT_EQUAL(status,
spi::ConfigurationStatus::Configured);
auto root = Logger::getRootLogger();
diff --git a/src/test/resources/input/xml/smtpAppenderValid.xml
b/src/test/resources/input/xml/smtpAppenderValid.xml
index 7df3d148..9942b522 100644
--- a/src/test/resources/input/xml/smtpAppenderValid.xml
+++ b/src/test/resources/input/xml/smtpAppenderValid.xml
@@ -23,7 +23,10 @@
<param name="to" value="${LOG4CXX_TEST_EMAIL_RECIPIENT}" />
<param name="subject" value="Test message" />
<param name="SMTPHost" value="${LOG4CXX_TEST_SMTP_HOST_NAME}"/>
- <param name="SMTPPort" value="587"/>
+ <param name="SMTPPort" value="${LOG4CXX_TEST_SMTP_HOST_PORT}"/>
+ <param name="SMTPUserName" value="${LOG4CXX_TEST_SMTP_HOST_ACCOUNT}" />
+ <param name="SMTPPassword"
value="${LOG4CXX_TEST_SMTP_HOST_ACCOUNT_PASSWORD}" />
+ <param name="AllowPlainTextAuthentication"
value="${LOG4CXX_TEST_SMTP_HOST_PLAIN_TEXT_CREDENTIALS}" />
<triggeringPolicy
class="org.apache.log4j.net.SMTPAppenderTest$MockTriggeringEventEvaluator"/>
<layout class="org.apache.log4j.PatternLayout">
