diff --git a/src/DataProtection/DataProtection/src/Error.cs b/src/DataProtection/DataProtection/src/Error.cs index 00c1d3b59ce3..7d2d48ebf958 100644 --- a/src/DataProtection/DataProtection/src/Error.cs +++ b/src/DataProtection/DataProtection/src/Error.cs @@ -102,4 +102,9 @@ public static InvalidOperationException KeyRingProvider_RefreshFailedOnOtherThre { return new InvalidOperationException(Resources.KeyRingProvider_RefreshFailedOnOtherThread, inner); } + + public static NotSupportedException XmlKeyManager_DoesNotSupportKeyDeletion() + { + return new NotSupportedException(Resources.XmlKeyManager_DoesNotSupportKeyDeletion); + } } diff --git a/src/DataProtection/DataProtection/src/KeyManagement/IDeletableKeyManager.cs b/src/DataProtection/DataProtection/src/KeyManagement/IDeletableKeyManager.cs new file mode 100644 index 000000000000..ab97cf559d32 --- /dev/null +++ b/src/DataProtection/DataProtection/src/KeyManagement/IDeletableKeyManager.cs @@ -0,0 +1,48 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading; + +namespace Microsoft.AspNetCore.DataProtection.KeyManagement; + +/// +/// An extension of that supports key deletion. +/// +public interface IDeletableKeyManager : IKeyManager +{ + /// + /// Indicates whether this key manager and the underlying support key deletion. + /// + /// + bool CanDeleteKeys { get; } + + /// + /// Deletes keys matching a predicate. + /// + /// Use with caution as deleting active keys will normally cause data loss. + /// + /// + /// A predicate applied to each key. + /// Returning true will cause the key to be deleted. + /// + /// + /// True if all attempted deletions succeeded. + /// + /// + /// Deletion is stronger than revocation. A revoked key is retained and can even be (forcefully) applied. + /// A deleted key is indistinguishable from a key that never existed. + /// + /// Generally, keys should only be deleted to save space. If space is not a concern, keys + /// should be revoked or allowed to expire instead. + /// + /// This method will not mutate existing IKey instances. After calling this method, + /// all existing IKey instances should be discarded, and GetAllKeys should be called again. + /// + /// + /// If is false. + /// + bool DeleteKeys(Func shouldDelete); +} diff --git a/src/DataProtection/DataProtection/src/KeyManagement/XmlKeyManager.cs b/src/DataProtection/DataProtection/src/KeyManagement/XmlKeyManager.cs index aa4438b82c64..e6befb0c11b8 100644 --- a/src/DataProtection/DataProtection/src/KeyManagement/XmlKeyManager.cs +++ b/src/DataProtection/DataProtection/src/KeyManagement/XmlKeyManager.cs @@ -164,19 +164,38 @@ private static string DateTimeOffsetToFilenameSafeString(DateTimeOffset dateTime public IReadOnlyCollection GetAllKeys() { var allElements = KeyRepository.GetAllElements(); + var processed = ProcessAllElements(allElements, out _); + return processed.OfType().ToList().AsReadOnly(); + } + + /// + /// Returns an array paralleling but: + /// 1. Key elements become IKeys (with revocation data) + /// 2. KeyId-based revocations become Guids + /// 3. Date-based revocations become DateTimeOffsets + /// 4. Unknown elements become null + /// + private object?[] ProcessAllElements(IReadOnlyCollection allElements, out DateTimeOffset? mostRecentMassRevocationDate) + { + var elementCount = allElements.Count; + + var results = new object?[elementCount]; - // We aggregate all the information we read into three buckets - Dictionary keyIdToKeyMap = new Dictionary(); + Dictionary keyIdToKeyMap = []; HashSet? revokedKeyIds = null; - DateTimeOffset? mostRecentMassRevocationDate = null; + mostRecentMassRevocationDate = null; + + var pos = 0; foreach (var element in allElements) { + object? result; if (element.Name == KeyElementName) { // ProcessKeyElement can return null in the case of failure, and if this happens we'll move on. // Still need to throw if we see duplicate keys with the same id. var key = ProcessKeyElement(element); + result = key; if (key != null) { if (keyIdToKeyMap.ContainsKey(key.KeyId)) @@ -189,19 +208,20 @@ public IReadOnlyCollection GetAllKeys() else if (element.Name == RevocationElementName) { var revocationInfo = ProcessRevocationElement(element); - if (revocationInfo is Guid) + result = revocationInfo; + if (revocationInfo is Guid revocationGuid) { // a single key was revoked - if (revokedKeyIds == null) + revokedKeyIds ??= []; + if (!revokedKeyIds.Add(revocationGuid)) { - revokedKeyIds = new HashSet(); + _logger.KeyRevokedMultipleTimes(revocationGuid); } - revokedKeyIds.Add((Guid)revocationInfo); } else { // all keys as of a certain date were revoked - DateTimeOffset thisMassRevocationDate = (DateTimeOffset)revocationInfo; + var thisMassRevocationDate = (DateTimeOffset)revocationInfo; if (!mostRecentMassRevocationDate.HasValue || mostRecentMassRevocationDate < thisMassRevocationDate) { mostRecentMassRevocationDate = thisMassRevocationDate; @@ -212,16 +232,18 @@ public IReadOnlyCollection GetAllKeys() { // Skip unknown elements. _logger.UnknownElementWithNameFoundInKeyringSkipping(element.Name); + result = null; } + + results[pos++] = result; } // Apply individual revocations - if (revokedKeyIds != null) + if (revokedKeyIds is not null) { - foreach (Guid revokedKeyId in revokedKeyIds) + foreach (var revokedKeyId in revokedKeyIds) { - keyIdToKeyMap.TryGetValue(revokedKeyId, out var key); - if (key != null) + if (keyIdToKeyMap.TryGetValue(revokedKeyId, out var key)) { key.SetRevoked(); _logger.MarkedKeyAsRevokedInTheKeyring(revokedKeyId); @@ -252,7 +274,7 @@ public IReadOnlyCollection GetAllKeys() } // And we're finished! - return keyIdToKeyMap.Values.ToList().AsReadOnly(); + return results; } /// @@ -385,6 +407,76 @@ public void RevokeKey(Guid keyId, string? reason = null) reason: reason); } + /// + public bool CanDeleteKeys => KeyRepository is IDeletableXmlRepository; + + /// + public bool DeleteKeys(Func shouldDelete) + { + if (KeyRepository is not IDeletableXmlRepository xmlRepositoryWithDeletion) + { + throw Error.XmlKeyManager_DoesNotSupportKeyDeletion(); + } + + return xmlRepositoryWithDeletion.DeleteElements((deletableElements) => + { + // It is important to delete key elements before the corresponding revocation elements, + // in case the deletion fails part way - we don't want to accidentally unrevoke a key + // and then not delete it. + // Start at a non-zero value just to make it a little clearer in the debugger that it + // was set explicitly. + const int deletionOrderKey = 1; + const int deletionOrderRevocation = 2; + const int deletionOrderMassRevocation = 3; + + var deletableElementsArray = deletableElements.ToArray(); + + var allElements = deletableElements.Select(d => d.Element).ToArray(); + var processed = ProcessAllElements(allElements, out var mostRecentMassRevocationDate); + + var allKeyIds = new HashSet(); + var deletedKeyIds = new HashSet(); + + for (var i = 0; i < deletableElementsArray.Length; i++) + { + var obj = processed[i]; + if (obj is IKey key) + { + var keyId = key.KeyId; + allKeyIds.Add(keyId); + + if (shouldDelete(key)) + { + _logger.DeletingKey(keyId); + deletedKeyIds.Add(keyId); + deletableElementsArray[i].DeletionOrder = deletionOrderKey; + } + } + else if (obj is DateTimeOffset massRevocationDate) + { + if (massRevocationDate < mostRecentMassRevocationDate) + { + // Delete superceded mass revocation elements + deletableElementsArray[i].DeletionOrder = deletionOrderMassRevocation; + } + } + } + + // Separate loop since deletedKeyIds and allKeyIds need to have been populated. + for (var i = 0; i < deletableElementsArray.Length; i++) + { + if (processed[i] is Guid revocationId) + { + // Delete individual revocations of keys that don't (still) exist + if (deletedKeyIds.Contains(revocationId) || !allKeyIds.Contains(revocationId)) + { + deletableElementsArray[i].DeletionOrder = deletionOrderRevocation; + } + } + } + }); + } + private void TriggerAndResetCacheExpirationToken([CallerMemberName] string? opName = null, bool suppressLogging = false) { if (!suppressLogging) diff --git a/src/DataProtection/DataProtection/src/LoggingExtensions.cs b/src/DataProtection/DataProtection/src/LoggingExtensions.cs index 553a4f5e1f60..280f818606f3 100644 --- a/src/DataProtection/DataProtection/src/LoggingExtensions.cs +++ b/src/DataProtection/DataProtection/src/LoggingExtensions.cs @@ -255,4 +255,25 @@ private static bool IsLogLevelEnabledCore([NotNullWhen(true)] ILogger? logger, L [LoggerMessage(73, LogLevel.Debug, "Key {KeyId:B} method {MethodName} failed. Retrying.", EventName = "RetryingMethodOfKeyAfterFailure")] public static partial void RetryingMethodOfKeyAfterFailure(this ILogger logger, Guid keyId, string methodName, Exception exception); + + [LoggerMessage(74, LogLevel.Debug, "Deleting file '{FileName}'.", EventName = "DeletingFile")] + public static partial void DeletingFile(this ILogger logger, string fileName); + + [LoggerMessage(75, LogLevel.Error, "Failed to delete file '{FileName}'. Not attempting further deletions.", EventName = "FailedToDeleteFile")] + public static partial void FailedToDeleteFile(this ILogger logger, string fileName, Exception exception); + + [LoggerMessage(76, LogLevel.Debug, "Deleting registry key '{RegistryKeyName}', value '{Value}'.", EventName = "RemovingDataFromRegistryKeyValue")] + public static partial void RemovingDataFromRegistryKeyValue(this ILogger logger, RegistryKey registryKeyName, string value); + + [LoggerMessage(77, LogLevel.Error, "Failed to delete registry key '{RegistryKeyName}', value '{ValueName}'. Not attempting further deletions.", EventName = "FailedToRemoveDataFromRegistryKeyValue")] + public static partial void FailedToRemoveDataFromRegistryKeyValue(this ILogger logger, RegistryKey registryKeyName, string valueName, Exception exception); + + [LoggerMessage(78, LogLevel.Trace, "Found multiple revocation entries for key {KeyId:B}.", EventName = "KeyRevokedMultipleTimes")] + public static partial void KeyRevokedMultipleTimes(this ILogger logger, Guid keyId); + + [LoggerMessage(79, LogLevel.Trace, "Ignoring revocation of keys created before {OlderDate:u} in favor of revocation of keys created before {NewerDate:u}.", EventName = "DateBasedRevocationSuperseded")] + public static partial void DateBasedRevocationSuperseded(this ILogger logger, DateTimeOffset olderDate, DateTimeOffset newerDate); + + [LoggerMessage(80, LogLevel.Debug, "Deleting key {KeyId:B}.", EventName = "DeletingKey")] + public static partial void DeletingKey(this ILogger logger, Guid keyId); } diff --git a/src/DataProtection/DataProtection/src/PublicAPI.Unshipped.txt b/src/DataProtection/DataProtection/src/PublicAPI.Unshipped.txt index 7dc5c58110bf..de9b28a68bda 100644 --- a/src/DataProtection/DataProtection/src/PublicAPI.Unshipped.txt +++ b/src/DataProtection/DataProtection/src/PublicAPI.Unshipped.txt @@ -1 +1,14 @@ #nullable enable +Microsoft.AspNetCore.DataProtection.KeyManagement.IDeletableKeyManager +Microsoft.AspNetCore.DataProtection.KeyManagement.IDeletableKeyManager.CanDeleteKeys.get -> bool +Microsoft.AspNetCore.DataProtection.KeyManagement.IDeletableKeyManager.DeleteKeys(System.Func! shouldDelete) -> bool +Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager.CanDeleteKeys.get -> bool +Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager.DeleteKeys(System.Func! shouldDelete) -> bool +Microsoft.AspNetCore.DataProtection.Repositories.IDeletableElement +Microsoft.AspNetCore.DataProtection.Repositories.IDeletableElement.DeletionOrder.get -> int? +Microsoft.AspNetCore.DataProtection.Repositories.IDeletableElement.DeletionOrder.set -> void +Microsoft.AspNetCore.DataProtection.Repositories.IDeletableElement.Element.get -> System.Xml.Linq.XElement! +Microsoft.AspNetCore.DataProtection.Repositories.IDeletableXmlRepository +Microsoft.AspNetCore.DataProtection.Repositories.IDeletableXmlRepository.DeleteElements(System.Action!>! chooseElements) -> bool +virtual Microsoft.AspNetCore.DataProtection.Repositories.FileSystemXmlRepository.DeleteElements(System.Action!>! chooseElements) -> bool +virtual Microsoft.AspNetCore.DataProtection.Repositories.RegistryXmlRepository.DeleteElements(System.Action!>! chooseElements) -> bool diff --git a/src/DataProtection/DataProtection/src/Repositories/EphemeralXmlRepository.cs b/src/DataProtection/DataProtection/src/Repositories/EphemeralXmlRepository.cs index 9d0d0eb65075..1251d2858bf6 100644 --- a/src/DataProtection/DataProtection/src/Repositories/EphemeralXmlRepository.cs +++ b/src/DataProtection/DataProtection/src/Repositories/EphemeralXmlRepository.cs @@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.DataProtection.Repositories; /// An ephemeral XML repository backed by process memory. This class must not be used for /// anything other than dev scenarios as the keys will not be persisted to storage. /// -internal sealed class EphemeralXmlRepository : IXmlRepository +internal sealed class EphemeralXmlRepository : IDeletableXmlRepository { private readonly List _storedElements = new List(); @@ -54,4 +54,62 @@ public void StoreElement(XElement element, string friendlyName) _storedElements.Add(cloned); } } + + /// + public bool DeleteElements(Action> chooseElements) + { + ArgumentNullThrowHelper.ThrowIfNull(chooseElements); + + var deletableElements = new List(); + + lock (_storedElements) + { + foreach (var storedElement in _storedElements) + { + // Make a deep copy so caller doesn't inadvertently modify it. + deletableElements.Add(new DeletableElement(storedElement, new XElement(storedElement))); + } + } + + chooseElements(deletableElements); + + var elementsToDelete = deletableElements + .Where(e => e.DeletionOrder.HasValue) + .OrderBy(e => e.DeletionOrder.GetValueOrDefault()); + + lock (_storedElements) + { + foreach (var deletableElement in elementsToDelete) + { + var storedElement = deletableElement.StoredElement; + var index = _storedElements.FindIndex(e => ReferenceEquals(e, storedElement)); + if (index >= 0) // Might not find it if the collection has changed since we started. + { + // It would be more efficient to remove the larger indices first, but deletion order + // is important for correctness. + _storedElements.RemoveAt(index); + } + } + } + + return true; + } + + private sealed class DeletableElement : IDeletableElement + { + public DeletableElement(XElement storedElement, XElement element) + { + StoredElement = storedElement; + Element = element; + } + + /// + public XElement Element { get; } + + /// The from which was cloned. + public XElement StoredElement { get; } + + /// + public int? DeletionOrder { get; set; } + } } diff --git a/src/DataProtection/DataProtection/src/Repositories/FileSystemXmlRepository.cs b/src/DataProtection/DataProtection/src/Repositories/FileSystemXmlRepository.cs index d80ec7d44543..942d5ddf45ad 100644 --- a/src/DataProtection/DataProtection/src/Repositories/FileSystemXmlRepository.cs +++ b/src/DataProtection/DataProtection/src/Repositories/FileSystemXmlRepository.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Runtime.InteropServices; @@ -16,7 +17,7 @@ namespace Microsoft.AspNetCore.DataProtection.Repositories; /// /// An XML repository backed by a file system. /// -public class FileSystemXmlRepository : IXmlRepository +public class FileSystemXmlRepository : IDeletableXmlRepository { private readonly ILogger _logger; @@ -81,12 +82,17 @@ private IEnumerable GetAllElementsCore() // set of elements. If a file contains well-formed XML but its contents are meaningless, we // won't fail that operation here. The caller is responsible for failing as appropriate given // that scenario. - foreach (var fileSystemInfo in Directory.EnumerateFileSystemInfos("*.xml", SearchOption.TopDirectoryOnly)) + foreach (var fileSystemInfo in EnumerateFileSystemInfos()) { yield return ReadElementFromFile(fileSystemInfo.FullName); } } + private IEnumerable EnumerateFileSystemInfos() + { + return Directory.EnumerateFileSystemInfos("*.xml", SearchOption.TopDirectoryOnly); + } + private static bool IsSafeFilename(string filename) { // Must be non-empty and contain only a-zA-Z0-9, hyphen, and underscore. @@ -169,4 +175,62 @@ private void StoreElementCore(XElement element, string filename) File.Delete(tempFilename); // won't throw if the file doesn't exist } } + + /// + public virtual bool DeleteElements(Action> chooseElements) + { + ArgumentNullThrowHelper.ThrowIfNull(chooseElements); + + var deletableElements = new List(); + + foreach (var fileSystemInfo in EnumerateFileSystemInfos()) + { + var fullPath = fileSystemInfo.FullName; + var element = ReadElementFromFile(fullPath); + deletableElements.Add(new DeletableElement(fileSystemInfo, element)); + } + + chooseElements(deletableElements); + + var elementsToDelete = deletableElements + .Where(e => e.DeletionOrder.HasValue) + .OrderBy(e => e.DeletionOrder.GetValueOrDefault()); + + foreach (var deletableElement in elementsToDelete) + { + var fileSystemInfo = deletableElement.FileSystemInfo; + _logger.DeletingFile(fileSystemInfo.FullName); + try + { + fileSystemInfo.Delete(); + } + catch (Exception ex) + { + Debug.Assert(fileSystemInfo.Exists, "Having previously been deleted should not have caused an exception"); + _logger.FailedToDeleteFile(fileSystemInfo.FullName, ex); + // Stop processing deletions to avoid deleting a revocation entry for a key that we failed to delete. + return false; + } + } + + return true; + } + + private sealed class DeletableElement : IDeletableElement + { + public DeletableElement(FileSystemInfo fileSystemInfo, XElement element) + { + FileSystemInfo = fileSystemInfo; + Element = element; + } + + /// + public XElement Element { get; } + + /// The FileSystemInfo from which was read. + public FileSystemInfo FileSystemInfo { get; } + + /// + public int? DeletionOrder { get; set; } + } } diff --git a/src/DataProtection/DataProtection/src/Repositories/IDeletableElement.cs b/src/DataProtection/DataProtection/src/Repositories/IDeletableElement.cs new file mode 100644 index 000000000000..567aba2a189f --- /dev/null +++ b/src/DataProtection/DataProtection/src/Repositories/IDeletableElement.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Xml.Linq; + +namespace Microsoft.AspNetCore.DataProtection.Repositories; + +/// +/// Represents an XML element in an that can be deleted. +/// +public interface IDeletableElement +{ + /// The XML element. + public XElement Element { get; } + /// Elements are deleted in increasing DeletionOrder. null means "don't delete". + public int? DeletionOrder { get; set; } +} diff --git a/src/DataProtection/DataProtection/src/Repositories/IDeletableXmlRepository.cs b/src/DataProtection/DataProtection/src/Repositories/IDeletableXmlRepository.cs new file mode 100644 index 000000000000..2d428f2c1a39 --- /dev/null +++ b/src/DataProtection/DataProtection/src/Repositories/IDeletableXmlRepository.cs @@ -0,0 +1,27 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Xml.Linq; + +namespace Microsoft.AspNetCore.DataProtection.Repositories; + +/// +/// An extension of that supports deletion of elements. +/// +public interface IDeletableXmlRepository : IXmlRepository +{ + /// + /// Deletes selected elements from the repository. + /// + /// + /// A snapshot of the elements in this repository. + /// For each, set to a non-null value if it should be deleted. + /// Elements are deleted in increasing order. If any deletion fails, the remaining deletions *MUST* be skipped. + /// + /// + /// True if all deletions succeeded. + /// + bool DeleteElements(Action> chooseElements); +} diff --git a/src/DataProtection/DataProtection/src/Repositories/RegistryXmlRepository.cs b/src/DataProtection/DataProtection/src/Repositories/RegistryXmlRepository.cs index c1fa11b7fbe4..f8beb45cdab3 100644 --- a/src/DataProtection/DataProtection/src/Repositories/RegistryXmlRepository.cs +++ b/src/DataProtection/DataProtection/src/Repositories/RegistryXmlRepository.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.DataProtection.Repositories; /// An XML repository backed by the Windows registry. /// [SupportedOSPlatform("windows")] -public class RegistryXmlRepository : IXmlRepository +public class RegistryXmlRepository : IDeletableXmlRepository { private static readonly Lazy _defaultRegistryKeyLazy = new Lazy(GetDefaultHklmStorageKey); @@ -155,4 +155,63 @@ private void StoreElementCore(XElement element, string valueName) // but the window for that should be small enough that we shouldn't have to worry about it. RegistryKey.SetValue(valueName, element.ToString(), RegistryValueKind.String); } + + /// + public virtual bool DeleteElements(Action> chooseElements) + { + ArgumentNullThrowHelper.ThrowIfNull(chooseElements); + + var deletableElements = new List(); + + foreach (var valueName in RegistryKey.GetValueNames()) + { + var element = ReadElementFromRegKey(RegistryKey, valueName); + if (element is not null) + { + deletableElements.Add(new DeletableElement(valueName, element)); + } + } + + chooseElements(deletableElements); + + var elementsToDelete = deletableElements + .Where(e => e.DeletionOrder.HasValue) + .OrderBy(e => e.DeletionOrder.GetValueOrDefault()); + + foreach (var deletableElement in elementsToDelete) + { + var valueName = deletableElement.ValueName; + _logger.RemovingDataFromRegistryKeyValue(RegistryKey, valueName); + try + { + RegistryKey.DeleteValue(valueName, throwOnMissingValue: false); + } + catch (Exception ex) + { + _logger.FailedToRemoveDataFromRegistryKeyValue(RegistryKey, valueName, ex); + // Stop processing deletions to avoid deleting a revocation entry for a key that we failed to delete. + return false; + } + } + + return true; + } + + private sealed class DeletableElement : IDeletableElement + { + public DeletableElement(string valueName, XElement element) + { + ValueName = valueName; + Element = element; + } + + /// + public XElement Element { get; } + + /// The name of the registry value from which was read. + public string ValueName { get; } + + /// + public int? DeletionOrder { get; set; } + } } diff --git a/src/DataProtection/DataProtection/src/Resources.resx b/src/DataProtection/DataProtection/src/Resources.resx index bb00dfe220bc..c9f64f385329 100644 --- a/src/DataProtection/DataProtection/src/Resources.resx +++ b/src/DataProtection/DataProtection/src/Resources.resx @@ -159,6 +159,9 @@ The key {0:B} already exists in the keyring. For more information go to https://aka.ms/aspnet/dataprotectionwarning + + This key manager does not support key deletion. + Argument cannot be null or empty. diff --git a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/XmlKeyManagerTests.cs b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/XmlKeyManagerTests.cs index e15a94d9ff14..998088277438 100644 --- a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/XmlKeyManagerTests.cs +++ b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/KeyManagement/XmlKeyManagerTests.cs @@ -928,6 +928,97 @@ public void NovelFetchedKeyRequiresDecryption() Assert.Equal(1, decryptionCount); // Still 1 (i.e. no change) } + [Fact] + public void DeleteKeys() + { + var repository = new EphemeralXmlRepository(NullLoggerFactory.Instance); + + var keyManager = new XmlKeyManager( + Options.Create(new KeyManagementOptions() + { + AuthenticatedEncryptorConfiguration = new AuthenticatedEncryptorConfiguration(), + XmlRepository = repository, + XmlEncryptor = null + }), + SimpleActivator.DefaultWithoutServices, + NullLoggerFactory.Instance); + + var activationTime = DateTimeOffset.UtcNow.AddHours(1); + + var key1 = keyManager.CreateNewKey(activationTime, activationTime.AddMinutes(10)); + keyManager.RevokeAllKeys(DateTimeOffset.UtcNow, "Revoking all keys"); // This should revoke key1 + var key2 = keyManager.CreateNewKey(activationTime, activationTime.AddMinutes(10)); + keyManager.RevokeAllKeys(DateTimeOffset.UtcNow, "Revoking all keys"); // This should revoke key1 and key2 + var key3 = keyManager.CreateNewKey(activationTime, activationTime.AddMinutes(10)); + var key4 = keyManager.CreateNewKey(activationTime, activationTime.AddMinutes(10)); + + keyManager.RevokeKey(key2.KeyId); // Revoked by time, but also individually + keyManager.RevokeKey(key3.KeyId); + keyManager.RevokeKey(key3.KeyId); // Nothing prevents us from revoking the same key twice + + Assert.Equal(9, repository.GetAllElements().Count); // 4 keys, 2 time-revocations, 3 guid-revocations + + // The keys are stale now, but we only care about the IDs + + var keyDictWithRevocations = keyManager.GetAllKeys().ToDictionary(k => k.KeyId); + Assert.Equal(4, keyDictWithRevocations.Count); + Assert.True(keyDictWithRevocations[key1.KeyId].IsRevoked); + Assert.True(keyDictWithRevocations[key2.KeyId].IsRevoked); + Assert.True(keyDictWithRevocations[key3.KeyId].IsRevoked); + Assert.False(keyDictWithRevocations[key4.KeyId].IsRevoked); + + Assert.True(keyManager.DeleteKeys(key => key.KeyId == key1.KeyId || key.KeyId == key3.KeyId)); + + Assert.Equal(4, repository.GetAllElements().Count); // 2 keys, 1 time-revocation, 1 guid-revocations + + var keyDictWithDeletions = keyManager.GetAllKeys().ToDictionary(k => k.KeyId); + Assert.Equal(2, keyDictWithDeletions.Count); + Assert.DoesNotContain(key1.KeyId, keyDictWithDeletions.Keys); + Assert.True(keyDictWithRevocations[key2.KeyId].IsRevoked); + Assert.DoesNotContain(key3.KeyId, keyDictWithDeletions.Keys); + Assert.False(keyDictWithRevocations[key4.KeyId].IsRevoked); + } + + [Fact] + public void CanDeleteKey() + { + var withDeletion = new XmlKeyManager(Options.Create(new KeyManagementOptions() + { + XmlRepository = XmlRepositoryWithDeletion.Instance, + XmlEncryptor = null + }), SimpleActivator.DefaultWithoutServices, NullLoggerFactory.Instance); + Assert.True(withDeletion.CanDeleteKeys); + + var withoutDeletion = new XmlKeyManager(Options.Create(new KeyManagementOptions() + { + XmlRepository = XmlRepositoryWithoutDeletion.Instance, + XmlEncryptor = null + }), SimpleActivator.DefaultWithoutServices, NullLoggerFactory.Instance); + Assert.False(withoutDeletion.CanDeleteKeys); + Assert.Throws(() => withoutDeletion.DeleteKeys(_ => false)); + } + + private sealed class XmlRepositoryWithoutDeletion : IXmlRepository + { + public static readonly IXmlRepository Instance = new XmlRepositoryWithoutDeletion(); + + private XmlRepositoryWithoutDeletion() { } + + IReadOnlyCollection IXmlRepository.GetAllElements() => []; + void IXmlRepository.StoreElement(XElement element, string friendlyName) => throw new InvalidOperationException(); + } + + private sealed class XmlRepositoryWithDeletion : IDeletableXmlRepository + { + public static readonly IDeletableXmlRepository Instance = new XmlRepositoryWithDeletion(); + + private XmlRepositoryWithDeletion() { } + + IReadOnlyCollection IXmlRepository.GetAllElements() => []; + void IXmlRepository.StoreElement(XElement element, string friendlyName) => throw new InvalidOperationException(); + bool IDeletableXmlRepository.DeleteElements(Action> chooseElements) => throw new InvalidOperationException(); + } + private class MyDeserializer : IAuthenticatedEncryptorDescriptorDeserializer { public IAuthenticatedEncryptorDescriptor ImportFromXml(XElement element) diff --git a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/EphemeralXmlRepositoryTests.cs b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/EphemeralXmlRepositoryTests.cs index fd517a23d90f..e00b13c5d7bd 100644 --- a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/EphemeralXmlRepositoryTests.cs +++ b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/EphemeralXmlRepositoryTests.cs @@ -34,4 +34,77 @@ public void Store_Then_Get() repository.StoreElement(element3, null); Assert.Equal(new[] { element1, element2, element3 }, repository.GetAllElements(), XmlAssert.EqualityComparer); } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void DeleteElements(bool delete1, bool delete2) + { + var repository = new EphemeralXmlRepository(NullLoggerFactory.Instance); + + var element1 = new XElement("element1"); + var element2 = new XElement("element2"); + + repository.StoreElement(element1, friendlyName: null); + repository.StoreElement(element2, friendlyName: null); + + var ranSelector = false; + + Assert.True(repository.DeleteElements(deletableElements => + { + ranSelector = true; + Assert.Equal(2, deletableElements.Count); + + foreach (var element in deletableElements) + { + switch (element.Element.Name.LocalName) + { + case "element1": + element.DeletionOrder = delete1 ? 1 : null; + break; + case "element2": + element.DeletionOrder = delete2 ? 2 : null; + break; + default: + Assert.Fail("Unexpected element name: " + element.Element.Name.LocalName); + break; + } + } + })); + Assert.True(ranSelector); + + var elementSet = new HashSet(repository.GetAllElements().Select(e => e.Name.LocalName)); + + Assert.InRange(elementSet.Count, 0, 2); + + Assert.Equal(!delete1, elementSet.Contains(element1.Name.LocalName)); + Assert.Equal(!delete2, elementSet.Contains(element2.Name.LocalName)); + } + + [Fact] + public void DeleteElementsWithOutOfBandDeletion() + { + var repository = new EphemeralXmlRepository(NullLoggerFactory.Instance); + + repository.StoreElement(new XElement("element1"), friendlyName: "friendly1"); + + var ranSelector = false; + + Assert.True(repository.DeleteElements(deletableElements => + { + ranSelector = true; + + // Now that the repository has read the element from the registry, delete it out-of-band. + repository.DeleteElements(deletableElements => deletableElements.First().DeletionOrder = 1); + + Assert.Equal(1, deletableElements.Count); + + deletableElements.First().DeletionOrder = 1; + })); + Assert.True(ranSelector); + + Assert.Empty(repository.GetAllElements()); + } } diff --git a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/FileSystemXmlRepositoryTests.cs b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/FileSystemXmlRepositoryTests.cs index 37aa52699d47..036b1ed4596d 100644 --- a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/FileSystemXmlRepositoryTests.cs +++ b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/FileSystemXmlRepositoryTests.cs @@ -138,6 +138,142 @@ public void StoreElements_ThenRetrieve_SeesAllElements() }); } + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void DeleteElements(bool delete1, bool delete2) + { + WithUniqueTempDirectory(dirInfo => + { + var repository = new FileSystemXmlRepository(dirInfo, NullLoggerFactory.Instance); + + var element1 = new XElement("element1"); + var element2 = new XElement("element2"); + + repository.StoreElement(element1, friendlyName: null); + repository.StoreElement(element2, friendlyName: null); + + var ranSelector = false; + + Assert.True(repository.DeleteElements(deletableElements => + { + ranSelector = true; + Assert.Equal(2, deletableElements.Count); + + foreach (var element in deletableElements) + { + switch (element.Element.Name.LocalName) + { + case "element1": + element.DeletionOrder = delete1 ? 1 : null; + break; + case "element2": + element.DeletionOrder = delete2 ? 2 : null; + break; + default: + Assert.Fail("Unexpected element name: " + element.Element.Name.LocalName); + break; + } + } + })); + Assert.True(ranSelector); + + var elementSet = new HashSet(repository.GetAllElements().Select(e => e.Name.LocalName)); + + Assert.InRange(elementSet.Count, 0, 2); + + Assert.Equal(!delete1, elementSet.Contains(element1.Name.LocalName)); + Assert.Equal(!delete2, elementSet.Contains(element2.Name.LocalName)); + }); + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Linux, SkipReason = "Making FileSystemInfo.Delete throw on Linux is hard")] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "Making FileSystemInfo.Delete throw on macOS is hard")] + public void DeleteElementsWithFailure() + { + WithUniqueTempDirectory(dirInfo => + { + var repository = new FileSystemXmlRepository(dirInfo, NullLoggerFactory.Instance); + + repository.StoreElement(new XElement("element1"), friendlyName: "friendly1"); + repository.StoreElement(new XElement("element2"), friendlyName: "friendly2"); + repository.StoreElement(new XElement("element3"), friendlyName: "friendly3"); + + var filePath1 = Path.Combine(dirInfo.FullName, "friendly1.xml"); + var filePath2 = Path.Combine(dirInfo.FullName, "friendly2.xml"); + var filePath3 = Path.Combine(dirInfo.FullName, "friendly3.xml"); + + Assert.True(File.Exists(filePath1)); + Assert.True(File.Exists(filePath2)); + Assert.True(File.Exists(filePath3)); + + IDisposable fileLock2 = null; + try + { + var ranSelector = false; + Assert.False(repository.DeleteElements(deletableElements => + { + ranSelector = true; + + // Now that the repository has read the files from disk, lock one to prevent deletion from succeeding + fileLock2 = new FileStream(Path.Combine(dirInfo.FullName, "friendly2.xml"), FileMode.Open, FileAccess.ReadWrite, FileShare.None); + + Assert.Equal(3, deletableElements.Count); + + var i = 4; + foreach (var deletableElement in deletableElements) + { + // Delete in reverse alphabetical order, so the results aren't coincidental. + deletableElement.DeletionOrder = i--; + } + })); + Assert.True(ranSelector); + } + finally + { + fileLock2?.Dispose(); + } + + Assert.True(File.Exists(filePath1)); // Deletion not attempted after failure + Assert.True(File.Exists(filePath2)); // Deletion fails because of lock + Assert.False(File.Exists(filePath3)); // Deleted before error + }); + } + + [Fact] + public void DeleteElementsWithOutOfBandDeletion() + { + WithUniqueTempDirectory(dirInfo => + { + var repository = new FileSystemXmlRepository(dirInfo, NullLoggerFactory.Instance); + + repository.StoreElement(new XElement("element1"), friendlyName: "friendly1"); + + var filePath = Path.Combine(dirInfo.FullName, "friendly1.xml"); + Assert.True(File.Exists(filePath)); + + var ranSelector = false; + + Assert.True(repository.DeleteElements(deletableElements => + { + ranSelector = true; + + // Now that the repository has read the element from disk, delete it out-of-band. + File.Delete(filePath); + + Assert.Equal(1, deletableElements.Count); + + deletableElements.First().DeletionOrder = 1; + })); + Assert.True(ranSelector); + + Assert.False(File.Exists(filePath)); + }); + } + [ConditionalFact] [DockerOnly] [Trait("Docker", "true")] diff --git a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/RegistryXmlRepositoryTests.cs b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/RegistryXmlRepositoryTests.cs index e098785d0678..1835eb4b39a1 100644 --- a/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/RegistryXmlRepositoryTests.cs +++ b/src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Repositories/RegistryXmlRepositoryTests.cs @@ -125,6 +125,97 @@ public void StoreElements_ThenRetrieve_SeesAllElements() }); } + [ConditionalTheory] + [ConditionalRunTestOnlyIfHkcuRegistryAvailable] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void DeleteElements(bool delete1, bool delete2) + { + WithUniqueTempRegKey(regKey => + { + var repository = new RegistryXmlRepository(regKey, NullLoggerFactory.Instance); + + var element1 = new XElement("element1"); + var element2 = new XElement("element2"); + + repository.StoreElement(element1, friendlyName: null); + repository.StoreElement(element2, friendlyName: null); + + var ranSelector = false; + + Assert.True(repository.DeleteElements(deletableElements => + { + ranSelector = true; + Assert.Equal(2, deletableElements.Count); + + foreach (var element in deletableElements) + { + switch (element.Element.Name.LocalName) + { + case "element1": + element.DeletionOrder = delete1 ? 1 : null; + break; + case "element2": + element.DeletionOrder = delete2 ? 2 : null; + break; + default: + Assert.Fail("Unexpected element name: " + element.Element.Name.LocalName); + break; + } + } + })); + Assert.True(ranSelector); + + var elementSet = new HashSet(repository.GetAllElements().Select(e => e.Name.LocalName)); + + Assert.InRange(elementSet.Count, 0, 2); + + Assert.Equal(!delete1, elementSet.Contains(element1.Name.LocalName)); + Assert.Equal(!delete2, elementSet.Contains(element2.Name.LocalName)); + }); + } + + // It would be nice to have a test paralleling the one in FileSystemXmlRepositoryTests.cs, + // but there's no obvious way to simulate a failure for only one of the values. You can + // lock a whole key, but not individual values, and we don't have a hook to let us lock the + // whole key while a particular value deletion is attempted. + //[ConditionalFact] + //[ConditionalConditionalRunTestOnlyIfHkcuRegistryAvailable] + //public void DeleteElementsWithFailure() + + [ConditionalFact] + [ConditionalRunTestOnlyIfHkcuRegistryAvailable] + public void DeleteElementsWithOutOfBandDeletion() + { + WithUniqueTempRegKey(regKey => + { + var repository = new RegistryXmlRepository(regKey, NullLoggerFactory.Instance); + + repository.StoreElement(new XElement("element1"), friendlyName: "friendly1"); + + Assert.NotNull(regKey.GetValue("friendly1")); + + var ranSelector = false; + + Assert.True(repository.DeleteElements(deletableElements => + { + ranSelector = true; + + // Now that the repository has read the element from the registry, delete it out-of-band. + regKey.DeleteValue("friendly1"); + + Assert.Equal(1, deletableElements.Count); + + deletableElements.First().DeletionOrder = 1; + })); + Assert.True(ranSelector); + + Assert.Null(regKey.GetValue("friendly1")); + }); + } + /// /// Runs a test and cleans up the registry key afterward. ///