From 190328c0cfaa5fe436f8fa4e5f555940aafd7a20 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 4 Apr 2025 11:58:15 -0400 Subject: [PATCH 01/11] Introduce options for adding certificates to the X509ChainPolicy.CustomTrustStore Co-authored-by: tangowithfoxtrot --- .../PostConfigureX509ChainOptions.cs | 94 ++++++++++++ ...ustomizationServiceCollectionExtensions.cs | 53 +++++++ .../X509ChainOptions.cs | 72 +++++++++ .../MailKitSmtpMailDeliveryService.cs | 17 ++- .../MailKitSmtpMailDeliveryServiceTests.cs | 49 +++++-- ...izationServiceCollectionExtensionsTests.cs | 137 ++++++++++++++++++ .../Services/HandlebarsMailServiceTests.cs | 4 +- .../MailKitSmtpMailDeliveryServiceTests.cs | 7 +- 8 files changed, 412 insertions(+), 21 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..3361201093 --- /dev/null +++ b/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs @@ -0,0 +1,94 @@ +#nullable enable + +using System.Security.Cryptography.X509Certificates; +using Microsoft.Extensions.Options; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Hosting; +using Bit.Core.Settings; + +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) + { + 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..45de2c5460 --- /dev/null +++ b/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs @@ -0,0 +1,53 @@ +using Bit.Core.Platform.X509ChainCustomization; +using Microsoft.Extensions.Options; +using Microsoft.Extensions.DependencyInjection.Extensions; + +namespace Microsoft.Extensions.DependencyInjection; + +/// +/// Extension methods for setting up the ability to provide customization to how TLS works in an . +/// +public static class X509ChainCustomizationServiceCollectionExtensions +{ + /// + /// Configures X509ChainPolicy customization through the root level TlsOptions 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 `PostConfigureTlsOptions` 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 tlsOptions = sp.GetRequiredService>().Value; + + var handler = new HttpClientHandler(); + + if (tlsOptions.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..19cbd73ae3 --- /dev/null +++ b/src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs @@ -0,0 +1,72 @@ +#nullable enable + +using System.Diagnostics.CodeAnalysis; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; + +namespace Bit.Core.Platform.X509ChainCustomization; + +/// +/// Allows for customization of +/// +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..f9a6762772 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 _x509CertificateCustomization; private readonly string _replyDomain; private readonly string _replyEmail; public MailKitSmtpMailDeliveryService( GlobalSettings globalSettings, - ILogger logger) + ILogger logger, + IOptions tlsOptions) { if (globalSettings.Mail?.Smtp?.Host == null) { @@ -31,6 +36,7 @@ public class MailKitSmtpMailDeliveryService : IMailDeliveryService _globalSettings = globalSettings; _logger = logger; + _x509CertificateCustomization = tlsOptions.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 (_x509CertificateCustomization.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..7914a6d147 100644 --- a/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs +++ b/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs @@ -3,9 +3,11 @@ using System.Security.Cryptography.X509Certificates; using Bit.Core.Models.Mail; using Bit.Core.Services; using Bit.Core.Settings; +using Bit.Core.Platform.TlsCustomization; 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; @@ -100,7 +102,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509CertificateCustomizationOptions()) ); await Assert.ThrowsAsync( @@ -113,7 +116,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 +133,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 tlsOptions = new X509CertificateCustomizationOptions + { + AdditionalCustomTrustCertificates = + [ + _selfSignedCert, + ], + }; var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(tlsOptions) ); var tcs = new TaskCompletionSource(); @@ -162,7 +171,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 +188,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 tlsOptions = new X509CertificateCustomizationOptions + { + AdditionalCustomTrustCertificates = + [ + _selfSignedCert, + CreateSelfSignedCert("example.com"), + ], + }; var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(tlsOptions) ); var tcs = new TaskCompletionSource(); @@ -234,7 +247,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509CertificateCustomizationOptions()) ); var tcs = new TaskCompletionSource(); @@ -280,7 +294,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509CertificateCustomizationOptions()) ); var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); @@ -315,7 +330,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509CertificateCustomizationOptions()) ); var tcs = new TaskCompletionSource(); @@ -381,7 +397,8 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, - NullLogger.Instance + NullLogger.Instance, + Options.Create(new X509CertificateCustomizationOptions()) ); var tcs = new TaskCompletionSource(); diff --git a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs new file mode 100644 index 0000000000..2af9b7cc8a --- /dev/null +++ b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs @@ -0,0 +1,137 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Bit.Core.Platform.X509ChainCustomization; +using Bit.Core.Settings; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.Platform.TlsCustomization; + +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) => + { + gs.SelfHosted = true; + + environment.EnvironmentName = "Development"; + + 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; + + environment.EnvironmentName = "Development"; + + 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 options = CreateOptions((gs, environment, config) => + { + gs.SelfHosted = false; + + environment.EnvironmentName = "Development"; + + config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; + }, services => + { + services.Configure(options => + { + options.AdditionalCustomTrustCertificates = [CreateSelfSignedCert("example.com")]; + }); + }); + + Assert.True(options.TryGetCustomRemoteCertificateValidationCallback(out var callback)); + var cert = Assert.Single(options.AdditionalCustomTrustCertificates); + Assert.Equal("CN=example.com", cert.Subject); + } + + 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(); + var hostEnvironment = Substitute.For(); + var config = new Dictionary(); + + configure(globalSettings, hostEnvironment, config); + + var services = new ServiceCollection(); + 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()) ); } From 56e82b1c15bf9f569edfefb21fa9cb485c9bdf59 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 4 Apr 2025 12:04:10 -0400 Subject: [PATCH 02/11] Add comments --- .../X509ChainCustomization/PostConfigureX509ChainOptions.cs | 2 ++ src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs b/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs index 3361201093..583672f2f2 100644 --- a/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs +++ b/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs @@ -28,6 +28,8 @@ internal sealed class PostConfigureX509ChainOptions : IPostConfigureOptions -/// Allows for customization of +/// Allows for customization of the and access to a custom server certificate validator +/// if customization has been made. /// public sealed class X509ChainOptions { From 665ae0cf2464311061bedf30ad60680586ebefd9 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 4 Apr 2025 12:08:52 -0400 Subject: [PATCH 03/11] Fix places I am still calling it TLS options --- ...ustomizationServiceCollectionExtensions.cs | 10 +++++----- .../MailKitSmtpMailDeliveryService.cs | 8 ++++---- .../MailKitSmtpMailDeliveryServiceTests.cs | 20 +++++++++---------- ...izationServiceCollectionExtensionsTests.cs | 2 +- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs b/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs index 45de2c5460..780bf70f7f 100644 --- a/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs +++ b/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs @@ -5,12 +5,12 @@ using Microsoft.Extensions.DependencyInjection.Extensions; namespace Microsoft.Extensions.DependencyInjection; /// -/// Extension methods for setting up the ability to provide customization to how TLS works in an . +/// 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 TlsOptions configuration section + /// Configures X509ChainPolicy customization through the root level X509ChainOptions configuration section /// and configures the primary to use custom certificate validation /// when customized to do so. /// @@ -23,7 +23,7 @@ public static class X509ChainCustomizationServiceCollectionExtensions services.AddOptions() .BindConfiguration(nameof(X509ChainOptions)); - // Use TryAddEnumerable to make sure `PostConfigureTlsOptions` isn't added multiple + // Use TryAddEnumerable to make sure `PostConfigureX509ChainOptions` isn't added multiple // times even if this method is called multiple times. services.TryAddEnumerable(ServiceDescriptor.Singleton, PostConfigureX509ChainOptions>()); @@ -32,11 +32,11 @@ public static class X509ChainCustomizationServiceCollectionExtensions { builder.ConfigurePrimaryHttpMessageHandler(sp => { - var tlsOptions = sp.GetRequiredService>().Value; + var x509ChainOptions = sp.GetRequiredService>().Value; var handler = new HttpClientHandler(); - if (tlsOptions.TryGetCustomRemoteCertificateValidationCallback(out var callback)) + if (x509ChainOptions.TryGetCustomRemoteCertificateValidationCallback(out var callback)) { handler.ServerCertificateCustomValidationCallback = (sender, certificate, chain, errors) => { diff --git a/src/Core/Services/Implementations/MailKitSmtpMailDeliveryService.cs b/src/Core/Services/Implementations/MailKitSmtpMailDeliveryService.cs index f9a6762772..3a7cabd39e 100644 --- a/src/Core/Services/Implementations/MailKitSmtpMailDeliveryService.cs +++ b/src/Core/Services/Implementations/MailKitSmtpMailDeliveryService.cs @@ -13,14 +13,14 @@ public class MailKitSmtpMailDeliveryService : IMailDeliveryService { private readonly GlobalSettings _globalSettings; private readonly ILogger _logger; - private readonly X509ChainOptions _x509CertificateCustomization; + private readonly X509ChainOptions _x509ChainOptions; private readonly string _replyDomain; private readonly string _replyEmail; public MailKitSmtpMailDeliveryService( GlobalSettings globalSettings, ILogger logger, - IOptions tlsOptions) + IOptions x509ChainOptions) { if (globalSettings.Mail?.Smtp?.Host == null) { @@ -36,7 +36,7 @@ public class MailKitSmtpMailDeliveryService : IMailDeliveryService _globalSettings = globalSettings; _logger = logger; - _x509CertificateCustomization = tlsOptions.Value; + _x509ChainOptions = x509ChainOptions.Value; } public async Task SendEmailAsync(Models.Mail.MailMessage message) @@ -81,7 +81,7 @@ public class MailKitSmtpMailDeliveryService : IMailDeliveryService { client.ServerCertificateValidationCallback = (s, c, h, e) => true; } - else if (_x509CertificateCustomization.TryGetCustomRemoteCertificateValidationCallback(out var callback)) + else if (_x509ChainOptions.TryGetCustomRemoteCertificateValidationCallback(out var callback)) { client.ServerCertificateValidationCallback = (sender, cert, chain, errors) => { diff --git a/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs b/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs index 7914a6d147..8668263fa4 100644 --- a/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs +++ b/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs @@ -1,9 +1,9 @@ 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 Bit.Core.Platform.TlsCustomization; using MailKit.Security; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -103,7 +103,7 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, NullLogger.Instance, - Options.Create(new X509CertificateCustomizationOptions()) + Options.Create(new X509ChainOptions()) ); await Assert.ThrowsAsync( @@ -133,7 +133,7 @@ public class MailKitSmtpMailDeliveryServiceTests gs.Mail.Smtp.Ssl = true; }); - var tlsOptions = new X509CertificateCustomizationOptions + var x509ChainOptions = new X509ChainOptions { AdditionalCustomTrustCertificates = [ @@ -144,7 +144,7 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, NullLogger.Instance, - Options.Create(tlsOptions) + Options.Create(x509ChainOptions) ); var tcs = new TaskCompletionSource(); @@ -188,7 +188,7 @@ public class MailKitSmtpMailDeliveryServiceTests gs.Mail.Smtp.Ssl = true; }); - var tlsOptions = new X509CertificateCustomizationOptions + var x509ChainOptions = new X509ChainOptions { AdditionalCustomTrustCertificates = [ @@ -200,7 +200,7 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, NullLogger.Instance, - Options.Create(tlsOptions) + Options.Create(x509ChainOptions) ); var tcs = new TaskCompletionSource(); @@ -248,7 +248,7 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, NullLogger.Instance, - Options.Create(new X509CertificateCustomizationOptions()) + Options.Create(new X509ChainOptions()) ); var tcs = new TaskCompletionSource(); @@ -295,7 +295,7 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, NullLogger.Instance, - Options.Create(new X509CertificateCustomizationOptions()) + Options.Create(new X509ChainOptions()) ); var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); @@ -331,7 +331,7 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, NullLogger.Instance, - Options.Create(new X509CertificateCustomizationOptions()) + Options.Create(new X509ChainOptions()) ); var tcs = new TaskCompletionSource(); @@ -398,7 +398,7 @@ public class MailKitSmtpMailDeliveryServiceTests var mailKitDeliveryService = new MailKitSmtpMailDeliveryService( globalSettings, NullLogger.Instance, - Options.Create(new X509CertificateCustomizationOptions()) + Options.Create(new X509ChainOptions()) ); var tcs = new TaskCompletionSource(); diff --git a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs index 2af9b7cc8a..30eb19d8c0 100644 --- a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs +++ b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs @@ -9,7 +9,7 @@ using Microsoft.Extensions.Options; using NSubstitute; using Xunit; -namespace Bit.Core.Test.Platform.TlsCustomization; +namespace Bit.Core.Test.Platform.X509ChainCustomization; public class X509ChainCustomizationServiceCollectionExtensionsTests { From 32567f0fecf341835e9701c36b65f66b505fab8a Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 4 Apr 2025 12:22:43 -0400 Subject: [PATCH 04/11] Format --- ...ainCustomizationServiceCollectionExtensionsTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs index 30eb19d8c0..d4f927f4a1 100644 --- a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs +++ b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs @@ -1,4 +1,4 @@ -using System.Security.Cryptography; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Bit.Core.Platform.X509ChainCustomization; using Bit.Core.Settings; @@ -24,7 +24,7 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests 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)); @@ -36,7 +36,7 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; }); - + // Create options once var firstOptions = services.GetRequiredService>().Value; @@ -61,7 +61,7 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests 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)); @@ -81,7 +81,7 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests 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)); From c34c31aaa195935a92e5538615f5eacb3fd0de0c Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 4 Apr 2025 12:31:07 -0400 Subject: [PATCH 05/11] Format from root --- .../PostConfigureX509ChainOptions.cs | 8 ++++---- .../X509ChainCustomizationServiceCollectionExtensions.cs | 4 ++-- .../Platform/X509ChainCustomization/X509ChainOptions.cs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs b/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs index 583672f2f2..963294e85f 100644 --- a/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs +++ b/src/Core/Platform/X509ChainCustomization/PostConfigureX509ChainOptions.cs @@ -1,10 +1,10 @@ -#nullable enable +#nullable enable using System.Security.Cryptography.X509Certificates; -using Microsoft.Extensions.Options; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Hosting; using Bit.Core.Settings; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Bit.Core.Platform.X509ChainCustomization; diff --git a/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs b/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs index 780bf70f7f..46bd5b37e6 100644 --- a/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs +++ b/src/Core/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensions.cs @@ -1,6 +1,6 @@ -using Bit.Core.Platform.X509ChainCustomization; -using Microsoft.Extensions.Options; +using Bit.Core.Platform.X509ChainCustomization; using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Options; namespace Microsoft.Extensions.DependencyInjection; diff --git a/src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs b/src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs index bcfed0d437..189a1087f5 100644 --- a/src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs +++ b/src/Core/Platform/X509ChainCustomization/X509ChainOptions.cs @@ -1,4 +1,4 @@ -#nullable enable +#nullable enable using System.Diagnostics.CodeAnalysis; using System.Net.Security; @@ -45,7 +45,7 @@ public sealed class X509ChainOptions /// [MemberNotNullWhen(true, nameof(AdditionalCustomTrustCertificates))] public bool TryGetCustomRemoteCertificateValidationCallback( - [MaybeNullWhen(false)]out Func callback) + [MaybeNullWhen(false)] out Func callback) { callback = null; if (AdditionalCustomTrustCertificates == null || AdditionalCustomTrustCertificates.Count == 0) From f5e10d606281b328b5a14f3fc80a03ffaddbede7 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 4 Apr 2025 13:56:51 -0400 Subject: [PATCH 06/11] Add more tests --- test/Core.Test/Core.Test.csproj | 1 + ...izationServiceCollectionExtensionsTests.cs | 111 ++++++++++++++++-- 2 files changed, 100 insertions(+), 12 deletions(-) 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 index d4f927f4a1..703a37cc30 100644 --- a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs +++ b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs @@ -5,6 +5,7 @@ using Bit.Core.Settings; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using NSubstitute; using Xunit; @@ -30,10 +31,6 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests var services = CreateServices((gs, environment, config) => { - gs.SelfHosted = true; - - environment.EnvironmentName = "Development"; - config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; }); @@ -69,8 +66,6 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests { gs.SelfHosted = false; - environment.EnvironmentName = "Development"; - config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; }); @@ -85,12 +80,8 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests var tempCert = Path.Combine(tempDir.FullName, "test.crt"); await File.WriteAllBytesAsync(tempCert, CreateSelfSignedCert("localhost").Export(X509ContentType.Cert)); - var options = CreateOptions((gs, environment, config) => + var services = CreateServices((gs, environment, config) => { - gs.SelfHosted = false; - - environment.EnvironmentName = "Development"; - config["X509ChainOptions:AdditionalCustomTrustCertificatesDirectory"] = tempDir.FullName; }, services => { @@ -100,9 +91,95 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests }); }); + 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}'" + ); } private static X509ChainOptions CreateOptions(Action> configure, Action? after = null) @@ -113,13 +190,23 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests private static IServiceProvider CreateServices(Action> configure, Action? after = null) { - var globalSettings = new GlobalSettings(); + 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( From 62bdd91cf3095b1cb170cdd1007e1aa3d3145127 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 4 Apr 2025 15:02:21 -0400 Subject: [PATCH 07/11] Add HTTP Tests --- ...izationServiceCollectionExtensionsTests.cs | 104 +++++++++++++++++- 1 file changed, 103 insertions(+), 1 deletion(-) diff --git a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs index 703a37cc30..06ce513b51 100644 --- a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs +++ b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs @@ -1,7 +1,11 @@ -using System.Security.Cryptography; +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; @@ -182,6 +186,104 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests ); } + [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) + { + // Start HTTP Server with self signed cert + var builder = WebApplication.CreateSlimBuilder(); + builder.Logging.AddFakeLogging(); + 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); From 0400ae187af10c93a5d4ce3336b59a4e9b46b0d3 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Fri, 4 Apr 2025 15:03:59 -0400 Subject: [PATCH 08/11] Format --- ...509ChainCustomizationServiceCollectionExtensionsTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs index 06ce513b51..a9762e3a65 100644 --- a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs +++ b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs @@ -195,7 +195,7 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests options.ServerCertificate = selfSignedCertificate; }); - var services = CreateServices((gs, environment, config) => {}, services => + var services = CreateServices((gs, environment, config) => { }, services => { services.Configure(options => { @@ -218,7 +218,7 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests options.ServerCertificate = selfSignedCertificate; }); - var services = CreateServices((gs, environment, config) => {}, services => + var services = CreateServices((gs, environment, config) => { }, services => { services.Configure(options => { @@ -243,7 +243,7 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests options.ServerCertificate = selfSignedCertificate; }); - var services = CreateServices((gs, environment, config) => {}, services => + var services = CreateServices((gs, environment, config) => { }, services => { services.Configure(options => { From ee4678453bc96d137c2f7be372f44f0d283ce67a Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Sun, 6 Apr 2025 10:06:42 -0400 Subject: [PATCH 09/11] Switch to empty builder --- .../X509ChainCustomizationServiceCollectionExtensionsTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs index a9762e3a65..2a4ed55489 100644 --- a/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs +++ b/test/Core.Test/Platform/X509ChainCustomization/X509ChainCustomizationServiceCollectionExtensionsTests.cs @@ -259,9 +259,7 @@ public class X509ChainCustomizationServiceCollectionExtensionsTests private static async Task CreateServerAsync(int port, Action configure) { - // Start HTTP Server with self signed cert - var builder = WebApplication.CreateSlimBuilder(); - builder.Logging.AddFakeLogging(); + var builder = WebApplication.CreateEmptyBuilder(new WebApplicationOptions()); builder.Services.AddRoutingCore(); builder.WebHost.UseKestrelCore() .ConfigureKestrel(options => From 2c27c8447d885f9893f2bc713af90ba00489f724 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Sun, 6 Apr 2025 10:28:07 -0400 Subject: [PATCH 10/11] Remove unneeded helper --- .../MailKitSmtpMailDeliveryServiceTests.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs b/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs index 8668263fa4..24c477cf05 100644 --- a/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs +++ b/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs @@ -1,4 +1,4 @@ -using System.Security.Cryptography; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Bit.Core.Models.Mail; using Bit.Core.Platform.X509ChainCustomization; @@ -32,11 +32,6 @@ 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) { // Unfortunately this package doesn't public expose its logging infrastructure From a94e7047e79e29d29e279adb45d75d1c3b657eef Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Sun, 6 Apr 2025 10:29:57 -0400 Subject: [PATCH 11/11] Configure logging only once --- .../MailKitSmtpMailDeliveryServiceTests.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs b/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs index 24c477cf05..38c18f26f9 100644 --- a/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs +++ b/test/Core.IntegrationTest/MailKitSmtpMailDeliveryServiceTests.cs @@ -1,4 +1,4 @@ -using System.Security.Cryptography; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Bit.Core.Models.Mail; using Bit.Core.Platform.X509ChainCustomization; @@ -16,6 +16,7 @@ namespace Bit.Core.IntegrationTest; public class MailKitSmtpMailDeliveryServiceTests { + private static int _loggingConfigured; private readonly X509Certificate2 _selfSignedCert; public MailKitSmtpMailDeliveryServiceTests(ITestOutputHelper testOutputHelper) @@ -34,6 +35,12 @@ public class MailKitSmtpMailDeliveryServiceTests 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