Data Protection: Azure modes, certificate rotation, and comprehensive tests - #93
Merged
Conversation
…edis CertificateKeyEncryptionConfigurator only set XmlEncryptor; decryption fell back to a default that looks up certificates in the OS cert store, but the providers load with EphemeralKeySet, so any cross-host scenario silently lost access to existing key XML at startup. DataProtectionModule now eagerly loads the cert provider and calls UnprotectKeysWithAnyCertificate(current + previous), the only public surface for wiring XmlKeyDecryptionOptions. Adds DataProtection:PreviousCertificatePaths and DataProtection:PreviousAzureBlobUrls to support certificate rotation without stranding key XML written under the previous cert. Also switches IDataProtectionCertificateProvider from async to sync — every consumer is sync at the DataProtection wiring boundary, and AzureBlob now uses BlobClient.DownloadContent (sync) rather than blocking on DownloadContentAsync.
Covers key persistence across API host restarts, key rotation, and certificate rotation for both FileSystem and Redis modes. Adds a Testcontainers-backed Redis fixture and a shared base class for the new lifecycle suite. Updates the existing FileSystem provider and CertificateKeyEncryptionConfigurator unit tests for the new synchronous IDataProtectionCertificateProvider interface.
Updates the configuration guide, security model, and deployment architecture doc to reflect the actual flat configuration keys (CertificatePath, AzureBlobUrl) and the new PreviousCertificatePaths / PreviousAzureBlobUrls arrays. Replaces aspirational nested config examples and adds the rotation workflow.
…ith options classes
Move certificate-provider configuration off raw IConfiguration["..."]
reads onto strongly-typed FileSystemCertificateOptions and
AzureBlobCertificateOptions with source-generated [OptionsValidator]
validators, mirroring the existing Redis/Azure sub-options pattern.
The certificate providers now inject IOptions<T> instead of
IConfiguration; the dead duplicate properties on DataProtectionOptions
are removed; and the parent validator is reduced to a thin
mode-conditional dispatcher.
Also fix a latent bug where Redis/Azure validation failures were
silently swallowed by an inverted TryValidate condition.
Schema change (feature is new on this branch):
- DataProtection:CertificatePath -> DataProtection:FileSystemCertificate:Path
- DataProtection:CertificatePassword -> DataProtection:FileSystemCertificate:Password
or DataProtection:AzureBlobCertificate:Password
- DataProtection:PreviousCertificatePaths -> DataProtection:FileSystemCertificate:PreviousPaths
- DataProtection:AzureBlobUrl -> DataProtection:AzureBlobCertificate:BlobUri
- DataProtection:PreviousAzureBlobUrls -> DataProtection:AzureBlobCertificate:PreviousBlobUris
Replace hard-coded DefaultAzureCredential in AzureKeyRingConfigurator and AzureBlobCertificateProvider with a configurable AzureCredentialType (Default, ManagedIdentity, WorkloadIdentity, ClientSecret, AzureCli, Environment) bound under DataProtection:AzureCredential, with cross-field validation per type. The credential is built once in DataProtectionModule and shared across the key ring configurator and the cert provider.
…stom IXmlDecryptor Replace the eager UnprotectKeysWithAnyCertificate + manual provider instantiation with a custom IXmlEncryptor/IXmlDecryptor pair. The encryptor delegates the cryptographic work to the framework's CertificateXmlEncryptor and only swaps the recorded DecryptorType to GroundControlCertificateXmlDecryptor; the decryptor takes the IServiceProvider that ASP.NET Core's SimpleActivator hands it and resolves IDataProtectionCertificateProvider lazily on first decrypt. Eliminates the bootstrap-container question, the NullLogger workaround, and the triple-cert-load on startup. The decryptor falls back to EncryptedXml.DecryptEncryptedKey when no matching cert is found, so the existing rotation tests still pass.
GroundControlCertificateXmlEncryptor previously constructed a fresh framework CertificateXmlEncryptor on every Encrypt call. Cache it in a single-slot keyed by thumbprint so we only rebuild when the underlying certificate changes. Single-reference assignment is atomic, so no lock is needed; concurrent first-time writes may construct a duplicate inner encryptor (harmless). Also adds a negative round-trip test asserting that decryption with no matching certificate throws CryptographicException and emits the "no matching cert" warning naming the orphaned thumbprint.
The hosted service existed to (a) log the active cert thumbprint at startup and (b) act as a fail-fast checkpoint by eagerly loading the cert. After the IXmlEncryptor/IXmlDecryptor refactor: - Logging is already covered by FileSystemCertificateProvider and AzureBlobCertificateProvider, which log thumbprints on every load. - Cert loading is now fully lazy through the DI-resolved provider, so removing the logger means the host no longer fail-fasts on a missing cert; misconfig will surface on the first request that protects or unprotects a sensitive value instead.
…vider Production wiring is unchanged: the public ctor still constructs a real BlobClient against the supplied URI and TokenCredential. The internal overload accepts a Func<Uri, TokenCredential, BlobClient> so unit tests can mock the download path without depending on Azurite or live Azure. ActivatorUtilities only considers public constructors, so DI keeps picking the production ctor and ignores the seam.
Fill the gaps in Data Protection coverage that the existing lifecycle suite did not exercise: - options validators (DataProtectionOptions, AzureCredentialOptions) - AzureCredentialFactory mode-to-credential mapping - DataProtectionModule DI registration across every mode/provider combo - GroundControlCertificateXmlEncryptor inner-encryptor cache identity and concurrency - GroundControlCertificateXmlDecryptor edge cases (null args, missing private key, SimpleActivator-compatible IServiceProvider ctor) - AzureBlobCertificateProvider download/load via the new factory seam - FileSystem DPAPI Windows / non-Windows branches - Redis connection-failure wrap, Azure registration happy path - DataProtectionValueProtector empty/unicode/large payloads, malformed input, multi-protector interop, concurrent round-trip - DataProtectionOptions configuration binding (JSON + env vars) - FileSystem and Redis scaled-out scenarios (live peer reads) - Certificate rotation negative paths (premature old-cert removal) Adds Azure.Storage.Blobs to the test project so AzureBlobCertificateProviderTests can construct mocked BlobDownloadResult responses via BlobsModelFactory.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings the Data Protection module from "FileSystem-mode happy path only" to a fully testable, documented, multi-mode feature. The branch covers four logical chunks:
Feature & infrastructure (existing commits)
IXmlDecryptorthat resolves certificates lazily — no eager loading orUnprotectKeysWithAnyCertificate.Default,ManagedIdentity,WorkloadIdentity,ClientSecret,AzureCli,Environment) shared between the Azure key ring and the AzureBlob certificate provider.DataProtectionOptions,AzureCredentialOptions,RedisOptions,AzureOptions,FileSystemCertificateOptions,AzureBlobCertificateOptions) and source-generated validators.GroundControlCertificateXmlEncryptorto avoid re-allocatingCertificateXmlEncryptoron every protect.CertificateStartupLogger— the certificate provider is now exercised on first protect/unprotect rather than at startup.docs/design-docs/Security-Model.md.GHSA-pggp-6c3x-2xmx.Test seam (final two commits)
AzureBlobCertificateProvidergains an internal 4-arg ctor accepting aFunc<Uri, TokenCredential, BlobClient>. Production wiring picks the public 3-arg ctor; tests inject a mockedBlobClientso the X.509 download path can be exercised without Azurite or live Azure.Test coverage expansion
Adds 11 new test files plus extensions to 7 existing ones, closing every gap identified in the test audit:
DataProtectionOptions,AzureCredentialOptions)AzureCredentialFactorymode-to-credential mappingDataProtectionModuleDI registration across every mode/provider comboGroundControlCertificateXmlEncryptorinner-encryptor cache identity and concurrencyGroundControlCertificateXmlDecryptoredge cases (null args, missing private key,SimpleActivator-compatibleIServiceProviderctor)AzureBlobCertificateProviderdownload/load via the new factory seamDataProtectionValueProtectorempty/unicode/large payloads, malformed input, multi-protector interop, concurrent round-tripDataProtectionOptionsconfiguration binding (JSON + env vars)Out of scope
ProtectKeysWithAzureKeyVaultmakes real KV calls when wrapping/unwrapping keys. Coverage stops at registration verification (AzureKeyRingConfiguratorTests.Configure_HappyPath_WiresUpAzureBlobRepositoryAndAzureKeyVaultEncryptor).