From a8403f3dc2a13e5b5469dd7ac857f6ac4e21d4dd Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Mon, 7 Apr 2025 14:10:36 -0400 Subject: [PATCH] [PM-19601] Introduce options for adding certificates to trust without root (#5609) * Introduce options for adding certificates to the X509ChainPolicy.CustomTrustStore Co-authored-by: tangowithfoxtrot * Add comments * Fix places I am still calling it TLS options * Format * Format from root * Add more tests * Add HTTP Tests * Format * Switch to empty builder * Remove unneeded helper * Configure logging only once --------- Co-authored-by: tangowithfoxtrot --- .../PostConfigureX509ChainOptions.cs | 96 ++++++ ...ustomizationServiceCollectionExtensions.cs | 53 +++ .../X509ChainOptions.cs | 73 ++++ .../MailKitSmtpMailDeliveryService.cs | 17 +- .../MailKitSmtpMailDeliveryServiceTests.cs | 61 ++-- test/Core.Test/Core.Test.csproj | 1 + ...izationServiceCollectionExtensionsTests.cs | 324 ++++++++++++++++++ .../Services/HandlebarsMailServiceTests.cs | 4 +- .../MailKitSmtpMailDeliveryServiceTests.cs | 7 +- 9 files changed, 610 insertions(+), 26 deletions(-) create mode 100644 src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs create mode 100644 src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs create mode 100644 src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs create mode 100644 test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs diff --git a/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs b/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs new file mode 100644 index 0000000000..963294e85f --- /dev/null +++ b/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs @@ -0,0 +1,96 @@ +#nullable enable + +using System.Security.Cryptography.X509Certificates; +using Bit.Core.Settings; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace Bit.Core.Platform.X509ChainCustomization; + +internal sealed class PostConfigureX509ChainOptions : IPostConfigureOptions +{ + const string CertificateSearchPattern = "*.crt"; + + private readonly ILogger _logger; + private readonly IHostEnvironment _hostEnvironment; + private readonly GlobalSettings _globalSettings; + + public PostConfigureX509ChainOptions( + ILogger logger, + IHostEnvironment hostEnvironment, + GlobalSettings globalSettings) + { + _logger = logger; + _hostEnvironment = hostEnvironment; + _globalSettings = globalSettings; + } + + public void PostConfigure(string? name, X509ChainOptions options) + { + // We don't register or request a named instance of these options, + // so don't customize it. + if (name != Options.DefaultName) + { + return; + } + + // We only allow this setting to be configured on self host. + if (!_globalSettings.SelfHosted) + { + options.AdditionalCustomTrustCertificatesDirectory = null; + return; + } + + if (options.AdditionalCustomTrustCertificates != null) + { + // Additional certificates were added directly, this overwrites the need to + // read them from the directory. + _logger.LogInformation( + "Additional custom trust certificates were added directly, skipping loading them from '{Directory}'", + options.AdditionalCustomTrustCertificatesDirectory + ); + return; + } + + if (string.IsNullOrEmpty(options.AdditionalCustomTrustCertificatesDirectory)) + { + return; + } + + if (!Directory.Exists(options.AdditionalCustomTrustCertificatesDirectory)) + { + // The default directory is volume mounted via the default Bitwarden setup process. + // If the directory doesn't exist it could indicate a error in configuration but this + // directory is never expected in a normal development environment so lower the log + // level in that case. + var logLevel = _hostEnvironment.IsDevelopment() + ? LogLevel.Debug + : LogLevel.Warning; + _logger.Log( + logLevel, + "An additional custom trust certificate directory was given '{Directory}' but that directory does not exist.", + options.AdditionalCustomTrustCertificatesDirectory + ); + return; + } + + var certificates = new List(); + + foreach (var certFile in Directory.EnumerateFiles(options.AdditionalCustomTrustCertificatesDirectory, CertificateSearchPattern)) + { + certificates.Add(new X509Certificate2(certFile)); + } + + if (options.AdditionalCustomTrustCertificatesDirectory != X509ChainOptions.DefaultAdditionalCustomTrustCertificatesDirectory && certificates.Count == 0) + { + // They have intentionally given us a non-default directory but there weren't certificates, that is odd. + _logger.LogWarning( + "No additional custom trust certificates were found in '{Directory}'", + options.AdditionalCustomTrustCertificatesDirectory + ); + } + + options.AdditionalCustomTrustCertificates = certificates; + } +} diff --git a/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs b/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs new file mode 100644 index 0000000000..46bd5b37e6 --- /dev/null +++ b/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs @@ -0,0 +1,53 @@ +using Bit.Core.Platform.X509ChainCustomization; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Options; + +namespace Microsoft.Extensions.DependencyInjection; + +/// +/// Extension methods for setting up the ability to provide customization to how X509 chain validation works in an . +/// +public static class X509ChainCustomizationServiceCollectionExtensions +{ + /// + /// Configures X509ChainPolicy customization through the root level X509ChainOptions configuration section + /// and configures the primary to use custom certificate validation + /// when customized to do so. + /// + /// The . + /// The for additional chaining. + public static IServiceCollection AddX509ChainCustomization(this IServiceCollection services) + { + ArgumentNullException.ThrowIfNull(services); + + services.AddOptions() + .BindConfiguration(nameof(X509ChainOptions)); + + // Use TryAddEnumerable to make sure `PostConfigureX509ChainOptions` isn't added multiple + // times even if this method is called multiple times. + services.TryAddEnumerable(ServiceDescriptor.Singleton, PostConfigureX509ChainOptions>()); + + services.AddHttpClient() + .ConfigureHttpClientDefaults(builder => + { + builder.ConfigurePrimaryHttpMessageHandler(sp => + { + var x509ChainOptions = sp.GetRequiredService>().Value; + + var handler = new HttpClientHandler(); + + if (x509ChainOptions.TryGetCustomRemoteCertificateValidationCallback(out var callback)) + { + handler.ServerCertificateCustomValidationCallback = (sender, certificate, chain, errors) => + { + return callback(certificate, chain, errors); + }; + } + + return handler; + }); + }); + + return services; + } +} diff --git a/src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs b/src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs new file mode 100644 index 0000000000..189a1087f5 --- /dev/null +++ b/src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs @@ -0,0 +1,73 @@ +#nullable enable + +using System.Diagnostics.CodeAnalysis; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; + +namespace Bit.Core.Platform.X509ChainCustomization; + +/// +/// Allows for customization of the and access to a custom server certificate validator +/// if customization has been made. +/// +public sealed class X509ChainOptions +{ + // This is the directory that we historically used to allow certificates be added inside our container + // and then on start of the container we would move them to `/usr/local/share/ca-certificates/` and call + // `update-ca-certificates` but since that operation requires root we can't do it in a rootless container. + // Ref: https://github.com/bitwarden/server/blob/67d7d685a619a5fc413f8532dacb09681ee5c956/src/Api/entrypoint.sh#L38-L41 + public const string DefaultAdditionalCustomTrustCertificatesDirectory = "/etc/bitwarden/ca-certificates/"; + + /// + /// A directory where additional certificates should be read from and included in . + /// + /// + /// Only certificates suffixed with *.crt will be read. If is + /// set, then this directory will not be read from. + /// + public string? AdditionalCustomTrustCertificatesDirectory { get; set; } = DefaultAdditionalCustomTrustCertificatesDirectory; + + /// + /// A list of additional certificates that should be included in . + /// + /// + /// If this value is set manually, then will be ignored. + /// + public List? AdditionalCustomTrustCertificates { get; set; } + + /// + /// Attempts to retrieve a custom remote certificate validation callback. + /// + /// + /// Returns when we have custom remote certification that should be added, + /// when no custom validation is needed and the default validation callback should + /// be used instead. + /// + [MemberNotNullWhen(true, nameof(AdditionalCustomTrustCertificates))] + public bool TryGetCustomRemoteCertificateValidationCallback( + [MaybeNullWhen(false)] out Func callback) + { + callback = null; + if (AdditionalCustomTrustCertificates == null || AdditionalCustomTrustCertificates.Count == 0) + { + return false; + } + + // Ref: https://github.com/dotnet/runtime/issues/39835#issuecomment-663020581 + callback = (certificate, chain, errors) => + { + if (chain == null || certificate == null) + { + return false; + } + + chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; + foreach (var additionalCertificate in AdditionalCustomTrustCertificates) + { + chain.ChainPolicy.CustomTrustStore.Add(additionalCertificate); + } + return chain.Build(certificate); + }; + return true; + } +} diff --git a/src/Core/Services/Implementations/MailKitSmtpMailDeliveryService.cs b/src/Core/Services/Implementations/MailKitSmtpMailDeliveryService.cs index dc4e89aa23..3a7cabd39e 100644 --- a/src/Core/Services/Implementations/MailKitSmtpMailDeliveryService.cs +++ b/src/Core/Services/Implementations/MailKitSmtpMailDeliveryService.cs @@ -1,7 +1,10 @@ -using Bit.Core.Settings; +using System.Security.Cryptography.X509Certificates; +using Bit.Core.Platform.X509ChainCustomization; +using Bit.Core.Settings; using Bit.Core.Utilities; using MailKit.Net.Smtp; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using MimeKit; namespace Bit.Core.Services; @@ -10,12 +13,14 @@ public class MailKitSmtpMailDeliveryService : IMailDeliveryService { private readonly GlobalSettings _globalSettings; private readonly ILogger _logger; + private readonly X509ChainOptions _x509ChainOptions; private readonly string _replyDomain; private readonly string _replyEmail; public MailKitSmtpMailDeliveryService( GlobalSettings globalSettings, - ILogger logger) + ILogger logger, + IOptions x509ChainOptions) { if (globalSettings.Mail?.Smtp?.Host == null) { @@ -31,6 +36,7 @@ public class MailKitSmtpMailDeliveryService : IMailDeliveryService _globalSettings = globalSettings; _logger = logger; + _x509ChainOptions = x509ChainOptions.Value; } public async Task SendEmailAsync(Models.Mail.MailMessage message) @@ -75,6 +81,13 @@ public class MailKitSmtpMailDeliveryService : IMailDeliveryService { client.ServerCertificateValidationCallback = (s, c, h, e) => true; } + else if (_x509ChainOptions.TryGetCustomRemoteCertificateValidationCallback(out var callback)) + { + client.ServerCertificateValidationCallback = (sender, cert, chain, errors) => + { + return callback(new X509Certificate2(cert), chain, errors); + }; + } if (!_globalSettings.Mail.Smtp.StartTls && !_globalSettings.Mail.Smtp.Ssl && _globalSettings.Mail.Smtp.Port == 25) diff --git a/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs b/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs index db2b945fda..38c18f26f9 100644 --- a/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs +++ b/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs @@ -1,11 +1,13 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Bit.Core.Models.Mail; +using Bit.Core.Platform.X509ChainCustomization; using Bit.Core.Services; using Bit.Core.Settings; using MailKit.Security; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Rnwood.SmtpServer; using Rnwood.SmtpServer.Extensions.Auth; using Xunit.Abstractions; @@ -14,6 +16,7 @@ namespace Bit.Core.IntegrationTest; public class MailKitSmtpMailDeliveryServiceTests { + private static int _loggingConfigured; private readonly X509Certificate2 _selfSignedCert; public MailKitSmtpMailDeliveryServiceTests(ITestOutputHelper testOutputHelper) @@ -30,13 +33,14 @@ public class MailKitSmtpMailDeliveryServiceTests return certRequest.CreateSelfSigned(DateTimeOffset.UtcNow, DateTimeOffset.UtcNow.AddYears(1)); } - private static async Task SaveCertAsync(string filePath, X509Certificate2 certificate) - { - await File.WriteAllBytesAsync(filePath, certificate.Export(X509ContentType.Cert)); - } - private static void ConfigureSmtpServerLogging(ITestOutputHelper testOutputHelper) { + // The logging in SmtpServer is configured statically so if we add it for each test it duplicates + // but we cant add the logger statically either because we need ITestOutputHelper + if (Interlocked.CompareExchange(ref _loggingConfigured, 1, 0) == 0) + { + return; + } // Unfortunately this package doesn't public expose its logging infrastructure // so we use private reflection to try and access it. try @@ -100,7 +104,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509ChainOptions()) ); await Assert.ThrowsAsync( @@ -113,7 +118,7 @@ public class MailKitSmtpMailDeliveryServiceTests ); } - [Fact(Skip = "Upcoming feature")] + [Fact] public async Task SendEmailAsync_SmtpServerUsingSelfSignedCert_CertInCustomLocation_Works() { // If an SMTP server is using a self signed cert we will in the future @@ -130,12 +135,18 @@ public class MailKitSmtpMailDeliveryServiceTests gs.Mail.Smtp.Ssl = true; }); - // TODO: Setup custom location and save self signed cert there. - // await SaveCertAsync("./my-location", _selfSignedCert); + var x509ChainOptions = new X509ChainOptions + { + AdditionalCustomTrustCertificates = + [ + _selfSignedCert, + ], + }; var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(x509ChainOptions) ); var tcs = new TaskCompletionSource(); @@ -162,7 +173,7 @@ public class MailKitSmtpMailDeliveryServiceTests await tcs.Task; } - [Fact(Skip = "Upcoming feature")] + [Fact] public async Task SendEmailAsync_SmtpServerUsingSelfSignedCert_CertInCustomLocation_WithUnrelatedCerts_Works() { // If an SMTP server is using a self signed cert we will in the future @@ -179,15 +190,19 @@ public class MailKitSmtpMailDeliveryServiceTests gs.Mail.Smtp.Ssl = true; }); - // TODO: Setup custom location and save self signed cert there - // along with another self signed cert that is not related to - // the SMTP server. - // await SaveCertAsync("./my-location", _selfSignedCert); - // await SaveCertAsync("./my-location", CreateSelfSignedCert("example.com")); + var x509ChainOptions = new X509ChainOptions + { + AdditionalCustomTrustCertificates = + [ + _selfSignedCert, + CreateSelfSignedCert("example.com"), + ], + }; var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(x509ChainOptions) ); var tcs = new TaskCompletionSource(); @@ -234,7 +249,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509ChainOptions()) ); var tcs = new TaskCompletionSource(); @@ -280,7 +296,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509ChainOptions()) ); var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); @@ -315,7 +332,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509ChainOptions()) ); var tcs = new TaskCompletionSource(); @@ -381,7 +399,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509ChainOptions()) ); var tcs = new TaskCompletionSource(); diff --git a/test/Core.Test/Core.Test.csproj b/test/Core.Test/Core.Test.csproj index baace97710..cc19c50c35 100644 --- a/test/Core.Test/Core.Test.csproj +++ b/test/Core.Test/Core.Test.csproj @@ -10,6 +10,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive all + diff --git a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs new file mode 100644 index 0000000000..2a4ed55489 --- /dev/null +++ b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs @@ -0,0 +1,324 @@ +using System.Security.Authentication; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Bit.Core.Platform.X509ChainCustomization; +using Bit.Core.Settings; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Server.Kestrel.Https; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.Platform.X509ChainCustomization; + +public class X509ChainCustomizationServiceCollectionExtensionsTests +{ + private static X509Certificate2 CreateSelfSignedCert(string commonName) + { + using var rsa = RSA.Create(2048); + var certRequest = new CertificateRequest($"CN={commonName}", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + return certRequest.CreateSelfSigned(DateTimeOffset.UtcNow, DateTimeOffset.UtcNow.AddYears(1)); + } + + [Fact] + public async Task OptionsPatternReturnsCachedValue() + { + var tempDir = Directory.CreateTempSubdirectory("certs"); + + var tempCert = Path.Combine(tempDir.FullName, "test.crt"); + await File.WriteAllBytesAsync(tempCert, CreateSelfSignedCert("localhost").Export(X509ContentType.Cert)); + + var services = CreateServices((gs, environment, config) => + { + config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; + }); + + // Create options once + var firstOptions = services.GetRequiredService>().Value; + + Assert.NotNull(firstOptions.AdditionalCustomTrustCertificates); + var cert = Assert.Single(firstOptions.AdditionalCustomTrustCertificates); + Assert.Equal("CN=localhost", cert.Subject); + + // Since the second resolution should have cached values, deleting the file during operation + // should have no impact. + File.Delete(tempCert); + + // This is expected to be a cached version and doesn't actually need to go and read the file system + var secondOptions = services.GetRequiredService>().Value; + Assert.Same(firstOptions, secondOptions); + + // This is the same reference as the first one so it shouldn't be different but just in case. + Assert.NotNull(secondOptions.AdditionalCustomTrustCertificates); + Assert.Single(secondOptions.AdditionalCustomTrustCertificates); + } + + [Fact] + public async Task DoesNotProvideCustomCallbackOnCloud() + { + var tempDir = Directory.CreateTempSubdirectory("certs"); + + var tempCert = Path.Combine(tempDir.FullName, "test.crt"); + await File.WriteAllBytesAsync(tempCert, CreateSelfSignedCert("localhost").Export(X509ContentType.Cert)); + + var options = CreateOptions((gs, environment, config) => + { + gs.SelfHosted = false; + + config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; + }); + + Assert.False(options.TryGetCustomRemoteCertificateValidationCallback(out _)); + } + + [Fact] + public async Task ManuallyAddingOptionsTakesPrecedence() + { + var tempDir = Directory.CreateTempSubdirectory("certs"); + + var tempCert = Path.Combine(tempDir.FullName, "test.crt"); + await File.WriteAllBytesAsync(tempCert, CreateSelfSignedCert("localhost").Export(X509ContentType.Cert)); + + var services = CreateServices((gs, environment, config) => + { + config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; + }, services => + { + services.Configure(options => + { + options.AdditionalCustomTrustCertificates = [CreateSelfSignedCert("example.com")]; + }); + }); + + var options = services.GetRequiredService>().Value; + + Assert.True(options.TryGetCustomRemoteCertificateValidationCallback(out var callback)); + var cert = Assert.Single(options.AdditionalCustomTrustCertificates); + Assert.Equal("CN=example.com", cert.Subject); + + var fakeLogCollector = services.GetFakeLogCollector(); + + Assert.Contains(fakeLogCollector.GetSnapshot(), + r => r.Message == $"Additional custom trust certificates were added directly, skipping loading them from '{tempDir}'"); + } + + [Fact] + public void NullCustomDirectory_SkipsTryingToLoad() + { + var services = CreateServices((gs, environment, config) => + { + config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = null; + }); + + var options = services.GetRequiredService>().Value; + + Assert.False(options.TryGetCustomRemoteCertificateValidationCallback(out _)); + } + + [Theory] + [InlineData("Development", LogLevel.Debug)] + [InlineData("Production", LogLevel.Warning)] + public void CustomDirectoryDoesNotExist_Logs(string environment, LogLevel logLevel) + { + var fakeDir = "/fake/dir/that/does/not/exist"; + var services = CreateServices((gs, hostEnvironment, config) => + { + hostEnvironment.EnvironmentName = environment; + + config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = fakeDir; + }); + + var options = services.GetRequiredService>().Value; + + Assert.False(options.TryGetCustomRemoteCertificateValidationCallback(out _)); + + var fakeLogCollector = services.GetFakeLogCollector(); + + Assert.Contains(fakeLogCollector.GetSnapshot(), + r => r.Message == $"An additional custom trust certificate directory was given '{fakeDir}' but that directory does not exist." + && r.Level == logLevel + ); + } + + [Fact] + public async Task NamedOptions_NotConfiguredAsync() + { + // To help make sure this fails for the right reason we should add certs to the directory + var tempDir = Directory.CreateTempSubdirectory("certs"); + + var tempCert = Path.Combine(tempDir.FullName, "test.crt"); + await File.WriteAllBytesAsync(tempCert, CreateSelfSignedCert("localhost").Export(X509ContentType.Cert)); + + var services = CreateServices((gs, environment, config) => + { + config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; + }); + + var options = services.GetRequiredService>(); + + var namedOptions = options.Get("SomeName"); + + Assert.Null(namedOptions.AdditionalCustomTrustCertificates); + } + + [Fact] + public void CustomLocation_NoCertificates_Logs() + { + var tempDir = Directory.CreateTempSubdirectory("certs"); + var services = CreateServices((gs, hostEnvironment, config) => + { + config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; + }); + + var options = services.GetRequiredService>().Value; + + Assert.False(options.TryGetCustomRemoteCertificateValidationCallback(out _)); + + var fakeLogCollector = services.GetFakeLogCollector(); + + Assert.Contains(fakeLogCollector.GetSnapshot(), + r => r.Message == $"No additional custom trust certificates were found in '{tempDir.FullName}'" + ); + } + + [Fact] + public async Task CallHttpWithSelfSignedCert_SelfSignedCertificateConfigured_Works() + { + var selfSignedCertificate = CreateSelfSignedCert("localhost"); + await using var app = await CreateServerAsync(55555, options => + { + options.ServerCertificate = selfSignedCertificate; + }); + + var services = CreateServices((gs, environment, config) => { }, services => + { + services.Configure(options => + { + options.AdditionalCustomTrustCertificates = [selfSignedCertificate]; + }); + }); + + var httpClient = services.GetRequiredService().CreateClient(); + + var response = await httpClient.GetStringAsync("https://localhost:55555"); + Assert.Equal("Hi", response); + } + + [Fact] + public async Task CallHttpWithSelfSignedCert_SelfSignedCertificateNotConfigured_Throws() + { + var selfSignedCertificate = CreateSelfSignedCert("localhost"); + await using var app = await CreateServerAsync(55556, options => + { + options.ServerCertificate = selfSignedCertificate; + }); + + var services = CreateServices((gs, environment, config) => { }, services => + { + services.Configure(options => + { + options.AdditionalCustomTrustCertificates = [CreateSelfSignedCert("example.com")]; + }); + }); + + var httpClient = services.GetRequiredService().CreateClient(); + + var requestException = await Assert.ThrowsAsync(async () => await httpClient.GetStringAsync("https://localhost:55556")); + Assert.NotNull(requestException.InnerException); + var authenticationException = Assert.IsAssignableFrom(requestException.InnerException); + Assert.Equal("The remote certificate was rejected by the provided RemoteCertificateValidationCallback.", authenticationException.Message); + } + + [Fact] + public async Task CallHttpWithSelfSignedCert_SelfSignedCertificateConfigured_WithExtraCert_Works() + { + var selfSignedCertificate = CreateSelfSignedCert("localhost"); + await using var app = await CreateServerAsync(55557, options => + { + options.ServerCertificate = selfSignedCertificate; + }); + + var services = CreateServices((gs, environment, config) => { }, services => + { + services.Configure(options => + { + options.AdditionalCustomTrustCertificates = [selfSignedCertificate, CreateSelfSignedCert("example.com")]; + }); + }); + + var httpClient = services.GetRequiredService().CreateClient(); + + var response = await httpClient.GetStringAsync("https://localhost:55557"); + Assert.Equal("Hi", response); + } + + private static async Task CreateServerAsync(int port, Action configure) + { + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); + builder.Services.AddRoutingCore(); + builder.WebHost.UseKestrelCore() + .ConfigureKestrel(options => + { + options.ListenLocalhost(port, listenOptions => + { + listenOptions.UseHttps(httpsOptions => + { + configure(httpsOptions); + }); + }); + }); + + var app = builder.Build(); + + app.MapGet("/", () => "Hi"); + + await app.StartAsync(); + + return app; + } + + private static X509ChainOptions CreateOptions(Action> configure, Action? after = null) + { + var services = CreateServices(configure, after); + return services.GetRequiredService>().Value; + } + + private static IServiceProvider CreateServices(Action> configure, Action? after = null) + { + var globalSettings = new GlobalSettings + { + // A solid default for these tests as these settings aren't allowed to work in cloud. + SelfHosted = true, + }; + var hostEnvironment = Substitute.For(); + hostEnvironment.EnvironmentName = "Development"; + var config = new Dictionary(); + + configure(globalSettings, hostEnvironment, config); + + var services = new ServiceCollection(); + services.AddLogging(logging => + { + logging.SetMinimumLevel(LogLevel.Debug); + logging.AddFakeLogging(); + }); + services.AddSingleton(globalSettings); + services.AddSingleton(hostEnvironment); + services.AddSingleton( + new ConfigurationBuilder() + .AddInMemoryCollection(config) + .Build() + ); + + services.AddX509ChainCustomization(); + + after?.Invoke(services); + + return services.BuildServiceProvider(); + } +} diff --git a/test/Core.Test/Services/HandlebarsMailServiceTests.cs b/test/Core.Test/Services/HandlebarsMailServiceTests.cs index 89d9a211e0..35c2f8fe3b 100644 --- a/test/Core.Test/Services/HandlebarsMailServiceTests.cs +++ b/test/Core.Test/Services/HandlebarsMailServiceTests.cs @@ -4,9 +4,11 @@ using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Business; using Bit.Core.Entities; +using Bit.Core.Platform.X509ChainCustomization; using Bit.Core.Services; using Bit.Core.Settings; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using NSubstitute; using Xunit; @@ -136,7 +138,7 @@ public class HandlebarsMailServiceTests SiteName = "Bitwarden", }; - var mailDeliveryService = new MailKitSmtpMailDeliveryService(globalSettings, Substitute.For>()); + var mailDeliveryService = new MailKitSmtpMailDeliveryService(globalSettings, Substitute.For>(), Options.Create(new X509ChainOptions())); var handlebarsService = new HandlebarsMailService(globalSettings, mailDeliveryService, new BlockingMailEnqueuingService()); diff --git a/test/Core.Test/Services/MailKitSmtpMailDeliveryServiceTests.cs b/test/Core.Test/Services/MailKitSmtpMailDeliveryServiceTests.cs index 4e7e36fe02..06ee99dbef 100644 --- a/test/Core.Test/Services/MailKitSmtpMailDeliveryServiceTests.cs +++ b/test/Core.Test/Services/MailKitSmtpMailDeliveryServiceTests.cs @@ -1,6 +1,8 @@ -using Bit.Core.Services; +using Bit.Core.Platform.X509ChainCustomization; +using Bit.Core.Services; using Bit.Core.Settings; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using NSubstitute; using Xunit; @@ -23,7 +25,8 @@ public class MailKitSmtpMailDeliveryServiceTests _sut = new MailKitSmtpMailDeliveryService( _globalSettings, - _logger + _logger, + Options.Create(new X509ChainOptions()) ); }