diff --git a/src/SharedWeb/Utilities/DataProtectionServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/DataProtectionServiceCollectionExtensions.cs new file mode 100644 index 000000000000..d81cbc1bcd75 --- /dev/null +++ b/src/SharedWeb/Utilities/DataProtectionServiceCollectionExtensions.cs @@ -0,0 +1,102 @@ +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Azure; +using Azure.Storage.Blobs; +using Bit.Core.Settings; +using Bit.Core.Utilities; +using Microsoft.AspNetCore.DataProtection; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; + +namespace Bit.SharedWeb.Utilities; + +public static class DataProtectionServiceCollectionExtensions +{ + public static void AddCustomDataProtectionServices( + this IServiceCollection services, IWebHostEnvironment env, GlobalSettings globalSettings) + { + var builder = services.AddDataProtection().SetApplicationName("Bitwarden"); + + if (globalSettings.SelfHosted && CoreHelpers.SettingHasValue(globalSettings.DataProtection.Directory)) + { + builder.PersistKeysToFileSystem(new DirectoryInfo(globalSettings.DataProtection.Directory)); + } + + if (!globalSettings.SelfHosted && CoreHelpers.SettingHasValue(globalSettings.Storage?.ConnectionString)) + { + X509Certificate2? dataProtectionCert = null; + if (CoreHelpers.SettingHasValue(globalSettings.DataProtection.CertificateThumbprint)) + { + dataProtectionCert = CoreHelpers.GetCertificate( + globalSettings.DataProtection.CertificateThumbprint) + ?? throw new InvalidOperationException( + $"No data protection certificate could be found with thumbprint '{globalSettings.DataProtection.CertificateThumbprint}'."); + } + else if (CoreHelpers.SettingHasValue(globalSettings.DataProtection.CertificatePassword)) + { + dataProtectionCert = DownloadRequiredCertFromBlobStorage( + globalSettings.Storage.ConnectionString, + "certificates", + globalSettings.DataProtection.BlobName, + globalSettings.DataProtection.CertificatePassword, + "protect" + ); + } + + if (!env.IsDevelopment()) + { + if (dataProtectionCert is null) + { + throw new InvalidOperationException("A data protection certificate could not be acquired and one is required when running in non-development cloud environments. Please make sure your configuration has a valid connection string to azure blob storage."); + } + + builder + .PersistKeysToAzureBlobStorage(globalSettings.Storage.ConnectionString, "aspnet-dataprotection", "keys.xml") + .ProtectKeysWithCertificate(dataProtectionCert); + + if (globalSettings.DataProtection.UnprotectCertificates.Length > 0) + { + var unprotectCertificates = globalSettings.DataProtection.UnprotectCertificates + .Index() + .Select(i => DownloadRequiredCertFromBlobStorage( + globalSettings.Storage.ConnectionString, + "certificates", + i.Item.FileName, + i.Item.Password, + $"Unprotect {i.Index}" + )).ToArray(); + + builder.UnprotectKeysWithAnyCertificate(unprotectCertificates); + } + } + } + } + + private static X509Certificate2 DownloadRequiredCertFromBlobStorage( + string connectionString, + string container, + string file, + string password, + string context) + { + try + { + var blobServiceClient = new BlobServiceClient(connectionString); + var containerClient = blobServiceClient.GetBlobContainerClient(container); + var blobClient = containerClient.GetBlobClient(file); + + using var memoryStream = new MemoryStream(); + blobClient.DownloadTo(memoryStream); + return X509CertificateLoader.LoadPkcs12(memoryStream.ToArray(), password); + } + catch (RequestFailedException ex) + { + throw new InvalidOperationException($"Unable to download certificate from azure blob storage: {context}", ex); + } + catch (CryptographicException ex) + { + throw new InvalidOperationException($"Unable to load certificate downloaded from azure blob storage; verify the password is correct and the blob contains valid PKCS#12 data: {context}", ex); + } + } +} diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 7b4199931c79..3b2756b63feb 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -4,7 +4,6 @@ using System.Net; using System.Reflection; using System.Security.Claims; -using System.Security.Cryptography.X509Certificates; using AspNetCoreRateLimit; using Bit.Core.AdminConsole.AbilitiesCache; using Bit.Core.AdminConsole.Models.Business.Tokenables; @@ -492,56 +491,7 @@ public static void AddIdentityAuthenticationServices( } } - public static void AddCustomDataProtectionServices( - this IServiceCollection services, IWebHostEnvironment env, GlobalSettings globalSettings) - { - var builder = services.AddDataProtection().SetApplicationName("Bitwarden"); - if (globalSettings.SelfHosted && CoreHelpers.SettingHasValue(globalSettings.DataProtection.Directory)) - { - builder.PersistKeysToFileSystem(new DirectoryInfo(globalSettings.DataProtection.Directory)); - } - - if (!globalSettings.SelfHosted && CoreHelpers.SettingHasValue(globalSettings.Storage?.ConnectionString)) - { - X509Certificate2 dataProtectionCert = null; - if (CoreHelpers.SettingHasValue(globalSettings.DataProtection.CertificateThumbprint)) - { - dataProtectionCert = CoreHelpers.GetCertificate( - globalSettings.DataProtection.CertificateThumbprint); - } - else if (CoreHelpers.SettingHasValue(globalSettings.DataProtection.CertificatePassword)) - { - dataProtectionCert = CoreHelpers.GetBlobCertificateAsync( - globalSettings.Storage.ConnectionString, - "certificates", - globalSettings.DataProtection.BlobName, - globalSettings.DataProtection.CertificatePassword) - .GetAwaiter().GetResult(); - } - - if (!env.IsDevelopment()) - { - builder - .PersistKeysToAzureBlobStorage(globalSettings.Storage.ConnectionString, "aspnet-dataprotection", "keys.xml") - .ProtectKeysWithCertificate(dataProtectionCert); - - if (globalSettings.DataProtection.UnprotectCertificates.Length > 0) - { - var unprotectCertificates = Task.WhenAll(globalSettings.DataProtection.UnprotectCertificates - .Select(ci => CoreHelpers.GetBlobCertificateAsync( - globalSettings.Storage.ConnectionString, - "certificates", - ci.FileName, - ci.Password - )) - ).GetAwaiter().GetResult(); - - builder.UnprotectKeysWithAnyCertificate(unprotectCertificates); - } - } - } - } public static IIdentityServerBuilder AddIdentityServerCertificate( this IIdentityServerBuilder identityServerBuilder, IWebHostEnvironment env, GlobalSettings globalSettings) diff --git a/test/SharedWeb.Test/DataProtectionServicesTests.cs b/test/SharedWeb.Test/DataProtectionServicesTests.cs index bc47778402a4..89f6fd0a7fa7 100644 --- a/test/SharedWeb.Test/DataProtectionServicesTests.cs +++ b/test/SharedWeb.Test/DataProtectionServicesTests.cs @@ -1,5 +1,6 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using Azure; using Azure.Storage.Blobs; using Bit.Core.Settings; using Bit.SharedWeb.Utilities; @@ -334,6 +335,152 @@ await certificates.Value.UploadBlobAsync( Assert.Equal("UpdatedConfig", revertedAppProtector.Unprotect(updatedConfigData)); } + [Fact] + public async Task UnprotectCertificateMissingFromBlobStorage_Throws() + { + // The previous implementation returned null on BlobNotFound and that null reached + // UnprotectKeysWithAnyCertificate, so pods came up, failed to unprotect existing keys + // with a NullReferenceException, and auto-generated their own keys — producing a + // divergent key ring across a rolling deploy. The current implementation throws + // InvalidOperationException at startup with the call-site context ("Unprotect 0") in + // the message so operators can tell which entry went wrong from the log alone. + await using var azurite = new ContainerBuilder("mcr.microsoft.com/azure-storage/azurite") + .WithPortBinding(10000, true) + .Build(); + + await azurite.StartAsync(); + + var azuriteConnectionString = $"DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://{azurite.Hostname}:{azurite.GetMappedPublicPort(10000)}/devstoreaccount1;"; + + var blobServiceClient = new BlobServiceClient(azuriteConnectionString); + + var certificates = await blobServiceClient.CreateBlobContainerAsync("certificates"); + var dataProtection = await blobServiceClient.CreateBlobContainerAsync("aspnet-dataprotection"); + + await certificates.Value.UploadBlobAsync("dataprotection.pfx", new BinaryData(FakeInitialCert)); + await dataProtection.Value.UploadBlobAsync("keys.xml", new BinaryData(KeysData)); + + // The unprotect cert blob "mynewcert.pfx" is intentionally never uploaded. + var exception = Assert.Throws(() => CreateApp(new Dictionary + { + { "GlobalSettings:Storage:ConnectionString", azuriteConnectionString }, + { "GlobalSettings:DataProtection:CertificatePassword", "Alongside-Unworthy-Query3-Cozy" }, + { "GlobalSettings:DataProtection:UnprotectCertificates:0:FileName", "mynewcert.pfx" }, + { "GlobalSettings:DataProtection:UnprotectCertificates:0:Password", "Undergrad-Police0-Maturely-Countless" }, + })); + Assert.Contains("Unable to download certificate from azure blob storage", exception.Message); + Assert.Contains("Unprotect 0", exception.Message); + Assert.IsType(exception.InnerException); + } + + [Fact] + public async Task UnprotectCertificatePasswordIncorrect_Throws() + { + // The previous implementation swallowed the X509 load failure and returned null, which + // reached UnprotectKeysWithAnyCertificate with the same downstream effects as a missing + // blob: pods failed to unprotect existing keys with a NullReferenceException and + // auto-generated their own divergent keys across a rolling deploy. The current + // implementation throws InvalidOperationException at startup with the call-site context + // ("Unprotect 0") in the message and the underlying CryptographicException as the inner + // exception so operators can tell which entry went wrong from the log alone. + await using var azurite = new ContainerBuilder("mcr.microsoft.com/azure-storage/azurite") + .WithPortBinding(10000, true) + .Build(); + + await azurite.StartAsync(); + + var azuriteConnectionString = $"DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://{azurite.Hostname}:{azurite.GetMappedPublicPort(10000)}/devstoreaccount1;"; + + var blobServiceClient = new BlobServiceClient(azuriteConnectionString); + + var certificates = await blobServiceClient.CreateBlobContainerAsync("certificates"); + var dataProtection = await blobServiceClient.CreateBlobContainerAsync("aspnet-dataprotection"); + + await certificates.Value.UploadBlobAsync("dataprotection.pfx", new BinaryData(FakeInitialCert)); + await dataProtection.Value.UploadBlobAsync("keys.xml", new BinaryData(KeysData)); + + // The unprotect cert blob is uploaded, but the configured password does not match the + // password the cert was exported with ("Alongside-Unworthy-Query3-Cozy"). + await certificates.Value.UploadBlobAsync("mynewcert.pfx", new BinaryData(FakeInitialCert)); + + var exception = Assert.Throws(() => CreateApp(new Dictionary + { + { "GlobalSettings:Storage:ConnectionString", azuriteConnectionString }, + { "GlobalSettings:DataProtection:CertificatePassword", "Alongside-Unworthy-Query3-Cozy" }, + { "GlobalSettings:DataProtection:UnprotectCertificates:0:FileName", "mynewcert.pfx" }, + { "GlobalSettings:DataProtection:UnprotectCertificates:0:Password", "Wrong-Password-For-Cert" }, + })); + Assert.Contains("Unable to load certificate downloaded from azure blob storage", exception.Message); + Assert.Contains("Unprotect 0", exception.Message); + Assert.IsType(exception.InnerException); + } + + [Fact] + public async Task ProtectionCertificateMissingFromBlobStorage_Throws() + { + // The previous implementation passed a null certificate to ProtectKeysWithCertificate + // and the call threw with an opaque error. The current implementation throws + // InvalidOperationException with the call-site context ("protect") in the message and + // the underlying RequestFailedException as the inner exception so operators can tell + // which cert went wrong from the log alone. + await using var azurite = new ContainerBuilder("mcr.microsoft.com/azure-storage/azurite") + .WithPortBinding(10000, true) + .Build(); + + await azurite.StartAsync(); + + var azuriteConnectionString = $"DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://{azurite.Hostname}:{azurite.GetMappedPublicPort(10000)}/devstoreaccount1;"; + + var blobServiceClient = new BlobServiceClient(azuriteConnectionString); + await blobServiceClient.CreateBlobContainerAsync("certificates"); + await blobServiceClient.CreateBlobContainerAsync("aspnet-dataprotection"); + + // The protection cert blob (default name "dataprotection.pfx") is intentionally never uploaded. + var exception = Assert.Throws(() => CreateApp(new Dictionary + { + { "GlobalSettings:Storage:ConnectionString", azuriteConnectionString }, + { "GlobalSettings:DataProtection:CertificatePassword", "Alongside-Unworthy-Query3-Cozy" }, + })); + Assert.Contains("Unable to download certificate from azure blob storage", exception.Message); + Assert.Contains("protect", exception.Message); + Assert.IsType(exception.InnerException); + } + + [Fact] + public async Task ProtectionCertificatePasswordIncorrect_Throws() + { + // The previous implementation swallowed the X509 load failure and returned null, which + // ProtectKeysWithCertificate rejected with an opaque error. The current implementation + // throws InvalidOperationException with the call-site context ("protect") in the message + // and the underlying CryptographicException as the inner exception so operators can tell + // which cert went wrong from the log alone. + await using var azurite = new ContainerBuilder("mcr.microsoft.com/azure-storage/azurite") + .WithPortBinding(10000, true) + .Build(); + + await azurite.StartAsync(); + + var azuriteConnectionString = $"DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://{azurite.Hostname}:{azurite.GetMappedPublicPort(10000)}/devstoreaccount1;"; + + var blobServiceClient = new BlobServiceClient(azuriteConnectionString); + + var certificates = await blobServiceClient.CreateBlobContainerAsync("certificates"); + await blobServiceClient.CreateBlobContainerAsync("aspnet-dataprotection"); + + // The protection cert blob is uploaded, but the configured password does not match the + // password the cert was exported with ("Alongside-Unworthy-Query3-Cozy"). + await certificates.Value.UploadBlobAsync("dataprotection.pfx", new BinaryData(FakeInitialCert)); + + var exception = Assert.Throws(() => CreateApp(new Dictionary + { + { "GlobalSettings:Storage:ConnectionString", azuriteConnectionString }, + { "GlobalSettings:DataProtection:CertificatePassword", "Wrong-Password-For-Cert" }, + })); + Assert.Contains("Unable to load certificate downloaded from azure blob storage", exception.Message); + Assert.Contains("protect", exception.Message); + Assert.IsType(exception.InnerException); + } + private record TestSetupContext(BlobContainerClient Certificates, BlobContainerClient DataProtection, IConfigurationBuilder Config); private record TestRunContext(IServiceProvider Services, IDataProtector Protector);