From bcc45c271b8c462af0c835a2900d89f4c8547a6d Mon Sep 17 00:00:00 2001 From: Riku Virtanen Date: Wed, 20 Nov 2024 11:53:52 +0200 Subject: [PATCH 1/4] SMTP.SendEmail - Added AcceptAllCerts parameter --- Frends.SMTP.SendEmail/CHANGELOG.md | 4 ++ .../Frends.Smtp.Tests.csproj | 1 + .../SendEmailTests.cs | 58 ++++++++++++++++++- .../Definitions/Options.cs | 7 +++ .../Frends.Smtp.SendEmail.csproj | 8 ++- .../Frends.Smtp.SendEmail/SendEmailTask.cs | 32 ++++++---- 6 files changed, 97 insertions(+), 13 deletions(-) diff --git a/Frends.SMTP.SendEmail/CHANGELOG.md b/Frends.SMTP.SendEmail/CHANGELOG.md index 984ac6d..27e4a6a 100644 --- a/Frends.SMTP.SendEmail/CHANGELOG.md +++ b/Frends.SMTP.SendEmail/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## [1.3.0] - 2024-09-30 +### Added +- [Breaking] Added parameter AcceptAllCerts which will make the Task to accept all server certifications. + ## [1.2.1] - 2024-01-02 ### Fixed - Fixed issue with connecting to SMTP servers which do not support authentication. diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/Frends.Smtp.Tests.csproj b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/Frends.Smtp.Tests.csproj index 51f3d35..ce8abc2 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/Frends.Smtp.Tests.csproj +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/Frends.Smtp.Tests.csproj @@ -18,6 +18,7 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive + diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs index 6d1b335..7943b37 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs @@ -1,8 +1,13 @@ using NUnit.Framework; +using Moq; using System; using System.IO; using System.Threading.Tasks; using Frends.SMTP.SendEmail.Definitions; +using MailKit.Net.Smtp; +using MailKit.Security; +using System.Net.Security; +using System.Threading; namespace Frends.SMTP.SendEmail.Tests; @@ -75,7 +80,8 @@ public void EmailTestSetup() SMTPServer = SMTPADDRESS, Port = PORT, UseOAuth2 = false, - SecureSocket = SecureSocketOption.None + SecureSocket = SecureSocketOption.None, + AcceptAllCerts = false }; } @@ -97,6 +103,19 @@ public async Task SendEmailWithPlainText() Assert.IsTrue(result.EmailSent); } + [Test] + public async Task EmailTestAcceptAllCertifications() + { + _options.SecureSocket = SecureSocketOption.StartTls; + _options.AcceptAllCerts = true; + + var input = _input; + input.Subject = "Email test - PlainText"; + + var result = await SMTP.SendEmail(input, null, _options, default); + Assert.IsTrue(result.EmailSent); + } + [Test] public async Task SendEmailWithFileAttachment() { @@ -190,4 +209,41 @@ public void TrySendingEmailWithNoFileAttachmentFoundException() var ex = Assert.ThrowsAsync(async () => await SMTP.SendEmail(input, Attachments, _options, default)); Assert.AreEqual(@$"The given filepath '{attachment.FilePath}' had no matching files", ex.Message); } + + [Test] + public async Task TestAcceptAllCerts() + { + var options = new Options + { + AcceptAllCerts = true, + SMTPServer = "smtp.example.com", + Port = 587, + SecureSocket = SecureSocketOption.StartTls, + UseOAuth2 = false, + }; + + var mockSmtpClient = new Mock { CallBase = true }; + + mockSmtpClient.Setup(client => client.ConnectAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())).Returns(Task.CompletedTask); + + mockSmtpClient.Setup(client => client.AuthenticateAsync( + It.IsAny(), + It.IsAny())).Returns(Task.CompletedTask); + + await SMTP.InitializeSmtpClient(options, default, mockSmtpClient.Object); + + // Assert + mockSmtpClient.Verify(client => client.ConnectAsync( + options.SMTPServer, + options.Port, + SecureSocketOptions.StartTls, + default), Times.Once); + + Assert.IsNotNull(mockSmtpClient.Object.ServerCertificateValidationCallback); + Assert.IsTrue(mockSmtpClient.Object.ServerCertificateValidationCallback.Invoke(null, null, null, SslPolicyErrors.None)); + } } diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Definitions/Options.cs b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Definitions/Options.cs index ed4f48d..393d138 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Definitions/Options.cs +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Definitions/Options.cs @@ -30,6 +30,13 @@ public class Options [DefaultValue(SecureSocketOption.Auto)] public SecureSocketOption SecureSocket { get; set; } + /// + /// Should the task accept all certificates from IMAP server, including invalid ones? + /// + /// true + [DefaultValue(false)] + public bool AcceptAllCerts { get; set; } + /// /// Set this true if SMTP server expectes OAuth token. /// diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Frends.Smtp.SendEmail.csproj b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Frends.Smtp.SendEmail.csproj index bf90403..c31391f 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Frends.Smtp.SendEmail.csproj +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Frends.Smtp.SendEmail.csproj @@ -2,7 +2,7 @@ net6.0 - 1.2.1 + 1.3.0 latest Frends Frends @@ -32,4 +32,10 @@ + + + <_Parameter1>$(MSBuildProjectName).Tests + + + diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs index 4fc5169..1159749 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs @@ -12,7 +12,9 @@ using System.Threading; using System.Threading.Tasks; using Frends.SMTP.SendEmail.Definitions; +using System.Runtime.CompilerServices; +[assembly: InternalsVisibleTo("Frends.SMTP.Tests")] namespace Frends.SMTP.SendEmail; /// /// Main class of the Task. @@ -96,11 +98,19 @@ public static async Task SendEmail([PropertyTab] Input input, [PropertyT /// /// Initializes new SmtpClient with given parameters. /// - private static async Task InitializeSmtpClient(Options settings, CancellationToken cancellationToken) + internal static async Task InitializeSmtpClient(Options options, CancellationToken cancellationToken, SmtpClient client = null) { - var client = new SmtpClient(); + client ??= new SmtpClient(); - var secureSocketOption = settings.SecureSocket switch + // Accept all certs? + if (options.AcceptAllCerts) + { +#pragma warning disable S4830 // Server certificates should be verified during SSL/TLS connections + client.ServerCertificateValidationCallback = (s, x509certificate, x590chain, sslPolicyErrors) => true; +#pragma warning restore S4830 // Server certificates should be verified during SSL/TLS connections + } + + var secureSocketOption = options.SecureSocket switch { SecureSocketOption.None => SecureSocketOptions.None, SecureSocketOption.SslOnConnect => SecureSocketOptions.SslOnConnect, @@ -109,16 +119,16 @@ private static async Task InitializeSmtpClient(Options settings, Can _ => SecureSocketOptions.Auto, }; - await client.ConnectAsync(settings.SMTPServer, settings.Port, secureSocketOption, cancellationToken); + await client.ConnectAsync(options.SMTPServer, options.Port, secureSocketOption, cancellationToken); SaslMechanism mechanism; - if (settings.UseOAuth2) - mechanism = new SaslMechanismOAuth2(settings.UserName, settings.Token); - else if (string.IsNullOrEmpty(settings.Password)) + if (options.UseOAuth2) + mechanism = new SaslMechanismOAuth2(options.UserName, options.Token); + else if (string.IsNullOrEmpty(options.Password)) return client; else - mechanism = new SaslMechanismLogin(new NetworkCredential(settings.UserName, settings.Password)); + mechanism = new SaslMechanismLogin(new NetworkCredential(options.UserName, options.Password)); await client.AuthenticateAsync(mechanism, cancellationToken); @@ -134,13 +144,13 @@ private static MimeMessage InitializeMimeMessage(Input input) var separators = new[] { ',', ';' }; MailboxAddress[] recipients = string.IsNullOrEmpty(input.To) - ? Array.Empty() + ? [] : input.To.Split(separators, StringSplitOptions.RemoveEmptyEntries).Select(x => MailboxAddress.Parse(x)).ToArray(); MailboxAddress[] ccRecipients = string.IsNullOrEmpty(input.Cc) - ? Array.Empty() + ? [] : input.Cc.Split(separators, StringSplitOptions.RemoveEmptyEntries).Select(x => MailboxAddress.Parse(x)).ToArray(); MailboxAddress[] bccRecipients = string.IsNullOrEmpty(input.Bcc) - ? Array.Empty() + ? [] : input.Bcc.Split(separators, StringSplitOptions.RemoveEmptyEntries).Select(x => MailboxAddress.Parse(x)).ToArray(); //Create mail object From 49f0b34996ecbe0e9393fd1a10043b10d22bcfd8 Mon Sep 17 00:00:00 2001 From: Riku Virtanen Date: Wed, 20 Nov 2024 11:57:35 +0200 Subject: [PATCH 2/4] Fixed build failures --- .../Frends.Smtp.SendEmail/SendEmailTask.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs index 1159749..f8cc66f 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs @@ -144,13 +144,13 @@ private static MimeMessage InitializeMimeMessage(Input input) var separators = new[] { ',', ';' }; MailboxAddress[] recipients = string.IsNullOrEmpty(input.To) - ? [] + ? Array.Empty() : input.To.Split(separators, StringSplitOptions.RemoveEmptyEntries).Select(x => MailboxAddress.Parse(x)).ToArray(); MailboxAddress[] ccRecipients = string.IsNullOrEmpty(input.Cc) - ? [] + ? Array.Empty() : input.Cc.Split(separators, StringSplitOptions.RemoveEmptyEntries).Select(x => MailboxAddress.Parse(x)).ToArray(); MailboxAddress[] bccRecipients = string.IsNullOrEmpty(input.Bcc) - ? [] + ? Array.Empty() : input.Bcc.Split(separators, StringSplitOptions.RemoveEmptyEntries).Select(x => MailboxAddress.Parse(x)).ToArray(); //Create mail object From 2b4fc988b195469158f9bd5d6f99e16b29b0b8bd Mon Sep 17 00:00:00 2001 From: Riku Virtanen Date: Wed, 20 Nov 2024 13:32:15 +0200 Subject: [PATCH 3/4] Fixed CodeRabbit review changes --- Frends.SMTP.SendEmail/CHANGELOG.md | 5 ++- .../SendEmailTests.cs | 34 ++++++++++++++++--- .../Definitions/Options.cs | 4 ++- .../Frends.Smtp.SendEmail/SendEmailTask.cs | 5 +++ 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/Frends.SMTP.SendEmail/CHANGELOG.md b/Frends.SMTP.SendEmail/CHANGELOG.md index 27e4a6a..4df71b8 100644 --- a/Frends.SMTP.SendEmail/CHANGELOG.md +++ b/Frends.SMTP.SendEmail/CHANGELOG.md @@ -2,7 +2,10 @@ ## [1.3.0] - 2024-09-30 ### Added -- [Breaking] Added parameter AcceptAllCerts which will make the Task to accept all server certifications. +- [Breaking] Added parameter AcceptAllCerts which allows bypassing SSL/TLS certificate validation for SMTP servers. + - Default value: false + - Warning: Enabling this option poses security risks. Only use when connecting to trusted SMTP servers with self-signed certificates. + - Usage: Enable only in controlled environments where certificate validation issues cannot be resolved through proper certificate installation. ## [1.2.1] - 2024-01-02 ### Fixed diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs index 7943b37..fd4d808 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs @@ -236,14 +236,38 @@ public async Task TestAcceptAllCerts() await SMTP.InitializeSmtpClient(options, default, mockSmtpClient.Object); - // Assert + // Verify connection parameters mockSmtpClient.Verify(client => client.ConnectAsync( - options.SMTPServer, + options.SMTPServer, options.Port, - SecureSocketOptions.StartTls, - default), Times.Once); + SecureSocketOptions.StartTls, + default), Times.Once); + // Verify certificate validation behavior Assert.IsNotNull(mockSmtpClient.Object.ServerCertificateValidationCallback); - Assert.IsTrue(mockSmtpClient.Object.ServerCertificateValidationCallback.Invoke(null, null, null, SslPolicyErrors.None)); + + var callback = mockSmtpClient.Object.ServerCertificateValidationCallback; + + // Test various SSL policy errors + Assert.Multiple(() => + { + Assert.IsTrue(callback.Invoke(null, null, null, SslPolicyErrors.None), "Should accept valid certificates"); + Assert.IsTrue(callback.Invoke(null, null, null, SslPolicyErrors.RemoteCertificateNotAvailable), "Should accept missing certificates"); + Assert.IsTrue(callback.Invoke(null, null, null, SslPolicyErrors.RemoteCertificateNameMismatch), "Should accept mismatched certificates"); + Assert.IsTrue(callback.Invoke(null, null, null, SslPolicyErrors.RemoteCertificateChainErrors), "Should accept invalid certificate chains"); + }); + + // Test with AcceptAllCerts = false + options.AcceptAllCerts = false; + await SMTP.InitializeSmtpClient(options, default, mockSmtpClient.Object); + callback = mockSmtpClient.Object.ServerCertificateValidationCallback; + + Assert.Multiple(() => + { + Assert.IsTrue(callback.Invoke(null, null, null, SslPolicyErrors.None), "Should accept valid certificates"); + Assert.IsFalse(callback.Invoke(null, null, null, SslPolicyErrors.RemoteCertificateNotAvailable), "Should reject missing certificates"); + Assert.IsFalse(callback.Invoke(null, null, null, SslPolicyErrors.RemoteCertificateNameMismatch), "Should reject mismatched certificates"); + Assert.IsFalse(callback.Invoke(null, null, null, SslPolicyErrors.RemoteCertificateChainErrors), "Should reject invalid certificate chains"); + }); } } diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Definitions/Options.cs b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Definitions/Options.cs index 393d138..9a089ee 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Definitions/Options.cs +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/Definitions/Options.cs @@ -31,7 +31,9 @@ public class Options public SecureSocketOption SecureSocket { get; set; } /// - /// Should the task accept all certificates from IMAP server, including invalid ones? + /// WARNING: Setting AcceptAllCerts to true disables SSL/TLS certificate validation. + /// This should only be used in development/test environments with self-signed certificates. + /// Using this option in production environments poses significant security risks. /// /// true [DefaultValue(false)] diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs index f8cc66f..fb6eaa6 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail/SendEmailTask.cs @@ -13,6 +13,7 @@ using System.Threading.Tasks; using Frends.SMTP.SendEmail.Definitions; using System.Runtime.CompilerServices; +using System.Net.Security; [assembly: InternalsVisibleTo("Frends.SMTP.Tests")] namespace Frends.SMTP.SendEmail; @@ -109,6 +110,10 @@ internal static async Task InitializeSmtpClient(Options options, Can client.ServerCertificateValidationCallback = (s, x509certificate, x590chain, sslPolicyErrors) => true; #pragma warning restore S4830 // Server certificates should be verified during SSL/TLS connections } + else + { + client.ServerCertificateValidationCallback = (s, x509certificate, x590chain, sslPolicyErrors) => sslPolicyErrors == SslPolicyErrors.None; + } var secureSocketOption = options.SecureSocket switch { From b913ee815afa48c2f0553ff6d91e35faffe2f5db Mon Sep 17 00:00:00 2001 From: Riku Virtanen Date: Wed, 20 Nov 2024 13:40:48 +0200 Subject: [PATCH 4/4] Removed unneccessary test --- .../Frends.Smtp.SendEmail.Tests/SendEmailTests.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs index fd4d808..8193f81 100644 --- a/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs +++ b/Frends.SMTP.SendEmail/Frends.Smtp.SendEmail.Tests/SendEmailTests.cs @@ -103,19 +103,6 @@ public async Task SendEmailWithPlainText() Assert.IsTrue(result.EmailSent); } - [Test] - public async Task EmailTestAcceptAllCertifications() - { - _options.SecureSocket = SecureSocketOption.StartTls; - _options.AcceptAllCerts = true; - - var input = _input; - input.Subject = "Email test - PlainText"; - - var result = await SMTP.SendEmail(input, null, _options, default); - Assert.IsTrue(result.EmailSent); - } - [Test] public async Task SendEmailWithFileAttachment() {