This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch harden_credentials in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit b81fc83a93b2757dd2e6586af8096bd7201ba797 Author: Stephen Webb <[email protected]> AuthorDate: Thu Aug 20 15:17:35 2026 +1000 Change SMTP and Telnet appenders to be secure-by-default --- src/main/cpp/propertysetter.cpp | 14 +++++- src/main/cpp/smtpappender.cpp | 25 ++++++++++- src/main/cpp/telnetappender.cpp | 4 +- src/main/include/log4cxx/net/telnetappender.h | 9 +++- src/test/cpp/net/smtpappendertestcase.cpp | 51 ++++++++++++++++++---- src/test/resources/input/xml/smtpAppenderValid.xml | 5 ++- 6 files changed, 94 insertions(+), 14 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 496c3d53..faf52224 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); } } @@ -441,6 +456,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()) @@ -599,6 +616,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); @@ -777,7 +800,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/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..087fd30a 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(); @@ -212,8 +213,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=\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..0267dbbf 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="SMTPUserPassword" 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">
