From 5126fc115ec6ba20c9f854a243484d06c393591d Mon Sep 17 00:00:00 2001 From: StefanGreve Date: Mon, 8 Jul 2024 19:24:18 +0200 Subject: [PATCH 1/7] Update readme --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index 93444ee..34b5d7c 100644 --- a/readme.md +++ b/readme.md @@ -15,7 +15,7 @@ This project uses [stryker](https://stryker-mutator.io/) for mutation testing, w setup to be installed with ```powershell -dotnet tool restore +dotnet tool restore --configfile .\AdvancedSystems.Security\nuget.config ``` Run stryker locally: From aa56e6a03b155e1e1bef8fb4564d353034c8849d Mon Sep 17 00:00:00 2001 From: StefanGreve Date: Mon, 8 Jul 2024 19:26:00 +0200 Subject: [PATCH 2/7] Test bad paths in CertificateService --- .../ICertificateService.cs | 2 +- .../Cryptography/HashTests.cs | 4 ++ .../Fixtures/CertificateFixture.cs | 27 +++++++ .../Fixtures/HashFixture.cs | 4 +- .../Services/CertificateServiceTests.cs | 70 +++++++++++++++++++ .../Services/HashServiceTests.cs | 22 +++--- .../Services/CertificateService.cs | 6 +- 7 files changed, 120 insertions(+), 15 deletions(-) create mode 100644 AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs create mode 100644 AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs diff --git a/AdvancedSystems.Security.Abstractions/ICertificateService.cs b/AdvancedSystems.Security.Abstractions/ICertificateService.cs index 96aabde..320b00f 100644 --- a/AdvancedSystems.Security.Abstractions/ICertificateService.cs +++ b/AdvancedSystems.Security.Abstractions/ICertificateService.cs @@ -6,7 +6,7 @@ public interface ICertificateService { #region Methods - X509Certificate2? GetStoreCertificate(StoreName storeName, StoreLocation storeLocation, string thumbprint); + X509Certificate2? GetStoreCertificate(string thumbprint, StoreName storeName, StoreLocation storeLocation); X509Certificate2? GetConfiguredCertificate(); diff --git a/AdvancedSystems.Security.Tests/Cryptography/HashTests.cs b/AdvancedSystems.Security.Tests/Cryptography/HashTests.cs index 0449199..699b0b5 100644 --- a/AdvancedSystems.Security.Tests/Cryptography/HashTests.cs +++ b/AdvancedSystems.Security.Tests/Cryptography/HashTests.cs @@ -10,6 +10,8 @@ namespace AdvancedSystems.Security.Tests.Cryptography; public class HashTests { + #region Tests + [Theory] [InlineData("Hello, World!", "65a8e27d8879283831b664bd8b7f0ad4", Format.Hex)] [InlineData("Hello, World!", "ZajifYh5KDgxtmS9i38K1A==", Format.Base64)] @@ -104,4 +106,6 @@ public void TestSHA512Hash(string input, string expected, Format format) // Assert Assert.Equal(expected, sha512); } + + #endregion } diff --git a/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs b/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs new file mode 100644 index 0000000..b9445e0 --- /dev/null +++ b/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs @@ -0,0 +1,27 @@ +using AdvancedSystems.Security.Abstractions; +using AdvancedSystems.Security.Options; +using AdvancedSystems.Security.Services; + +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +using Moq; + +namespace AdvancedSystems.Security.Tests.Fixtures; + +public class CertificateFixture +{ + public Mock> Logger { get; private set; } + + public ICertificateService CertificateService { get; private set; } + + public Mock> Options { get; private set; } + + public CertificateFixture() + { + this.Logger = new Mock>(); + + this.Options = new Mock>(); + this.CertificateService = new CertificateService(this.Logger.Object, this.Options.Object); + } +} diff --git a/AdvancedSystems.Security.Tests/Fixtures/HashFixture.cs b/AdvancedSystems.Security.Tests/Fixtures/HashFixture.cs index bf9ac75..f570feb 100644 --- a/AdvancedSystems.Security.Tests/Fixtures/HashFixture.cs +++ b/AdvancedSystems.Security.Tests/Fixtures/HashFixture.cs @@ -9,9 +9,9 @@ namespace AdvancedSystems.Security.Tests.Fixtures; public class HashServiceFixture { - public IHashService HashService { get; set; } + public Mock> Logger { get; private set; } - public Mock> Logger { get; set; } + public IHashService HashService { get; private set; } public HashServiceFixture() { diff --git a/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs b/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs new file mode 100644 index 0000000..6477eed --- /dev/null +++ b/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs @@ -0,0 +1,70 @@ +using System.Security.Cryptography.X509Certificates; + +using AdvancedSystems.Security.Options; +using AdvancedSystems.Security.Tests.Fixtures; + +using Xunit; + +namespace AdvancedSystems.Security.Tests.Services; + +public class CertificateServiceTests : IClassFixture +{ + private readonly CertificateFixture _fixture; + + public CertificateServiceTests(CertificateFixture fixture) + { + this._fixture = fixture; + } + + #region Tests + + [Fact] + public void TestGetStoreCertificate() + { + + } + + [Fact] + public void TestGetStoreCertificate_NotFound() + { + // Arrange + string thumbprint = "123456789"; + var storeName = StoreName.My; + var storeLocation = StoreLocation.CurrentUser; + + // Act + var certificate = this._fixture.CertificateService.GetStoreCertificate(thumbprint, storeName, storeLocation); + + // Assert + Assert.Null(certificate); + } + + [Fact] + public void GetConfiguredCertificate() + { + + } + + [Fact] + public void GetConfiguredCertificate_NotFound() + { + // Arrange + var options = new CertificateOptions + { + Thumbprint = "123456789", + StoreName = StoreName.My, + StoreLocation = StoreLocation.CurrentUser + }; + + this._fixture.Options.Setup(x => x.Value) + .Returns(options); + + // Act + var certificate = this._fixture.CertificateService.GetConfiguredCertificate(); + + // Assert + Assert.Null(certificate); + } + + #endregion +} diff --git a/AdvancedSystems.Security.Tests/Services/HashServiceTests.cs b/AdvancedSystems.Security.Tests/Services/HashServiceTests.cs index 5575fd1..360c04e 100644 --- a/AdvancedSystems.Security.Tests/Services/HashServiceTests.cs +++ b/AdvancedSystems.Security.Tests/Services/HashServiceTests.cs @@ -13,13 +13,15 @@ namespace AdvancedSystems.Security.Tests.Services; public class HashServiceTests : IClassFixture { - private readonly HashServiceFixture _sut; + private readonly HashServiceFixture _fixture; public HashServiceTests(HashServiceFixture fixture) { - this._sut = fixture; + this._fixture = fixture; } + #region Tests + [Fact] public void TestMD5Hash() { @@ -29,12 +31,12 @@ public void TestMD5Hash() // Act #pragma warning disable CS0618 // Type or member is obsolete - string md5 = this._sut.HashService.GetMD5Hash(buffer); + string md5 = this._fixture.HashService.GetMD5Hash(buffer); #pragma warning restore CS0618 // Type or member is obsolete // Assert Assert.Equal("9e107d9d372bb6826bd81d3542a419d6", md5); - this._sut.Logger.Verify(x => x.Log( + this._fixture.Logger.Verify(x => x.Log( LogLevel.Warning, It.IsAny(), It.Is((v, t) => v.ToString()!.StartsWith("Computing hash with a cryptographically insecure hash algorithm")), @@ -52,12 +54,12 @@ public void TestSHA1Hash() // Act #pragma warning disable CS0618 // Type or member is obsolete - string sha1 = this._sut.HashService.GetSHA1Hash(buffer); + string sha1 = this._fixture.HashService.GetSHA1Hash(buffer); #pragma warning restore CS0618 // Type or member is obsolete // Assert Assert.Equal("2fd4e1c67a2d28fced849ee1bb76e7391b93eb12", sha1); - this._sut.Logger.Verify(x => x.Log( + this._fixture.Logger.Verify(x => x.Log( LogLevel.Warning, It.IsAny(), It.Is((v, t) => v.ToString()!.StartsWith("Computing hash with a cryptographically insecure hash algorithm")), @@ -74,7 +76,7 @@ public void TestSHA256Hash() byte[] buffer = Encoding.UTF8.GetBytes(input); // Act - string sha256 = this._sut.HashService.GetSHA256Hash(buffer); + string sha256 = this._fixture.HashService.GetSHA256Hash(buffer); // Assert Assert.Equal("d7a8fbb307d7809469ca9abcb0082e4f8d5651e46d3cdb762d02d0bf37c9e592", sha256); @@ -88,7 +90,7 @@ public void TestSHA384Hash() byte[] buffer = Encoding.UTF8.GetBytes(input); // Act - string sha384 = this._sut.HashService.GetSHA384Hash(buffer); + string sha384 = this._fixture.HashService.GetSHA384Hash(buffer); // Assert Assert.Equal("ca737f1014a48f4c0b6dd43cb177b0afd9e5169367544c494011e3317dbf9a509cb1e5dc1e85a941bbee3d7f2afbc9b1", sha384); @@ -102,9 +104,11 @@ public void TestSHA512Hash() byte[] buffer = Encoding.UTF8.GetBytes(input); // Act - string sha512 = this._sut.HashService.GetSHA512Hash(buffer); + string sha512 = this._fixture.HashService.GetSHA512Hash(buffer); // Assert Assert.Equal("07e547d9586f6a73f73fbac0435ed76951218fb7d0c8d788a309d785436bbb642e93a252a954f23912547d1e8a3b5ed6e1bfd7097821233fa0538f3db854fee6", sha512); } + + #endregion } diff --git a/AdvancedSystems.Security/Services/CertificateService.cs b/AdvancedSystems.Security/Services/CertificateService.cs index e42ca14..b37d8ac 100644 --- a/AdvancedSystems.Security/Services/CertificateService.cs +++ b/AdvancedSystems.Security/Services/CertificateService.cs @@ -25,14 +25,14 @@ public CertificateService(ILogger logger, IOptions this._logger.LogError(exception, "Service failed to retrieve certificate."))) + catch (CertificateNotFoundException exception) when (True(() => this._logger.LogError(exception, "{Service} failed to retrieve certificate.", nameof(CertificateService)))) { return null; } @@ -41,7 +41,7 @@ public CertificateService(ILogger logger, IOptions Date: Tue, 9 Jul 2024 10:35:12 +0200 Subject: [PATCH 3/7] Add ICertificateStore abstraction to improve testability of the CertificateService * The default implementation of CertificateStore is still tighly coupled to the X509Store. Because it is a thin wrapper over a native library, testing this service won't be necessary. * The GetCertificate extension method now also returns invalid certificates, so that the caller side can take over the burden of validation, which also happens to make writing the unit tests much easier. --- .../CertificateNotFoundException.cs | 43 +++++++ .../ICertificateService.cs | 19 ++- .../ICertificateStore.cs | 110 ++++++++++++++++++ .../IHashService.cs | 2 +- .../Fixtures/CertificateFixture.cs | 34 +++++- .../Services/CertificateServiceTests.cs | 43 ++++++- .../CryptoServiceCollectionExtensions.cs | 18 +++ .../CertificateNotFoundException.cs | 42 ------- .../CertificateStore.cs} | 20 ++-- .../Services/CertificateService.cs | 10 +- .../Services/CertificateStore.cs | 108 +++++++++++++++++ 11 files changed, 387 insertions(+), 62 deletions(-) create mode 100644 AdvancedSystems.Security.Abstractions/Exceptions/CertificateNotFoundException.cs create mode 100644 AdvancedSystems.Security.Abstractions/ICertificateStore.cs delete mode 100644 AdvancedSystems.Security/Exceptions/CertificateNotFoundException.cs rename AdvancedSystems.Security/{Cryptography/Certificate.cs => Extensions/CertificateStore.cs} (53%) create mode 100644 AdvancedSystems.Security/Services/CertificateStore.cs diff --git a/AdvancedSystems.Security.Abstractions/Exceptions/CertificateNotFoundException.cs b/AdvancedSystems.Security.Abstractions/Exceptions/CertificateNotFoundException.cs new file mode 100644 index 0000000..3ecf24a --- /dev/null +++ b/AdvancedSystems.Security.Abstractions/Exceptions/CertificateNotFoundException.cs @@ -0,0 +1,43 @@ +using System; + +namespace AdvancedSystems.Security.Abstractions.Exceptions +{ + /// + /// Represents errors that occur because a specified certificate could not be located. + /// + /// + /// This exception is typically thrown when an attempt to retrieve a certificate by its + /// identifier, such as a thumbprint or subject name, fails. It indicates that the required + /// certificate is not present in the specified certificate store or location. + /// + public class CertificateNotFoundException : Exception + { + /// + /// Initializes a new instance of the class. + /// + public CertificateNotFoundException() + { + + } + + /// + /// Initializes a new instance of the class with a specified error . + /// + /// The error message that explains the reason for the exception. + public CertificateNotFoundException(string message) : base(message) + { + + } + + /// + /// Initializes a new instance of the class with a specified error + /// a reference to the exception that is the cause of this exception. + /// + /// The error message that explains the reason for the exception. + /// The exception that is the cause of the current exception, or a null reference if no inner exception is specified. + public CertificateNotFoundException(string message, Exception inner) : base(message, inner) + { + + } + } +} diff --git a/AdvancedSystems.Security.Abstractions/ICertificateService.cs b/AdvancedSystems.Security.Abstractions/ICertificateService.cs index 320b00f..8356af5 100644 --- a/AdvancedSystems.Security.Abstractions/ICertificateService.cs +++ b/AdvancedSystems.Security.Abstractions/ICertificateService.cs @@ -1,13 +1,30 @@ -using System.Security.Cryptography.X509Certificates; +using AdvancedSystems.Security.Abstractions.Exceptions; + +using System.Security.Cryptography.X509Certificates; namespace AdvancedSystems.Security.Abstractions { + /// + /// Defines a service for managing and retrieving X.509 certificates. + /// public interface ICertificateService { #region Methods + /// + /// Retrieves an X.509 certificate from the specified store using the provided . + /// + /// The thumbprint of the certificate to locate. + /// The certificate store from which to retrieve the certificate. + /// The location of the certificate store, such as or . + /// The object if the certificate is found, else null. + /// Thrown when no certificate with the specified thumbprint is found in the store. X509Certificate2? GetStoreCertificate(string thumbprint, StoreName storeName, StoreLocation storeLocation); + /// + /// Retrieves an application-configured X.509 certificate. + /// + /// The object if the certificate is found, else null. X509Certificate2? GetConfiguredCertificate(); #endregion diff --git a/AdvancedSystems.Security.Abstractions/ICertificateStore.cs b/AdvancedSystems.Security.Abstractions/ICertificateStore.cs new file mode 100644 index 0000000..4627ecc --- /dev/null +++ b/AdvancedSystems.Security.Abstractions/ICertificateStore.cs @@ -0,0 +1,110 @@ +using System; +using System.Security; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; + +namespace AdvancedSystems.Security.Abstractions +{ + /// + /// Represents an X.509 store, which is a physical store where certificates are persisted and managed. + /// + public interface ICertificateStore : IDisposable + { + #region Properties + + /// + /// Gets an handle to an HCERTSTORE store. + /// + IntPtr StoreHandle { get; } + + /// + /// Gets the location of the X.509 certificate store. + /// + StoreLocation Location { get; } + + /// + /// Gets the name of the X.509 certificate store. + /// + string? Name { get; } + + /// + /// Returns a collection of certificates located in an X.509 certificate store. + /// + X509Certificate2Collection Certificates { get; } + + /// + /// Gets a value that indicates whether the instance is connected to an open certificate store. + /// + bool IsOpen { get; } + + #endregion + + #region Methods + + /// + /// Opens an X.509 certificate store or creates a new store, depending on flag settings. + /// + /// A bitwise combination of enumeration values that specifies the way to open the X.509 certificate store. + /// The store cannot be opened as requested. + /// The caller does not have the required permission. + /// The store contains invalid values. + /// + /// Use this method to open an existing X.509 store. Note that you must have additional permissions, specified by + /// StorePermissionFlags, to enumerate the certificates in the store. You can create a new store + /// by passing a store name that does not exist to the class constructor, and then using any of the + /// flags except . + /// + void Open(OpenFlags flags); + + /// + /// Closes an X.509 certificate store. + /// + /// + /// This method releases all resources associated with the store. You should always + /// close an X.509 certificate store after use. + /// + void Close(); + + /// + /// Adds a certificate to an X.509 certificate store. + /// + /// The certificate to add. + /// is null. + /// The certificate could not be added to the store. + void Add(X509Certificate2 certificate); + + /// + /// Adds a collection of certificates to an X.509 certificate store. + /// + /// The collection of certificates to add. + /// is null. + /// The caller does not have the required permission. + /// + /// This method adds more than one certificate to an X.509 certificate store; if one certificate + /// addition fails, the operation is reverted and no certificates are added. + /// + void AddRange(X509Certificate2Collection certificates); + + /// + /// Removes a certificate from an X.509 certificate store. + /// + /// The certificate to remove. + /// is null. + /// The caller does not have the required permission. + void Remove(X509Certificate2 certificate); + + /// + /// Removes a range of certificates from an X.509 certificate store. + /// + /// A range of certificates to remove. + /// is null. + /// The caller does not have the required permission. + /// + /// This method removes more than one certificate from an X.509 certificate store; if one certificate + /// removal fails, the operation is reverted and no certificates are removed. + /// + void RemoveRange(X509Certificate2Collection certificates); + + #endregion + } +} diff --git a/AdvancedSystems.Security.Abstractions/IHashService.cs b/AdvancedSystems.Security.Abstractions/IHashService.cs index 1736b48..a2aa84d 100644 --- a/AdvancedSystems.Security.Abstractions/IHashService.cs +++ b/AdvancedSystems.Security.Abstractions/IHashService.cs @@ -3,7 +3,7 @@ namespace AdvancedSystems.Security.Abstractions { /// - /// Defines a methods for computing hash codes. + /// Defines a service for computing hash codes. /// public interface IHashService { diff --git a/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs b/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs index b9445e0..7554e6d 100644 --- a/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs +++ b/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs @@ -1,4 +1,9 @@ -using AdvancedSystems.Security.Abstractions; +using System; +using System.Linq; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; + +using AdvancedSystems.Security.Abstractions; using AdvancedSystems.Security.Options; using AdvancedSystems.Security.Services; @@ -17,11 +22,34 @@ public class CertificateFixture public Mock> Options { get; private set; } + public Mock Store { get; private set; } + public CertificateFixture() { this.Logger = new Mock>(); - this.Options = new Mock>(); - this.CertificateService = new CertificateService(this.Logger.Object, this.Options.Object); + this.Store = new Mock(); + this.CertificateService = new CertificateService(this.Logger.Object, this.Options.Object, this.Store.Object); + } + + #region Helper Methods + + public static X509Certificate2 CreateCertificate(string subjectName) + { + using var ecdsa = ECDsa.Create(); + var request = new CertificateRequest(subjectName, ecdsa, HashAlgorithmName.SHA256); + var certificate = request.CreateSelfSigned(DateTimeOffset.UtcNow, DateTimeOffset.UtcNow.AddHours(1)); + return certificate; + } + + public static X509Certificate2Collection CreateCertificateCollection(int length) + { + var certificates = Enumerable.Range(0, length) + .Select(_ => CreateCertificate("O=AdvancedSystems")) + .ToArray(); + + return new X509Certificate2Collection(certificates); } + + #endregion } diff --git a/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs b/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs index 6477eed..54a1e56 100644 --- a/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs +++ b/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs @@ -1,8 +1,11 @@ -using System.Security.Cryptography.X509Certificates; +using System.Linq; +using System.Security.Cryptography.X509Certificates; using AdvancedSystems.Security.Options; using AdvancedSystems.Security.Tests.Fixtures; +using Moq; + using Xunit; namespace AdvancedSystems.Security.Tests.Services; @@ -21,7 +24,19 @@ public CertificateServiceTests(CertificateFixture fixture) [Fact] public void TestGetStoreCertificate() { + // Arrange + var certificates = CertificateFixture.CreateCertificateCollection(3); + string thumbprint = certificates.Select(x => x.Thumbprint).First(); + this._fixture.Store.Setup(x => x.Certificates) + .Returns(certificates); + // Act + var certificate = this._fixture.CertificateService.GetStoreCertificate(thumbprint, StoreName.My, StoreLocation.CurrentUser); + + // Assert + Assert.NotNull(certificate); + Assert.Equal(thumbprint, certificate.Thumbprint); + this._fixture.Store.Verify(service => service.Open(It.Is(flags => flags == OpenFlags.ReadOnly))); } [Fact] @@ -31,6 +46,8 @@ public void TestGetStoreCertificate_NotFound() string thumbprint = "123456789"; var storeName = StoreName.My; var storeLocation = StoreLocation.CurrentUser; + this._fixture.Store.Setup(x => x.Certificates) + .Returns(new X509Certificate2Collection()); // Act var certificate = this._fixture.CertificateService.GetStoreCertificate(thumbprint, storeName, storeLocation); @@ -42,7 +59,28 @@ public void TestGetStoreCertificate_NotFound() [Fact] public void GetConfiguredCertificate() { + // Arrange + var certificates = CertificateFixture.CreateCertificateCollection(3); + var options = new CertificateOptions + { + Thumbprint = certificates.Select(x => x.Thumbprint).First(), + StoreName = StoreName.My, + StoreLocation = StoreLocation.CurrentUser + }; + + this._fixture.Options.Setup(x => x.Value) + .Returns(options); + + this._fixture.Store.Setup(x => x.Certificates) + .Returns(certificates); + // Act + var certificate = this._fixture.CertificateService.GetConfiguredCertificate(); + + // Assert + Assert.NotNull(certificate); + Assert.Equal(options.Thumbprint, certificate.Thumbprint); + this._fixture.Store.Verify(service => service.Open(It.Is(flags => flags == OpenFlags.ReadOnly))); } [Fact] @@ -59,6 +97,9 @@ public void GetConfiguredCertificate_NotFound() this._fixture.Options.Setup(x => x.Value) .Returns(options); + this._fixture.Store.Setup(x => x.Certificates) + .Returns(new X509Certificate2Collection()); + // Act var certificate = this._fixture.CertificateService.GetConfiguredCertificate(); diff --git a/AdvancedSystems.Security/DependencyInjection/CryptoServiceCollectionExtensions.cs b/AdvancedSystems.Security/DependencyInjection/CryptoServiceCollectionExtensions.cs index adba274..c951b96 100644 --- a/AdvancedSystems.Security/DependencyInjection/CryptoServiceCollectionExtensions.cs +++ b/AdvancedSystems.Security/DependencyInjection/CryptoServiceCollectionExtensions.cs @@ -1,8 +1,10 @@ using AdvancedSystems.Security.Abstractions; +using AdvancedSystems.Security.Options; using AdvancedSystems.Security.Services; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Options; namespace AdvancedSystems.Security.DependencyInjection; @@ -14,10 +16,26 @@ public static IServiceCollection AddHashService(this IServiceCollection services return services; } + public static IServiceCollection AddCertificateStore(this IServiceCollection services) + { + // TODO: Bind settings, and provide a ICertificateBuilder + services.AddOptions(); + + services.TryAdd(ServiceDescriptor.Singleton(serviceProvider => + { + var options = serviceProvider.GetRequiredService>().Value; + return new CertificateStore(options.StoreName, options.StoreLocation); + })); + + return services; + } + public static IServiceCollection AddCertificateService(this IServiceCollection services) { // TODO: Bind settings, and provide a ICertificateBuilder services.AddOptions(); + + services.AddCertificateStore(); services.TryAdd(ServiceDescriptor.Scoped()); return services; } diff --git a/AdvancedSystems.Security/Exceptions/CertificateNotFoundException.cs b/AdvancedSystems.Security/Exceptions/CertificateNotFoundException.cs deleted file mode 100644 index 3fdcbdb..0000000 --- a/AdvancedSystems.Security/Exceptions/CertificateNotFoundException.cs +++ /dev/null @@ -1,42 +0,0 @@ -using System; - -namespace AdvancedSystems.Security.Exceptions; - -/// -/// Represents errors that occur because a specified certificate could not be located. -/// -/// -/// This exception is typically thrown when an attempt to retrieve a certificate by its -/// identifier, such as a thumbprint or subject name, fails. It indicates that the required -/// certificate is not present in the specified certificate store or location. -/// -public class CertificateNotFoundException : Exception -{ - /// - /// Initializes a new instance of the class. - /// - public CertificateNotFoundException() - { - - } - - /// - /// Initializes a new instance of the class with a specified error . - /// - /// The error message that explains the reason for the exception. - public CertificateNotFoundException(string message) : base(message) - { - - } - - /// - /// Initializes a new instance of the class with a specified error - /// a reference to the exception that is the cause of this exception. - /// - /// The error message that explains the reason for the exception. - /// The exception that is the cause of the current exception, or a null reference if no inner exception is specified. - public CertificateNotFoundException(string message, Exception inner) : base(message, inner) - { - - } -} diff --git a/AdvancedSystems.Security/Cryptography/Certificate.cs b/AdvancedSystems.Security/Extensions/CertificateStore.cs similarity index 53% rename from AdvancedSystems.Security/Cryptography/Certificate.cs rename to AdvancedSystems.Security/Extensions/CertificateStore.cs index 22ca3e7..2b77d6f 100644 --- a/AdvancedSystems.Security/Cryptography/Certificate.cs +++ b/AdvancedSystems.Security/Extensions/CertificateStore.cs @@ -1,31 +1,31 @@ using System.Linq; using System.Security.Cryptography.X509Certificates; -using AdvancedSystems.Security.Exceptions; +using AdvancedSystems.Security.Abstractions; +using AdvancedSystems.Security.Abstractions.Exceptions; -namespace AdvancedSystems.Security.Cryptography; +namespace AdvancedSystems.Security.Extensions; /// /// Defines functions for interacting with X.509 certificates. /// /// -public static class Certificate +public static class CertificateStore { /// - /// Retrieves an X509 certificate from the specified store using the provided thumbprint. + /// Retrieves an X.509 certificate from the specified store using the provided thumbprint. /// - /// The name of the certificate store to search in, such as . - /// The location of the certificate store, such as or . + /// The type of the certificate store, which must implement the interface. + /// The certificate store from which to retrieve the certificate. /// The thumbprint of the certificate to locate. /// The object if the certificate is found. - /// Thrown when no valid certificate with the specified thumbprint is found in the store. - public static X509Certificate2 GetStoreCertificate(StoreName storeName, StoreLocation storeLocation, string thumbprint) + /// Thrown when no certificate with the specified thumbprint is found in the store. + public static X509Certificate2 GetCertificate(this T store, string thumbprint) where T : ICertificateStore { - using var store = new X509Store(storeName, storeLocation); store.Open(OpenFlags.ReadOnly); var certificate = store.Certificates - .Find(X509FindType.FindByThumbprint, thumbprint, validOnly: true) + .Find(X509FindType.FindByThumbprint, thumbprint, validOnly: false) .OfType() .FirstOrDefault(); diff --git a/AdvancedSystems.Security/Services/CertificateService.cs b/AdvancedSystems.Security/Services/CertificateService.cs index b37d8ac..fc8a3ba 100644 --- a/AdvancedSystems.Security/Services/CertificateService.cs +++ b/AdvancedSystems.Security/Services/CertificateService.cs @@ -1,8 +1,8 @@ using System.Security.Cryptography.X509Certificates; using AdvancedSystems.Security.Abstractions; -using AdvancedSystems.Security.Cryptography; -using AdvancedSystems.Security.Exceptions; +using AdvancedSystems.Security.Abstractions.Exceptions; +using AdvancedSystems.Security.Extensions; using AdvancedSystems.Security.Options; using Microsoft.Extensions.Logging; @@ -16,11 +16,13 @@ public sealed class CertificateService : ICertificateService { private readonly ILogger _logger; private readonly IOptions _options; + private readonly ICertificateStore _certificateStore; - public CertificateService(ILogger logger, IOptions options) + public CertificateService(ILogger logger, IOptions options, ICertificateStore certificateStore) { this._logger = logger; this._options = options; + this._certificateStore = certificateStore; } #region Public Methods @@ -30,7 +32,7 @@ public CertificateService(ILogger logger, IOptions this._logger.LogError(exception, "{Service} failed to retrieve certificate.", nameof(CertificateService)))) { diff --git a/AdvancedSystems.Security/Services/CertificateStore.cs b/AdvancedSystems.Security/Services/CertificateStore.cs new file mode 100644 index 0000000..145df8e --- /dev/null +++ b/AdvancedSystems.Security/Services/CertificateStore.cs @@ -0,0 +1,108 @@ +using System; +using System.Security.Cryptography.X509Certificates; + +using AdvancedSystems.Security.Abstractions; + +namespace AdvancedSystems.Security.Services; + +/// +/// Implements X.509 store, which is a physical store where certificates are persisted and managed. +/// This class cannot be inherited. +/// +public sealed class CertificateStore : ICertificateStore +{ + private readonly X509Store _store; + + public CertificateStore(StoreName storeName, StoreLocation storeLocation) + { + this._store = new X509Store(storeName, storeLocation); + } + + public static implicit operator X509Store(CertificateStore storeService) + { + return new X509Store(storeService.Name!, storeService.Location); + } + + #region Properties + + public IntPtr StoreHandle + { + get + { + return this._store.StoreHandle; + } + } + + public StoreLocation Location + { + get + { + return this._store.Location; + } + } + + public string? Name + { + get + { + return this._store.Name; + } + } + + public X509Certificate2Collection Certificates + { + get + { + return this._store.Certificates; + } + } + + public bool IsOpen + { + get + { + return this._store.IsOpen; + } + } + + #endregion + + #region Methods + + public void Open(OpenFlags flags) + { + this._store.Open(flags); + } + + public void Dispose() + { + this._store?.Dispose(); + } + + public void Close() + { + this._store?.Close(); + } + + public void Add(X509Certificate2 certificate) + { + this._store.Add(certificate); + } + + public void AddRange(X509Certificate2Collection certificates) + { + this._store.AddRange(certificates); + } + + public void Remove(X509Certificate2 certificate) + { + this._store.Remove(certificate); + } + + public void RemoveRange(X509Certificate2Collection certificates) + { + this._store.RemoveRange(certificates); + } + + #endregion +} From 2f6dabed3185844e58f006743eacbdbcf9c1fa2b Mon Sep 17 00:00:00 2001 From: StefanGreve Date: Thu, 11 Jul 2024 18:14:48 +0200 Subject: [PATCH 4/7] Implement DI methods and increase unit test coverage --- .github/workflows/dotnet-tests.yml | 5 +- .../AdvancedSystems.Security.Tests.csproj | 15 ++- .../Fixtures/CertificateFixture.cs | 18 ++- .../Fixtures/CertificateStoreFixture.cs | 19 +++ .../Fixtures/HashFixture.cs | 12 +- .../Services/CertificateServiceTests.cs | 127 ++++++++++++++---- .../Services/CertificateStore.cs | 89 ++++++++++++ .../Services/HashServiceTests.cs | 55 ++++++-- .../appsettings.json | 9 ++ .../AdvancedSystems.Security.csproj | 5 +- .../CryptoServiceCollectionExtensions.cs | 54 -------- .../ServiceCollectionExtensions.cs | 115 ++++++++++++++++ .../Options/CertificateOptions.cs | 13 +- .../Options/CertificateStoreOptions.cs | 15 +++ .../Options/RSACryptoOptions.cs | 2 +- AdvancedSystems.Security/Options/Sections.cs | 2 + .../Services/CertificateService.cs | 10 +- .../Services/RSACryptoService.cs | 9 +- 18 files changed, 448 insertions(+), 126 deletions(-) create mode 100644 AdvancedSystems.Security.Tests/Fixtures/CertificateStoreFixture.cs create mode 100644 AdvancedSystems.Security.Tests/Services/CertificateStore.cs create mode 100644 AdvancedSystems.Security.Tests/appsettings.json delete mode 100644 AdvancedSystems.Security/DependencyInjection/CryptoServiceCollectionExtensions.cs create mode 100644 AdvancedSystems.Security/DependencyInjection/ServiceCollectionExtensions.cs create mode 100644 AdvancedSystems.Security/Options/CertificateStoreOptions.cs diff --git a/.github/workflows/dotnet-tests.yml b/.github/workflows/dotnet-tests.yml index 5a7e4e5..eb0ed7b 100644 --- a/.github/workflows/dotnet-tests.yml +++ b/.github/workflows/dotnet-tests.yml @@ -1,7 +1,4 @@ -# This workflow will build a .NET project -# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-net - -name: .NET +name: "Unit Tests" on: push: diff --git a/AdvancedSystems.Security.Tests/AdvancedSystems.Security.Tests.csproj b/AdvancedSystems.Security.Tests/AdvancedSystems.Security.Tests.csproj index 7d746a6..ad997c4 100644 --- a/AdvancedSystems.Security.Tests/AdvancedSystems.Security.Tests.csproj +++ b/AdvancedSystems.Security.Tests/AdvancedSystems.Security.Tests.csproj @@ -14,18 +14,19 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive + - - + + all runtime; build; native; contentfiles; analyzers; buildtransitive - + all runtime; build; native; contentfiles; analyzers; buildtransitive - + all runtime; build; native; contentfiles; analyzers; buildtransitive @@ -36,4 +37,10 @@ + + + Always + + + diff --git a/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs b/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs index 7554e6d..75389b0 100644 --- a/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs +++ b/AdvancedSystems.Security.Tests/Fixtures/CertificateFixture.cs @@ -16,6 +16,16 @@ namespace AdvancedSystems.Security.Tests.Fixtures; public class CertificateFixture { + public CertificateFixture() + { + this.Logger = new Mock>(); + this.Options = new Mock>(); + this.Store = new Mock(); + this.CertificateService = new CertificateService(this.Logger.Object, this.Options.Object, this.Store.Object); + } + + #region Properties + public Mock> Logger { get; private set; } public ICertificateService CertificateService { get; private set; } @@ -24,13 +34,7 @@ public class CertificateFixture public Mock Store { get; private set; } - public CertificateFixture() - { - this.Logger = new Mock>(); - this.Options = new Mock>(); - this.Store = new Mock(); - this.CertificateService = new CertificateService(this.Logger.Object, this.Options.Object, this.Store.Object); - } + #endregion #region Helper Methods diff --git a/AdvancedSystems.Security.Tests/Fixtures/CertificateStoreFixture.cs b/AdvancedSystems.Security.Tests/Fixtures/CertificateStoreFixture.cs new file mode 100644 index 0000000..0a53936 --- /dev/null +++ b/AdvancedSystems.Security.Tests/Fixtures/CertificateStoreFixture.cs @@ -0,0 +1,19 @@ +using System.Security.Cryptography.X509Certificates; + +using AdvancedSystems.Security.Services; + +namespace AdvancedSystems.Security.Tests.Fixtures; + +public class CertificateStoreFixture +{ + public CertificateStoreFixture() + { + this.CertificateStore = new CertificateStore(StoreName.My, StoreLocation.CurrentUser); + } + + #region Properties + + public CertificateStore CertificateStore { get; set; } + + #endregion +} diff --git a/AdvancedSystems.Security.Tests/Fixtures/HashFixture.cs b/AdvancedSystems.Security.Tests/Fixtures/HashFixture.cs index f570feb..f2cdb82 100644 --- a/AdvancedSystems.Security.Tests/Fixtures/HashFixture.cs +++ b/AdvancedSystems.Security.Tests/Fixtures/HashFixture.cs @@ -9,13 +9,17 @@ namespace AdvancedSystems.Security.Tests.Fixtures; public class HashServiceFixture { - public Mock> Logger { get; private set; } - - public IHashService HashService { get; private set; } - public HashServiceFixture() { this.Logger = new Mock>(); this.HashService = new HashService(this.Logger.Object); } + + #region Properties + + public Mock> Logger { get; private set; } + + public IHashService HashService { get; private set; } + + #endregion } diff --git a/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs b/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs index 54a1e56..21c1536 100644 --- a/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs +++ b/AdvancedSystems.Security.Tests/Services/CertificateServiceTests.cs @@ -1,9 +1,18 @@ using System.Linq; using System.Security.Cryptography.X509Certificates; +using System.Threading.Tasks; +using AdvancedSystems.Security.Abstractions; +using AdvancedSystems.Security.DependencyInjection; using AdvancedSystems.Security.Options; using AdvancedSystems.Security.Tests.Fixtures; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; + using Moq; using Xunit; @@ -12,11 +21,11 @@ namespace AdvancedSystems.Security.Tests.Services; public class CertificateServiceTests : IClassFixture { - private readonly CertificateFixture _fixture; + private readonly CertificateFixture _sut; public CertificateServiceTests(CertificateFixture fixture) { - this._fixture = fixture; + this._sut = fixture; } #region Tests @@ -27,16 +36,16 @@ public void TestGetStoreCertificate() // Arrange var certificates = CertificateFixture.CreateCertificateCollection(3); string thumbprint = certificates.Select(x => x.Thumbprint).First(); - this._fixture.Store.Setup(x => x.Certificates) + this._sut.Store.Setup(x => x.Certificates) .Returns(certificates); // Act - var certificate = this._fixture.CertificateService.GetStoreCertificate(thumbprint, StoreName.My, StoreLocation.CurrentUser); + var certificate = this._sut.CertificateService.GetStoreCertificate(thumbprint, StoreName.My, StoreLocation.CurrentUser); // Assert Assert.NotNull(certificate); Assert.Equal(thumbprint, certificate.Thumbprint); - this._fixture.Store.Verify(service => service.Open(It.Is(flags => flags == OpenFlags.ReadOnly))); + this._sut.Store.Verify(service => service.Open(It.Is(flags => flags == OpenFlags.ReadOnly))); } [Fact] @@ -46,11 +55,11 @@ public void TestGetStoreCertificate_NotFound() string thumbprint = "123456789"; var storeName = StoreName.My; var storeLocation = StoreLocation.CurrentUser; - this._fixture.Store.Setup(x => x.Certificates) + this._sut.Store.Setup(x => x.Certificates) .Returns(new X509Certificate2Collection()); // Act - var certificate = this._fixture.CertificateService.GetStoreCertificate(thumbprint, storeName, storeLocation); + var certificate = this._sut.CertificateService.GetStoreCertificate(thumbprint, storeName, storeLocation); // Assert Assert.Null(certificate); @@ -61,50 +70,122 @@ public void GetConfiguredCertificate() { // Arrange var certificates = CertificateFixture.CreateCertificateCollection(3); - var options = new CertificateOptions + var certificateOptions = new CertificateOptions { Thumbprint = certificates.Select(x => x.Thumbprint).First(), - StoreName = StoreName.My, - StoreLocation = StoreLocation.CurrentUser + Store = new CertificateStoreOptions + { + Location = StoreLocation.CurrentUser, + Name = StoreName.My, + } }; - this._fixture.Options.Setup(x => x.Value) - .Returns(options); + this._sut.Options.Setup(x => x.Value) + .Returns(certificateOptions); - this._fixture.Store.Setup(x => x.Certificates) + this._sut.Store.Setup(x => x.Certificates) .Returns(certificates); // Act - var certificate = this._fixture.CertificateService.GetConfiguredCertificate(); + var certificate = this._sut.CertificateService.GetConfiguredCertificate(); // Assert Assert.NotNull(certificate); - Assert.Equal(options.Thumbprint, certificate.Thumbprint); - this._fixture.Store.Verify(service => service.Open(It.Is(flags => flags == OpenFlags.ReadOnly))); + Assert.Equal(certificateOptions.Thumbprint, certificate.Thumbprint); + this._sut.Store.Verify(service => service.Open(It.Is(flags => flags == OpenFlags.ReadOnly))); } [Fact] public void GetConfiguredCertificate_NotFound() { // Arrange - var options = new CertificateOptions + var certificateOptions = new CertificateOptions { Thumbprint = "123456789", - StoreName = StoreName.My, - StoreLocation = StoreLocation.CurrentUser + Store = new CertificateStoreOptions + { + Location = StoreLocation.CurrentUser, + Name = StoreName.My, + } }; - this._fixture.Options.Setup(x => x.Value) - .Returns(options); + this._sut.Options.Setup(x => x.Value) + .Returns(certificateOptions); - this._fixture.Store.Setup(x => x.Certificates) + this._sut.Store.Setup(x => x.Certificates) .Returns(new X509Certificate2Collection()); // Act - var certificate = this._fixture.CertificateService.GetConfiguredCertificate(); + var certificate = this._sut.CertificateService.GetConfiguredCertificate(); + + // Assert + Assert.Null(certificate); + } + + [Fact] + public async Task TestAddCertificateService_FromOptions() + { + // Arrange + using var hostBuilder = await new HostBuilder() + .ConfigureWebHost(builder => builder + .UseTestServer() + .ConfigureServices(services => + { + services.AddCertificateService(options => + { + options.Thumbprint = "123456789"; + options.Store = new CertificateStoreOptions + { + Location = StoreLocation.CurrentUser, + Name = StoreName.My, + }; + }); + }) + .Configure(app => + { + + })) + .StartAsync(); + + // Act + var certificateService = hostBuilder.Services.GetService(); + var certificate = certificateService?.GetConfiguredCertificate(); + + // Assert + Assert.NotNull(certificateService); + Assert.Null(certificate); + await hostBuilder.StopAsync(); + } + + [Fact] + public async Task TestAddCertificateService_FromAppSettings() + { + // Arrange + using var hostBuilder = await new HostBuilder() + .ConfigureWebHost(builder => builder + .UseTestServer() + .ConfigureAppConfiguration(config => + { + config.AddJsonFile("appsettings.json", optional: false); + }) + .ConfigureServices((context, services )=> + { + services.AddCertificateService(context.Configuration); + }) + .Configure(app => + { + + })) + .StartAsync(); + + // Act + var certificateService = hostBuilder.Services.GetService(); + var certificate = certificateService?.GetConfiguredCertificate(); // Assert + Assert.NotNull(certificateService); Assert.Null(certificate); + await hostBuilder.StopAsync(); } #endregion diff --git a/AdvancedSystems.Security.Tests/Services/CertificateStore.cs b/AdvancedSystems.Security.Tests/Services/CertificateStore.cs new file mode 100644 index 0000000..78106f6 --- /dev/null +++ b/AdvancedSystems.Security.Tests/Services/CertificateStore.cs @@ -0,0 +1,89 @@ +using System.Security.Cryptography.X509Certificates; +using System.Threading.Tasks; + +using AdvancedSystems.Security.Abstractions; +using AdvancedSystems.Security.DependencyInjection; +using AdvancedSystems.Security.Tests.Fixtures; + +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; + +using Xunit; + +namespace AdvancedSystems.Security.Tests.Services; + +public class CertificateStore : IClassFixture +{ + private readonly CertificateStoreFixture _sut; + + public CertificateStore(CertificateStoreFixture fixture) + { + this._sut = fixture; + } + + #region Tests + + [Fact] + public async Task TestAddCertificateStore_FromOptions() + { + // Arrange + using var hostBuilder = await new HostBuilder() + .ConfigureWebHost(builder => builder + .UseTestServer() + .ConfigureServices(services => + { + services.AddCertificateStore(options => + { + options.Name = StoreName.My; + options.Location = StoreLocation.CurrentUser; + }); + }) + .Configure(app => + { + + })) + .StartAsync(); + + // Act + var certificateStore = hostBuilder.Services.GetService(); + + // Assert + Assert.NotNull(certificateStore); + await hostBuilder.StopAsync(); + } + + [Fact] + public async Task TestAddCertificateStore_FromAppSettings() + { + // Arrange + using var hostBuilder = await new HostBuilder() + .ConfigureWebHost(builder => builder + .UseTestServer() + .ConfigureAppConfiguration(config => + { + config.AddJsonFile("appsettings.json", optional: false); + }) + .ConfigureServices((context, services) => + { + + services.AddCertificateStore(context.Configuration); + }) + .Configure(app => + { + + })) + .StartAsync(); + + // Act + var certificateStore = hostBuilder.Services.GetService(); + + // Assert + Assert.NotNull(certificateStore); + await hostBuilder.StopAsync(); + } + + #endregion +} diff --git a/AdvancedSystems.Security.Tests/Services/HashServiceTests.cs b/AdvancedSystems.Security.Tests/Services/HashServiceTests.cs index 360c04e..d6b0cc7 100644 --- a/AdvancedSystems.Security.Tests/Services/HashServiceTests.cs +++ b/AdvancedSystems.Security.Tests/Services/HashServiceTests.cs @@ -1,8 +1,15 @@ using System; using System.Text; +using System.Threading.Tasks; +using AdvancedSystems.Security.Abstractions; +using AdvancedSystems.Security.DependencyInjection; using AdvancedSystems.Security.Tests.Fixtures; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Moq; @@ -13,11 +20,11 @@ namespace AdvancedSystems.Security.Tests.Services; public class HashServiceTests : IClassFixture { - private readonly HashServiceFixture _fixture; + private readonly HashServiceFixture _sut; public HashServiceTests(HashServiceFixture fixture) { - this._fixture = fixture; + this._sut = fixture; } #region Tests @@ -31,12 +38,12 @@ public void TestMD5Hash() // Act #pragma warning disable CS0618 // Type or member is obsolete - string md5 = this._fixture.HashService.GetMD5Hash(buffer); + string md5 = this._sut.HashService.GetMD5Hash(buffer); #pragma warning restore CS0618 // Type or member is obsolete // Assert Assert.Equal("9e107d9d372bb6826bd81d3542a419d6", md5); - this._fixture.Logger.Verify(x => x.Log( + this._sut.Logger.Verify(x => x.Log( LogLevel.Warning, It.IsAny(), It.Is((v, t) => v.ToString()!.StartsWith("Computing hash with a cryptographically insecure hash algorithm")), @@ -54,12 +61,12 @@ public void TestSHA1Hash() // Act #pragma warning disable CS0618 // Type or member is obsolete - string sha1 = this._fixture.HashService.GetSHA1Hash(buffer); + string sha1 = this._sut.HashService.GetSHA1Hash(buffer); #pragma warning restore CS0618 // Type or member is obsolete // Assert Assert.Equal("2fd4e1c67a2d28fced849ee1bb76e7391b93eb12", sha1); - this._fixture.Logger.Verify(x => x.Log( + this._sut.Logger.Verify(x => x.Log( LogLevel.Warning, It.IsAny(), It.Is((v, t) => v.ToString()!.StartsWith("Computing hash with a cryptographically insecure hash algorithm")), @@ -76,7 +83,7 @@ public void TestSHA256Hash() byte[] buffer = Encoding.UTF8.GetBytes(input); // Act - string sha256 = this._fixture.HashService.GetSHA256Hash(buffer); + string sha256 = this._sut.HashService.GetSHA256Hash(buffer); // Assert Assert.Equal("d7a8fbb307d7809469ca9abcb0082e4f8d5651e46d3cdb762d02d0bf37c9e592", sha256); @@ -90,7 +97,7 @@ public void TestSHA384Hash() byte[] buffer = Encoding.UTF8.GetBytes(input); // Act - string sha384 = this._fixture.HashService.GetSHA384Hash(buffer); + string sha384 = this._sut.HashService.GetSHA384Hash(buffer); // Assert Assert.Equal("ca737f1014a48f4c0b6dd43cb177b0afd9e5169367544c494011e3317dbf9a509cb1e5dc1e85a941bbee3d7f2afbc9b1", sha384); @@ -104,11 +111,41 @@ public void TestSHA512Hash() byte[] buffer = Encoding.UTF8.GetBytes(input); // Act - string sha512 = this._fixture.HashService.GetSHA512Hash(buffer); + string sha512 = this._sut.HashService.GetSHA512Hash(buffer); // Assert Assert.Equal("07e547d9586f6a73f73fbac0435ed76951218fb7d0c8d788a309d785436bbb642e93a252a954f23912547d1e8a3b5ed6e1bfd7097821233fa0538f3db854fee6", sha512); } + [Fact] + public async Task TestAddHashService() + { + // Arrange + using var hostBuilder = await new HostBuilder() + .ConfigureWebHost(builder => builder + .UseTestServer() + .ConfigureServices(services => + { + services.AddHashService(); + }) + .Configure(app => + { + + })) + .StartAsync(); + + string input = "The quick brown fox jumps over the lazy dog"; + byte[] buffer = Encoding.UTF8.GetBytes(input); + + // Act + var hashService = hostBuilder.Services.GetService(); + string? sha256 = hashService?.GetSHA256Hash(buffer); + + // Assert + Assert.NotNull(hashService); + Assert.Equal("d7a8fbb307d7809469ca9abcb0082e4f8d5651e46d3cdb762d02d0bf37c9e592", sha256); + await hostBuilder.StopAsync(); + } + #endregion } diff --git a/AdvancedSystems.Security.Tests/appsettings.json b/AdvancedSystems.Security.Tests/appsettings.json new file mode 100644 index 0000000..bd97412 --- /dev/null +++ b/AdvancedSystems.Security.Tests/appsettings.json @@ -0,0 +1,9 @@ +{ + "Certificate": { + "Thumbprint": "123456789", + "Store": { + "Location": "CurrentUser", + "Name": "My" + } + } +} \ No newline at end of file diff --git a/AdvancedSystems.Security/AdvancedSystems.Security.csproj b/AdvancedSystems.Security/AdvancedSystems.Security.csproj index f20d0c5..b693231 100644 --- a/AdvancedSystems.Security/AdvancedSystems.Security.csproj +++ b/AdvancedSystems.Security/AdvancedSystems.Security.csproj @@ -6,9 +6,12 @@ + - + + + diff --git a/AdvancedSystems.Security/DependencyInjection/CryptoServiceCollectionExtensions.cs b/AdvancedSystems.Security/DependencyInjection/CryptoServiceCollectionExtensions.cs deleted file mode 100644 index c951b96..0000000 --- a/AdvancedSystems.Security/DependencyInjection/CryptoServiceCollectionExtensions.cs +++ /dev/null @@ -1,54 +0,0 @@ -using AdvancedSystems.Security.Abstractions; -using AdvancedSystems.Security.Options; -using AdvancedSystems.Security.Services; - -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.DependencyInjection.Extensions; -using Microsoft.Extensions.Options; - -namespace AdvancedSystems.Security.DependencyInjection; - -public static class CryptoServiceCollectionExtensions -{ - public static IServiceCollection AddHashService(this IServiceCollection services) - { - services.TryAdd(ServiceDescriptor.Scoped()); - return services; - } - - public static IServiceCollection AddCertificateStore(this IServiceCollection services) - { - // TODO: Bind settings, and provide a ICertificateBuilder - services.AddOptions(); - - services.TryAdd(ServiceDescriptor.Singleton(serviceProvider => - { - var options = serviceProvider.GetRequiredService>().Value; - return new CertificateStore(options.StoreName, options.StoreLocation); - })); - - return services; - } - - public static IServiceCollection AddCertificateService(this IServiceCollection services) - { - // TODO: Bind settings, and provide a ICertificateBuilder - services.AddOptions(); - - services.AddCertificateStore(); - services.TryAdd(ServiceDescriptor.Scoped()); - return services; - } - - public static IServiceCollection AddRSACryptoService(this IServiceCollection services) - { - // Register services required by RSACryptoService - services.AddCertificateService(); - - // TODO: Bind settings, and provide a IRSACryptoBuilder - services.AddOptions(); - - services.TryAdd(ServiceDescriptor.Singleton()); - return services; - } -} diff --git a/AdvancedSystems.Security/DependencyInjection/ServiceCollectionExtensions.cs b/AdvancedSystems.Security/DependencyInjection/ServiceCollectionExtensions.cs new file mode 100644 index 0000000..35542a1 --- /dev/null +++ b/AdvancedSystems.Security/DependencyInjection/ServiceCollectionExtensions.cs @@ -0,0 +1,115 @@ +using System; + +using AdvancedSystems.Security.Abstractions; +using AdvancedSystems.Security.Options; +using AdvancedSystems.Security.Services; + +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Options; + +namespace AdvancedSystems.Security.DependencyInjection; + +public static class ServiceCollectionExtensions +{ + #region HashService + + public static IServiceCollection AddHashService(this IServiceCollection services) + { + services.TryAdd(ServiceDescriptor.Scoped()); + return services; + } + + #endregion + + #region CertificateStore + + internal static void AddCertificateStore(this IServiceCollection services) where TOptions : class + { + services.TryAdd(ServiceDescriptor.Singleton(serviceProvider => + { + var options = serviceProvider.GetRequiredService>().Value switch + { + CertificateOptions certificateOptions => new { certificateOptions.Store.Name, certificateOptions.Store.Location }, + CertificateStoreOptions storeOptions => new { storeOptions.Name, storeOptions.Location }, + _ => throw new NotImplementedException() + }; + + return new CertificateStore(options.Name, options.Location); + })); + } + + public static IServiceCollection AddCertificateStore(this IServiceCollection services, Action setupAction) + { + services.AddOptions() + .Configure(setupAction); + + services.AddCertificateStore(); + return services; + } + + public static IServiceCollection AddCertificateStore(this IServiceCollection services, IConfiguration configuration) + { + services.AddOptions() + .Bind(configuration.GetRequiredSection(Sections.CERTIFICATE_STORE)) + .ValidateDataAnnotations() + .ValidateOnStart(); + + services.AddCertificateStore(); + return services; + } + + #endregion + + #region CertificateService + + internal static void AddCertificateService(this IServiceCollection services) + { + services.TryAdd(ServiceDescriptor.Scoped()); + } + + public static IServiceCollection AddCertificateService(this IServiceCollection services, Action setupAction) + { + services.AddOptions() + .Configure(setupAction); + + services.AddCertificateStore(); + services.AddCertificateService(); + + return services; + } + + public static IServiceCollection AddCertificateService(this IServiceCollection services, IConfiguration configuration) + { + services.AddOptions() + .Bind(configuration.GetRequiredSection(Sections.CERTIFICATE)) + .ValidateDataAnnotations() + .ValidateOnStart(); + + services.AddCertificateStore(configuration); + services.AddCertificateService(); + + return services; + } + + #endregion + + #region RSACryptoService + + internal static IServiceCollection AddRSACryptoService(this IServiceCollection services) + { + throw new NotImplementedException(); + } + public static IServiceCollection AddRSACryptoService(this IServiceCollection services, Action setupAction) + { + throw new NotImplementedException(); + } + + public static IServiceCollection AddRSACryptoService(this IServiceCollection services, IConfiguration configuration) + { + throw new NotImplementedException(); + } + + #endregion +} diff --git a/AdvancedSystems.Security/Options/CertificateOptions.cs b/AdvancedSystems.Security/Options/CertificateOptions.cs index 94c38f3..499149d 100644 --- a/AdvancedSystems.Security/Options/CertificateOptions.cs +++ b/AdvancedSystems.Security/Options/CertificateOptions.cs @@ -1,16 +1,13 @@ using System.ComponentModel.DataAnnotations; -using System.Security.Cryptography.X509Certificates; namespace AdvancedSystems.Security.Options; -public record CertificateOptions +public sealed class CertificateOptions { [Key] - public required string Thumbprint { get; init; } + [Required(AllowEmptyStrings = false)] + public required string Thumbprint { get; set; } - [EnumDataType(typeof(StoreName))] - public required StoreName StoreName { get; init; } - - [EnumDataType(typeof(StoreLocation))] - public required StoreLocation StoreLocation { get; init; } + [Required] + public required CertificateStoreOptions Store { get; set; } } diff --git a/AdvancedSystems.Security/Options/CertificateStoreOptions.cs b/AdvancedSystems.Security/Options/CertificateStoreOptions.cs new file mode 100644 index 0000000..551ab74 --- /dev/null +++ b/AdvancedSystems.Security/Options/CertificateStoreOptions.cs @@ -0,0 +1,15 @@ +using System.ComponentModel.DataAnnotations; +using System.Security.Cryptography.X509Certificates; + +namespace AdvancedSystems.Security.Options; + +public sealed class CertificateStoreOptions +{ + [Required] + [EnumDataType(typeof(StoreName))] + public required StoreName Name { get; set; } + + [Required] + [EnumDataType(typeof(StoreLocation))] + public required StoreLocation Location { get; set; } +} diff --git a/AdvancedSystems.Security/Options/RSACryptoOptions.cs b/AdvancedSystems.Security/Options/RSACryptoOptions.cs index a41585c..0abea89 100644 --- a/AdvancedSystems.Security/Options/RSACryptoOptions.cs +++ b/AdvancedSystems.Security/Options/RSACryptoOptions.cs @@ -3,7 +3,7 @@ namespace AdvancedSystems.Security.Options; -public record RSACryptoOptions +public sealed class RSACryptoOptions { public required HashAlgorithmName HashAlgorithmName { get; init; } diff --git a/AdvancedSystems.Security/Options/Sections.cs b/AdvancedSystems.Security/Options/Sections.cs index 48062a5..8409b23 100644 --- a/AdvancedSystems.Security/Options/Sections.cs +++ b/AdvancedSystems.Security/Options/Sections.cs @@ -2,6 +2,8 @@ public readonly record struct Sections { + public const string CERTIFICATE_STORE = "Certificate:Store"; + public const string CERTIFICATE = "Certificate"; public const string RSA = "RSA"; diff --git a/AdvancedSystems.Security/Services/CertificateService.cs b/AdvancedSystems.Security/Services/CertificateService.cs index fc8a3ba..87e83ef 100644 --- a/AdvancedSystems.Security/Services/CertificateService.cs +++ b/AdvancedSystems.Security/Services/CertificateService.cs @@ -15,13 +15,13 @@ namespace AdvancedSystems.Security.Services; public sealed class CertificateService : ICertificateService { private readonly ILogger _logger; - private readonly IOptions _options; + private readonly IOptions _certificateOptions; private readonly ICertificateStore _certificateStore; - public CertificateService(ILogger logger, IOptions options, ICertificateStore certificateStore) + public CertificateService(ILogger logger, IOptions certificateOptions, ICertificateStore certificateStore) { this._logger = logger; - this._options = options; + this._certificateOptions = certificateOptions; this._certificateStore = certificateStore; } @@ -42,8 +42,8 @@ public CertificateService(ILogger logger, IOptions Date: Thu, 11 Jul 2024 18:30:19 +0200 Subject: [PATCH 5/7] Move ObjectSerializer to Core project --- .../Common/ObjectSerializer.cs | 38 ------------------- 1 file changed, 38 deletions(-) delete mode 100644 AdvancedSystems.Security/Common/ObjectSerializer.cs diff --git a/AdvancedSystems.Security/Common/ObjectSerializer.cs b/AdvancedSystems.Security/Common/ObjectSerializer.cs deleted file mode 100644 index 1c2d48b..0000000 --- a/AdvancedSystems.Security/Common/ObjectSerializer.cs +++ /dev/null @@ -1,38 +0,0 @@ -using System; -using System.Buffers; -using System.Text.Json; -using System.Text.Json.Serialization; - -namespace AdvancedSystems.Security.Common; - -public static class ObjectSerializer -{ - /// - /// Converts the provided value into a . - /// - /// The type of the value to serialize. - /// The to convert and write. - /// A representation of the . - /// There is no compatible for or its serializable members. - public static ReadOnlySpan Serialize(T value) where T : class, new() - { - var buffer = new ArrayBufferWriter(); - using var writer = new Utf8JsonWriter(buffer); - JsonSerializer.Serialize(writer, value); - return buffer.WrittenSpan; - } - - /// - /// Parses the text representing a single JSON value into a . - /// - /// The type to deserialize the JSON value into. - /// JSON text to parse. - /// A representation of the JSON value. - /// The JSON is invalid, is not compatible with the JSON, or there is remaining data in the Stream. - /// There is no compatible for or its serializable members. - public static T Deserialize(ReadOnlySpan buffer) where T : class, new() - { - var payload = new Utf8JsonReader(buffer); - return JsonSerializer.Deserialize(ref payload)!; - } -} From 3fa5aa46840d0cb66957dfd7aab7cb1f03fc58ed Mon Sep 17 00:00:00 2001 From: StefanGreve Date: Thu, 11 Jul 2024 18:30:57 +0200 Subject: [PATCH 6/7] Turn RSACryptoOptions into a sealed mutable class --- AdvancedSystems.Security/Options/RSACryptoOptions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/AdvancedSystems.Security/Options/RSACryptoOptions.cs b/AdvancedSystems.Security/Options/RSACryptoOptions.cs index 0abea89..6119e1b 100644 --- a/AdvancedSystems.Security/Options/RSACryptoOptions.cs +++ b/AdvancedSystems.Security/Options/RSACryptoOptions.cs @@ -5,11 +5,11 @@ namespace AdvancedSystems.Security.Options; public sealed class RSACryptoOptions { - public required HashAlgorithmName HashAlgorithmName { get; init; } + public required HashAlgorithmName HashAlgorithmName { get; set; } - public required RSAEncryptionPadding EncryptionPadding { get; init; } + public required RSAEncryptionPadding EncryptionPadding { get; set; } - public required RSASignaturePadding SignaturePadding { get; init; } + public required RSASignaturePadding SignaturePadding { get; set; } - public required Encoding Encoding { get; init; } + public required Encoding Encoding { get; set; } } From c8728c5a93701673a267b89e883987511e18ef32 Mon Sep 17 00:00:00 2001 From: StefanGreve Date: Fri, 12 Jul 2024 20:58:21 +0200 Subject: [PATCH 7/7] Apply PR feedback --- .../AdvancedSystems.Security.Tests.csproj | 2 +- AdvancedSystems.Security/Services/CertificateStore.cs | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/AdvancedSystems.Security.Tests/AdvancedSystems.Security.Tests.csproj b/AdvancedSystems.Security.Tests/AdvancedSystems.Security.Tests.csproj index ad997c4..8a8f61e 100644 --- a/AdvancedSystems.Security.Tests/AdvancedSystems.Security.Tests.csproj +++ b/AdvancedSystems.Security.Tests/AdvancedSystems.Security.Tests.csproj @@ -39,7 +39,7 @@ - Always + PreserveNewest diff --git a/AdvancedSystems.Security/Services/CertificateStore.cs b/AdvancedSystems.Security/Services/CertificateStore.cs index 145df8e..df6bc76 100644 --- a/AdvancedSystems.Security/Services/CertificateStore.cs +++ b/AdvancedSystems.Security/Services/CertificateStore.cs @@ -18,11 +18,6 @@ public CertificateStore(StoreName storeName, StoreLocation storeLocation) this._store = new X509Store(storeName, storeLocation); } - public static implicit operator X509Store(CertificateStore storeService) - { - return new X509Store(storeService.Name!, storeService.Location); - } - #region Properties public IntPtr StoreHandle