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 bd35fe0d5d9d83150fed88e63fb7eb4e887174ea Author: Jan Friedrich <[email protected]> AuthorDate: Mon Aug 17 22:15:15 2026 +0200 add a TransportSecurity option to the MailKit SmtpAppender EnableSsl mapped to MailKit's SecureSocketOptions.Auto, which is opportunistic on every port other than 465. An attacker able to strip STARTTLS from the EHLO response silently downgraded the session to plaintext, taking the credentials passed to Authenticate and the log content with it, while the operator had asked for an encrypted connection. EnableSsl now requires transport security: implicit TLS on port 465 and mandatory STARTTLS elsewhere, so connecting fails when the server offers no TLS, as System.Net.Mail.SmtpClient.EnableSsl does. The appender ships for the first time in this release, so no configuration changes behaviour. Opportunistic STARTTLS is still reachable, but only by asking for it. The new TransportSecurity option carries the full set of modes and EnableSsl became a shorthand for it, so the two cannot disagree. TransportSecurity also covers a server expecting implicit TLS on a port other than 465, which neither Auto nor the legacy appender could reach. The option uses its own enum rather than MailKit's SecureSocketOptions, which is not CLS compliant and would have needed CLSCompliant(false) on the primary TLS setting. The remarks on EnableSsl offered a custom ISmtpTransport for finer control, which no caller can supply because both the interface and the constructor taking it are internal. Removed, since TransportSecurity is the answer now. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../309-require-tls-when-enablessl-is-set.xml | 15 ++++ .../Appender/SmtpAppenderTest.cs | 76 ++++++++++++++++++++- src/log4net.Ext.Mail/Appender/SmtpAppender.cs | 75 ++++++++++++++++---- .../Appender/SmtpTransportSecurity.cs | 79 ++++++++++++++++++++++ .../configuration/appenders/adonetappender.adoc | 2 +- .../configuration/appenders/smtpappender.adoc | 58 +++++++++++++++- .../configuration/appenders/telnetappender.adoc | 4 +- 7 files changed, 289 insertions(+), 20 deletions(-) diff --git a/src/changelog/3.4.0/309-require-tls-when-enablessl-is-set.xml b/src/changelog/3.4.0/309-require-tls-when-enablessl-is-set.xml new file mode 100644 index 00000000..0e279930 --- /dev/null +++ b/src/changelog/3.4.0/309-require-tls-when-enablessl-is-set.xml @@ -0,0 +1,15 @@ +<?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="added"> + <issue id="309" link="https://github.com/apache/logging-log4net/pull/309"/> + <description format="asciidoc"> + add a `TransportSecurity` option to the `log4net.Ext.Mail` `SmtpAppender` and make `EnableSsl` + a shorthand for it. `EnableSsl` requires transport security, selecting implicit TLS on port 465 and + mandatory `STARTTLS` elsewhere, so connecting fails when the server offers no TLS instead of + silently continuing in plaintext, as `System.Net.Mail.SmtpClient.EnableSsl` does. Opportunistic + `STARTTLS` remains available, but only by asking for it with + `TransportSecurity=StartTlsWhenAvailable` (audit 1231d72-f007) + </description> +</entry> diff --git a/src/log4net.Ext.Mail.Tests/Appender/SmtpAppenderTest.cs b/src/log4net.Ext.Mail.Tests/Appender/SmtpAppenderTest.cs index bb23df02..33810395 100644 --- a/src/log4net.Ext.Mail.Tests/Appender/SmtpAppenderTest.cs +++ b/src/log4net.Ext.Mail.Tests/Appender/SmtpAppenderTest.cs @@ -207,15 +207,87 @@ public void EnableSslOffConnectsWithoutTransportSecurity() Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.None)); } + /// <summary> + /// SecureSocketOptions.Auto is opportunistic away from port 465, so an attacker who strips + /// STARTTLS from the EHLO response downgrades the session to plaintext. Asking for SSL has to + /// mean mandatory STARTTLS. + /// </summary> [Test] - public void EnableSslOnNegotiatesTransportSecurity() + public void EnableSslOnRequiresStartTls() { SmtpAppender appender = CreateAppender(); appender.EnableSsl = true; Append(appender); - Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.Auto)); + Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.StartTls)); + } + + /// <summary> + /// Port 465 is implicit TLS: the session is encrypted before the SMTP greeting, so STARTTLS is + /// never offered there and must not be demanded. + /// </summary> + [Test] + public void EnableSslOnPort465UsesImplicitTls() + { + SmtpAppender appender = CreateAppender(); + appender.EnableSsl = true; + appender.Port = 465; + + Append(appender); + + Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.SslOnConnect)); + } + + /// <summary> + /// A server doing implicit TLS on a port other than 465 cannot be reached with Required, which + /// would try to negotiate STARTTLS there. + /// </summary> + [Test] + public void ImplicitTlsUsesSslOnConnectRegardlessOfThePort() + { + SmtpAppender appender = CreateAppender(); + appender.TransportSecurity = SmtpTransportSecurity.ImplicitTls; + appender.Port = 8465; + + Append(appender); + + Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.SslOnConnect)); + } + + /// <summary> + /// Opportunistic transport security stays available, but only for an operator who asks for it. + /// </summary> + [Test] + public void StartTlsWhenAvailableIsOpportunistic() + { + SmtpAppender appender = CreateAppender(); + appender.TransportSecurity = SmtpTransportSecurity.StartTlsWhenAvailable; + + Append(appender); + + Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.StartTlsWhenAvailable)); + } + + /// <summary> + /// EnableSsl and TransportSecurity are the same setting, so they cannot disagree. + /// </summary> + [Test] + public void EnableSslIsAShorthandForTransportSecurity() + { + SmtpAppender appender = CreateAppender(); + + Assert.That(appender.TransportSecurity, Is.EqualTo(SmtpTransportSecurity.None)); + Assert.That(appender.EnableSsl, Is.False); + + appender.EnableSsl = true; + Assert.That(appender.TransportSecurity, Is.EqualTo(SmtpTransportSecurity.Required)); + + appender.TransportSecurity = SmtpTransportSecurity.StartTlsWhenAvailable; + Assert.That(appender.EnableSsl, Is.True); + + appender.EnableSsl = false; + Assert.That(appender.TransportSecurity, Is.EqualTo(SmtpTransportSecurity.None)); } [Test] diff --git a/src/log4net.Ext.Mail/Appender/SmtpAppender.cs b/src/log4net.Ext.Mail/Appender/SmtpAppender.cs index 54ce6a91..e2d631a4 100644 --- a/src/log4net.Ext.Mail/Appender/SmtpAppender.cs +++ b/src/log4net.Ext.Mail/Appender/SmtpAppender.cs @@ -69,6 +69,12 @@ namespace log4net.Ext.Mail.Appender; /// </remarks> public class SmtpAppender : BufferingAppenderSkeleton { + /// <summary> + /// The port reserved for SMTP over implicit TLS, where TLS starts before the SMTP greeting + /// rather than being negotiated with <c>STARTTLS</c>. + /// </summary> + private const int ImplicitTlsPort = 465; + private readonly Func<ISmtpTransport> _transportFactory; /// <summary> @@ -239,18 +245,40 @@ internal SmtpAppender(Func<ISmtpTransport> transportFactory) /// </summary> /// <remarks> /// <para> - /// When <see langword="true"/>, <see cref="SecureSocketOptions.Auto"/> is used, which - /// negotiates implicit TLS or <c>STARTTLS</c> depending on the <see cref="Port"/> and - /// what the server advertises. When <see langword="false"/> the connection is not - /// encrypted at all (<see cref="SecureSocketOptions.None"/>), matching the behaviour of - /// <see cref="System.Net.Mail.SmtpClient.EnableSsl"/>. + /// This is a shorthand for <see cref="TransportSecurity"/>: setting it to + /// <see langword="true"/> selects <see cref="SmtpTransportSecurity.Required"/> and setting it to + /// <see langword="false"/> selects <see cref="SmtpTransportSecurity.None"/>. The two properties + /// are the same setting, so the one assigned last wins. /// </para> /// <para> - /// Use <see cref="SecureSocketOptions"/> directly via a custom - /// <see cref="ISmtpTransport"/> if you need finer control. + /// When <see langword="true"/>, transport security is required: implicit TLS on port 465 and + /// <c>STARTTLS</c> on every other port. Connecting fails if the server does not offer TLS, + /// rather than continuing unencrypted, which matches the behaviour of + /// <see cref="System.Net.Mail.SmtpClient.EnableSsl"/>. Use <see cref="TransportSecurity"/> when + /// the server needs something else. + /// </para> + /// </remarks> + public bool EnableSsl + { + get => TransportSecurity != SmtpTransportSecurity.None; + set => TransportSecurity = value ? SmtpTransportSecurity.Required : SmtpTransportSecurity.None; + } + + /// <summary> + /// Gets or sets how the connection to the SMTP server is secured. + /// </summary> + /// <value> + /// One of the <see cref="SmtpTransportSecurity"/> values. The default is + /// <see cref="SmtpTransportSecurity.None"/>. + /// </value> + /// <remarks> + /// <para> + /// <see cref="EnableSsl"/> is a shorthand for this property and covers the usual cases; set this + /// one when the server expects implicit TLS on a port other than 465, or when only opportunistic + /// <c>STARTTLS</c> is possible. /// </para> /// </remarks> - public bool EnableSsl { get; set; } + public SmtpTransportSecurity TransportSecurity { get; set; } = SmtpTransportSecurity.None; /// <summary> /// Gets or sets the reply-to e-mail address. @@ -315,6 +343,32 @@ protected override void SendBuffer(LoggingEvent[] events) /// </summary> protected override bool RequiresLayout => true; + /// <summary> + /// Translates <see cref="TransportSecurity"/> into the mail library's own representation. + /// </summary> + /// <returns>The transport security to connect with.</returns> + /// <remarks> + /// <para> + /// <see cref="SecureSocketOptions.Auto"/> is deliberately never used: away from port 465 it is + /// opportunistic, so an attacker who strips <c>STARTTLS</c> from the EHLO response silently + /// downgrades the session to plaintext, taking the credentials and the log content with it. + /// Opportunistic behavior is available, but only by asking for it with + /// <see cref="SmtpTransportSecurity.StartTlsWhenAvailable"/>. + /// </para> + /// </remarks> + private SecureSocketOptions ResolveSecureSocketOptions() => TransportSecurity switch + { + SmtpTransportSecurity.None => SecureSocketOptions.None, + SmtpTransportSecurity.Required => Port == ImplicitTlsPort + ? SecureSocketOptions.SslOnConnect + : SecureSocketOptions.StartTls, + SmtpTransportSecurity.ImplicitTls => SecureSocketOptions.SslOnConnect, + SmtpTransportSecurity.StartTls => SecureSocketOptions.StartTls, + SmtpTransportSecurity.StartTlsWhenAvailable => SecureSocketOptions.StartTlsWhenAvailable, + _ => throw SystemInfo.CreateArgumentOutOfRangeException(nameof(TransportSecurity), TransportSecurity, + $"The value specified for TransportSecurity is not one of the {nameof(SmtpTransportSecurity)} values.") + }; + /// <summary> /// Send the email message /// </summary> @@ -324,10 +378,7 @@ protected virtual void SendEmail(string messageBody) using MimeMessage message = CreateMessage(messageBody); using ISmtpTransport transport = _transportFactory().EnsureNotNull(); - transport.Connect( - SmtpHost.EnsureNotNullOrEmpty(), - Port, - EnableSsl ? SecureSocketOptions.Auto : SecureSocketOptions.None); + transport.Connect(SmtpHost.EnsureNotNullOrEmpty(), Port, ResolveSecureSocketOptions()); try { switch (Authentication) diff --git a/src/log4net.Ext.Mail/Appender/SmtpTransportSecurity.cs b/src/log4net.Ext.Mail/Appender/SmtpTransportSecurity.cs new file mode 100644 index 00000000..a6ec293b --- /dev/null +++ b/src/log4net.Ext.Mail/Appender/SmtpTransportSecurity.cs @@ -0,0 +1,79 @@ +#region Apache License +// +// Licensed to the Apache Software Foundation (ASF) under one or more +// contributor license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright ownership. +// The ASF licenses this file to you under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +#endregion + +namespace log4net.Ext.Mail.Appender; + +/// <summary> +/// How <see cref="SmtpAppender"/> secures its connection to the SMTP server. +/// </summary> +/// <remarks> +/// <para> +/// This mirrors the transport security modes of the underlying mail library without exposing its +/// types, so that the appender's configuration surface stays CLS compliant. +/// </para> +/// </remarks> +public enum SmtpTransportSecurity +{ + /// <summary> + /// The connection is not encrypted. + /// </summary> + None, + + /// <summary> + /// Transport security is required, and the mechanism follows the port: implicit TLS on port 465, + /// <c>STARTTLS</c> on every other port. + /// </summary> + /// <remarks> + /// <para> + /// Connecting fails when the server does not offer transport security, rather than continuing + /// unencrypted. This is what <see cref="SmtpAppender.EnableSsl"/> selects and is the right + /// choice unless the server does something unusual. + /// </para> + /// </remarks> + Required, + + /// <summary> + /// The session is encrypted before the SMTP greeting, without <c>STARTTLS</c>. + /// </summary> + /// <remarks> + /// <para> + /// Use this for a server that expects implicit TLS on a port other than 465, which + /// <see cref="Required"/> would try to negotiate with <c>STARTTLS</c> instead. + /// </para> + /// </remarks> + ImplicitTls, + + /// <summary> + /// <c>STARTTLS</c> is required, whatever the port. + /// </summary> + StartTls, + + /// <summary> + /// <c>STARTTLS</c> is used when the server advertises it, and the connection continues + /// unencrypted when it does not. + /// </summary> + /// <remarks> + /// <para> + /// This is opportunistic: an attacker who can modify the traffic can remove the server's + /// <c>STARTTLS</c> advertisement and the session then proceeds in plaintext, exposing the + /// credentials and the log content. Only choose it for a network where that is acceptable. + /// </para> + /// </remarks> + StartTlsWhenAvailable +} diff --git a/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/adonetappender.adoc b/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/adonetappender.adoc index 06e91c5b..37a43137 100644 --- a/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/adonetappender.adoc +++ b/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/adonetappender.adoc @@ -35,7 +35,7 @@ Always configure `CommandText` together with `parameter` elements, as every exam If `CommandText` is omitted, the appender falls back to a legacy mode in which the rendered `Layout` output *is* the SQL statement that gets executed. Layouts perform no SQL quoting or escaping and offer no way to add it, so anything that reaches -a log statement -- a user name, a request parameter, an exception message -- is executed as part +a log statement (a user name, a request parameter, an exception message) is executed as part of the statement, with the privileges of the appender's connection. The appender logs an error at startup when it is configured this way. ==== diff --git a/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/smtpappender.adoc b/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/smtpappender.adoc index 2f5ca1ee..0c33679b 100644 --- a/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/smtpappender.adoc +++ b/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/smtpappender.adoc @@ -190,7 +190,12 @@ This example authenticates against a mail server that requires an encrypted conn |The port the SMTP server listens on. Defaults to `25`. |`enableSsl` -|Whether to secure the connection. Defaults to `false`. See xref:#mailkit-smtpappender-differences[]. +|Whether to require transport security. Defaults to `false`. +Shorthand for `transportSecurity`: `true` selects `Required`, `false` selects `None`. + +|`transportSecurity` +|How the connection is secured. One of `None` (default), `Required`, `ImplicitTls`, `StartTls` or `StartTlsWhenAvailable`. +See xref:#mailkit-smtpappender-transport-security[]. |`authentication` |One of `None` (default), `Basic` or `Ntlm`. `Basic` and `Ntlm` both require `username` and `password`. @@ -222,17 +227,64 @@ This example authenticates against a mail server that requires an encrypted conn `to`, `cc` and `bcc` also accept semicolons as separators, and quoted display names such as `"Doe, John" <[email protected]>`. +[#mailkit-smtpappender-transport-security] +=== Transport security + +`enableSsl` covers the usual cases and behaves as it does in the legacy appender: `true` requires +transport security, so connecting fails when the server does not offer it rather than continuing +unencrypted. + +`transportSecurity` is the same setting expressed precisely, for servers that need something else. +The two properties cannot disagree: whichever is assigned last wins. + +[cols="Value,Description"] +|=== +|Value |Description + +|`None` +|The connection is not encrypted. Equivalent to `enableSsl` set to `false`, and the default. + +|`Required` +|Transport security is required and the mechanism follows the port: implicit TLS on port 465, +`STARTTLS` on every other port. Equivalent to `enableSsl` set to `true`. + +|`ImplicitTls` +|The session is encrypted before the SMTP greeting, whatever the port. +Use this for a server expecting implicit TLS on a port other than 465, which `Required` would try +to negotiate with `STARTTLS` instead. + +|`StartTls` +|`STARTTLS` is required, whatever the port. + +|`StartTlsWhenAvailable` +|`STARTTLS` is used when the server advertises it, and the connection continues unencrypted when it +does not. +|=== + +[WARNING] +==== +`StartTlsWhenAvailable` is opportunistic. +An attacker able to modify the traffic can remove the server's `STARTTLS` advertisement, and the +session then proceeds in plaintext, exposing the credentials sent by `authentication` and the log +content itself. +Only choose it for a network where that is acceptable, and prefer `Required`. +==== + [#mailkit-smtpappender-differences] === Differences from the legacy appender The options above are named exactly as in the legacy appender, but a few behave differently: * `smtpHost` is *required*. MailKit has no notion of a machine-wide default SMTP server, so there is nothing to fall back on when the option is omitted. -* `enableSsl` set to `true` negotiates transport security automatically: implicit TLS on port 465, otherwise `STARTTLS` when the server advertises it. -Set to `false`, the connection is not encrypted at all. * `authentication` set to `Ntlm` requires `username` and `password`. MailKit cannot reuse the Windows logon session of the current thread or process, which the legacy appender did. * Semicolon-delimited recipient lists in `to`, `cc` and `bcc` are parsed correctly. +* `transportSecurity` has no counterpart in the legacy appender, which offers only `enableSsl`. + +`enableSsl` keeps its meaning, so a migrated configuration secures the connection exactly as before. +If the legacy appender reached your server with `enableSsl` set to `true`, so does this one. +The new `transportSecurity` option is only needed for a server that the legacy appender could not +reach either, such as one expecting implicit TLS on a port other than 465. [#legacy-smtpappender] == Built-in SmtpAppender (deprecated) diff --git a/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/telnetappender.adoc b/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/telnetappender.adoc index 27891b57..6ce35769 100644 --- a/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/telnetappender.adoc +++ b/src/site/antora/modules/ROOT/pages/manual/configuration/appenders/telnetappender.adoc @@ -23,7 +23,7 @@ every connected client, so that a running application's log can be watched over telnet client. Unlike every other appender, it does not write to a destination you configure: it accepts connections from clients that reach it. -It is intended for diagnostic use on trusted networks -- see <<telnetappender-trust>>. +It is intended for diagnostic use on trusted networks; see <<telnetappender-trust>>. At most 20 clients may be connected at the same time; further connection attempts are answered with a message and closed. @@ -78,7 +78,7 @@ interfaces*. There is no option to restrict the listen address, require a credential, or enable TLS. Any client that can reach the port receives the full rendered log stream, including whatever the -layout renders -- user names, session identifiers, request parameters, stack traces. +layout renders: user names, session identifiers, request parameters, stack traces. Keeping untrusted parties away from the port is the operator's responsibility, exactly as it is for a log file:
