diff --git a/src/changelog/3.5.0/329-ext-mail-tls-by-default.xml b/src/changelog/3.5.0/329-ext-mail-tls-by-default.xml new file mode 100644 index 000000000..73fd4b120 --- /dev/null +++ b/src/changelog/3.5.0/329-ext-mail-tls-by-default.xml @@ -0,0 +1,14 @@ + + + + + require transport security by default in the `log4net.Ext.Mail` `SmtpAppender`: its + `TransportSecurity` now defaults to `Required` instead of `None`, so body and credentials no + longer cross the network in cleartext (CWE-319). A relay without `STARTTLS` stops working on + upgrade; set `transportSecurity` to `None`, or `enableSsl` to `false`, to keep the old behaviour + (audit da18b6fd-f026, implemented by @FreeAndNil) + + diff --git a/src/log4net.Ext.Mail.Tests/Appender/SmtpAppenderTest.cs b/src/log4net.Ext.Mail.Tests/Appender/SmtpAppenderTest.cs index b3407a432..c11c8389e 100644 --- a/src/log4net.Ext.Mail.Tests/Appender/SmtpAppenderTest.cs +++ b/src/log4net.Ext.Mail.Tests/Appender/SmtpAppenderTest.cs @@ -201,30 +201,38 @@ public void NtlmAuthenticationUsesTheNtlmSaslMechanism() Assert.That(_transport.SaslMechanism!.Credentials.GetCredential(null, null).UserName, Is.EqualTo("user")); } + /// An operator who configures nothing must not get a cleartext session. [Test] - public void EnableSslOffConnectsWithoutTransportSecurity() + public void TransportSecurityDefaultsToRequired() + => Assert.That(CreateAppender().TransportSecurity, Is.EqualTo(SmtpTransportSecurity.Required)); + + /// The shorthand follows the default of the property it stands for. + [Test] + public void EnableSslDefaultsToTrue() + => Assert.That(CreateAppender().EnableSsl, Is.True); + + /// + /// The default port is 25, so the default must mean mandatory STARTTLS, not the downgradable Auto. + /// + [Test] + public void TheDefaultConnectionRequiresStartTls() { SmtpAppender appender = CreateAppender(); Append(appender); - Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.None)); + Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.StartTls)); } - /// - /// 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. - /// [Test] - public void EnableSslOnRequiresStartTls() + public void EnableSslOffConnectsWithoutTransportSecurity() { SmtpAppender appender = CreateAppender(); - appender.EnableSsl = true; + appender.EnableSsl = false; Append(appender); - Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.StartTls)); + Assert.That(_transport.SecureSocketOptions, Is.EqualTo(SecureSocketOptions.None)); } /// @@ -281,8 +289,8 @@ public void EnableSslIsAShorthandForTransportSecurity() { SmtpAppender appender = CreateAppender(); + appender.EnableSsl = false; 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)); @@ -290,8 +298,8 @@ public void EnableSslIsAShorthandForTransportSecurity() appender.TransportSecurity = SmtpTransportSecurity.StartTlsWhenAvailable; Assert.That(appender.EnableSsl, Is.True); - appender.EnableSsl = false; - Assert.That(appender.TransportSecurity, Is.EqualTo(SmtpTransportSecurity.None)); + appender.TransportSecurity = SmtpTransportSecurity.None; + Assert.That(appender.EnableSsl, Is.False); } [Test] @@ -619,7 +627,7 @@ public void DefaultConstructorUsesTheMailKitTransport() Assert.That(appender.Authentication, Is.EqualTo(SmtpAppender.SmtpAuthentication.None)); Assert.That(appender.SubjectEncoding, Is.EqualTo(Encoding.UTF8)); Assert.That(appender.BodyEncoding, Is.EqualTo(Encoding.UTF8)); - Assert.That(appender.EnableSsl, Is.False); + Assert.That(appender.EnableSsl, Is.True); } /// An unbounded send stalls every thread logging through the appender. diff --git a/src/log4net.Ext.Mail/Appender/SmtpAppender.cs b/src/log4net.Ext.Mail/Appender/SmtpAppender.cs index 1c2bd3da0..d104c178e 100644 --- a/src/log4net.Ext.Mail/Appender/SmtpAppender.cs +++ b/src/log4net.Ext.Mail/Appender/SmtpAppender.cs @@ -252,11 +252,11 @@ public string? Bcc /// are the same setting, so the one assigned last wins. /// /// - /// When , transport security is required: implicit TLS on port 465 and - /// STARTTLS on every other port. Connecting fails if the server does not offer TLS, - /// rather than continuing unencrypted, which matches the behaviour of - /// . Use when - /// the server needs something else. + /// When , which is the default, transport security is required: implicit + /// TLS on port 465 and STARTTLS on every other port. Connecting fails if the server does + /// not offer TLS, rather than continuing unencrypted, which matches the behaviour, though not + /// the default, of . Use + /// when the server needs something else. /// /// public bool EnableSsl @@ -270,7 +270,8 @@ public bool EnableSsl /// /// /// One of the values. The default is - /// . + /// ; set it to + /// to allow a cleartext connection. /// /// /// @@ -279,7 +280,7 @@ public bool EnableSsl /// STARTTLS is possible. /// /// - public SmtpTransportSecurity TransportSecurity { get; set; } = SmtpTransportSecurity.None; + public SmtpTransportSecurity TransportSecurity { get; set; } = SmtpTransportSecurity.Required; /// /// Gets or sets the reply-to e-mail address. diff --git a/src/log4net.Ext.Mail/README.md b/src/log4net.Ext.Mail/README.md index 0dfc23563..3c3c7ee58 100644 --- a/src/log4net.Ext.Mail/README.md +++ b/src/log4net.Ext.Mail/README.md @@ -64,10 +64,10 @@ Then configure log4net in your code: | Feature | Built-in | log4net.Ext.Mail.Appender.SmtpAppender | |---------|----------|----------------------------------------| | SMTP Client | `System.Net.Mail.SmtpClient` (deprecated) | MailKit | -| TLS Support | Limited | Full (Auto, Explicit, Implicit) | +| TLS Support | Limited | Required, ImplicitTls, StartTls, StartTlsWhenAvailable | | Modern Servers | Often fails | ✓ Recommended | | `smtpHost` | Optional | Required | -| `enableSsl` | true/false | true/false (negotiates Auto/None) | +| `enableSsl` | true/false (default: false) | true/false (default: true) | | `Ntlm` auth | Uses Windows logon | Requires explicit credentials | ## Configuration Options @@ -83,7 +83,8 @@ Then configure log4net in your code: - `bcc` - blind carbon copy recipients - `replyTo` - reply-to address - `port` - SMTP port (default: 25) -- `enableSsl` - negotiate TLS/STARTTLS (true/false, default: false) +- `enableSsl` - require TLS/STARTTLS (true/false, default: true) +- `transportSecurity` - `None`, `Required` (default), `ImplicitTls`, `StartTls` or `StartTlsWhenAvailable` - `authentication` - `None`, `Basic`, or `Ntlm` (default: `None`) - `username` - username for authentication - `password` - password for authentication 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 e28f30cd2..874063240 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 @@ -194,11 +194,11 @@ This example authenticates against a mail server that requires an encrypted conn The mail goes out under the appender lock, so an unresponsive server would otherwise stall logging. |`enableSsl` -|Whether to require transport security. Defaults to `false`. +|Whether to require transport security. Defaults to `true`. Shorthand for `transportSecurity`: `true` selects `Required`, `false` selects `None`. |`transportSecurity` -|How the connection is secured. One of `None` (default), `Required`, `ImplicitTls`, `StartTls` or `StartTlsWhenAvailable`. +|How the connection is secured. One of `Required` (default), `None`, `ImplicitTls`, `StartTls` or `StartTlsWhenAvailable`. See xref:#mailkit-smtpappender-transport-security[]. |`authentication` @@ -253,11 +253,11 @@ The two properties cannot disagree: whichever is assigned last wins. |Value |Description |`None` -|The connection is not encrypted. Equivalent to `enableSsl` set to `false`, and the default. +|The connection is not encrypted. Equivalent to `enableSsl` set to `false`, and the opt-out from the secure 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`. +`STARTTLS` on every other port. Equivalent to `enableSsl` set to `true`, and the default. |`ImplicitTls` |The session is encrypted before the SMTP greeting, whatever the port. @@ -298,7 +298,8 @@ server no longer stalls logging. Failures are reported to the error handler as b the logging call has returned, and mail still queued is lost if the process is killed. `sendQueueSize` and `enqueueTimeoutMillis` bound the queue. -`enableSsl` keeps its meaning, so a migrated configuration secures the connection exactly as before. +`enableSsl` keeps its meaning but not its default: it is `true` here and `false` in the legacy +appender, so a configuration that set neither option now requires transport security. 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.