diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index e5fba1e4108a..c169d1a17047 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -812,6 +812,50 @@ internal static string ToCertificateDescription(IEnumerable ce internal static string GetDescription(X509Certificate2 c) => $"{c.Thumbprint} - {c.Subject} - Valid from {c.NotBefore:u} to {c.NotAfter:u} - IsHttpsDevelopmentCertificate: {IsHttpsDevelopmentCertificate(c).ToString().ToLowerInvariant()} - IsExportable: {Instance.IsExportable(c).ToString().ToLowerInvariant()}"; + /// + /// is not adequate for security purposes. + /// + internal static bool AreCertificatesEqual(X509Certificate2 cert1, X509Certificate2 cert2) + { + return cert1.RawDataMemory.Span.SequenceEqual(cert2.RawDataMemory.Span); + } + + /// + /// Given a certificate, usually from the store, try to find the + /// corresponding certificate in (usually the store)."/> + /// + /// An open . + /// A certificate to search for. + /// The certificate, if any, corresponding to in . + /// True if a corresponding certificate was found. + /// has richer filtering and a lot of debugging output that's unhelpful here. + internal static bool TryFindCertificateInStore(X509Store store, X509Certificate2 certificate, [NotNullWhen(true)] out X509Certificate2? foundCertificate) + { + foundCertificate = null; + + // We specifically don't search by thumbprint to avoid being flagged for using a SHA-1 hash. + var certificatesWithSubjectName = store.Certificates.Find(X509FindType.FindBySerialNumber, certificate.SerialNumber, validOnly: false); + if (certificatesWithSubjectName.Count == 0) + { + return false; + } + + var certificatesToDispose = new List(); + foreach (var candidate in certificatesWithSubjectName.OfType()) + { + if (foundCertificate is null && AreCertificatesEqual(candidate, certificate)) + { + foundCertificate = candidate; + } + else + { + certificatesToDispose.Add(candidate); + } + } + DisposeCertificates(certificatesToDispose); + return foundCertificate is not null; + } + [EventSource(Name = "Dotnet-dev-certs")] public sealed class CertificateManagerEventSource : EventSource { diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index 30237e445f61..f55c57025e55 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -11,6 +11,11 @@ namespace Microsoft.AspNetCore.Certificates.Generation; +/// +/// Normally, we avoid the use of because it's a SHA-1 hash and, therefore, +/// not adequate for security applications. However, the MacOS security tool uses SHA-1 hashes for certificate +/// identification, so we're stuck. +/// internal sealed class MacOSCertificateManager : CertificateManager { // User keychain. Guard with quotes when using in command lines since users may have set diff --git a/src/Shared/CertificateGeneration/WindowsCertificateManager.cs b/src/Shared/CertificateGeneration/WindowsCertificateManager.cs index 69f57066438b..ee732bed9076 100644 --- a/src/Shared/CertificateGeneration/WindowsCertificateManager.cs +++ b/src/Shared/CertificateGeneration/WindowsCertificateManager.cs @@ -71,26 +71,22 @@ protected override X509Certificate2 SaveCertificateCore(X509Certificate2 certifi protected override void TrustCertificateCore(X509Certificate2 certificate) { - using var publicCertificate = new X509Certificate2(certificate.Export(X509ContentType.Cert)); - - publicCertificate.FriendlyName = certificate.FriendlyName; - using var store = new X509Store(StoreName.Root, StoreLocation.CurrentUser); - store.Open(OpenFlags.ReadWrite); - var existing = store.Certificates.Find(X509FindType.FindByThumbprint, publicCertificate.Thumbprint, validOnly: false); - if (existing.Count > 0) + + if (TryFindCertificateInStore(store, certificate, out _)) { Log.WindowsCertificateAlreadyTrusted(); - DisposeCertificates(existing.OfType()); return; } try { Log.WindowsAddCertificateToRootStore(); + + using var publicCertificate = X509CertificateLoader.LoadCertificate(certificate.Export(X509ContentType.Cert)); + publicCertificate.FriendlyName = certificate.FriendlyName; store.Add(publicCertificate); - store.Close(); } catch (CryptographicException exception) when (exception.HResult == UserCancelledErrorCode) { @@ -102,14 +98,11 @@ protected override void TrustCertificateCore(X509Certificate2 certificate) protected override void RemoveCertificateFromTrustedRoots(X509Certificate2 certificate) { Log.WindowsRemoveCertificateFromRootStoreStart(); - using var store = new X509Store(StoreName.Root, StoreLocation.CurrentUser); + using var store = new X509Store(StoreName.Root, StoreLocation.CurrentUser); store.Open(OpenFlags.ReadWrite); - var matching = store.Certificates - .OfType() - .SingleOrDefault(c => c.SerialNumber == certificate.SerialNumber); - if (matching != null) + if (TryFindCertificateInStore(store, certificate, out var matching)) { store.Remove(matching); } @@ -118,14 +111,13 @@ protected override void RemoveCertificateFromTrustedRoots(X509Certificate2 certi Log.WindowsRemoveCertificateFromRootStoreNotFound(); } - store.Close(); Log.WindowsRemoveCertificateFromRootStoreEnd(); } public override bool IsTrusted(X509Certificate2 certificate) { return ListCertificates(StoreName.Root, StoreLocation.CurrentUser, isValid: true, requireExportable: false) - .Any(c => c.Thumbprint == certificate.Thumbprint); + .Any(c => AreCertificatesEqual(c, certificate)); } protected override IList GetCertificatesToRemove(StoreName storeName, StoreLocation storeLocation)