From b852683e5669bd564adec16352afe133f866b693 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Mon, 13 Jun 2022 21:00:30 -0700 Subject: [PATCH 01/22] macOS dev-certs changes - Store dev-certs for macOS in a well-known location on disk in addition to the keychain. - Misc code cleanup/refactoring. - Update error messages. --- .../CertificateManager.cs | 58 ++-- .../MacOSCertificateManager.cs | 278 ++++++++++++------ src/Tools/dotnet-dev-certs/src/Program.cs | 14 +- 3 files changed, 230 insertions(+), 120 deletions(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index ec4c1db015ae..44467ac9ae96 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -78,7 +78,7 @@ public IList ListCertificates( { using var store = new X509Store(storeName, location); store.Open(OpenFlags.ReadOnly); - certificates.AddRange(store.Certificates.OfType()); + PopulateCertificatesFromStore(store, certificates); IEnumerable matchingCertificates = certificates; matchingCertificates = matchingCertificates .Where(c => HasOid(c, AspNetHttpsOid)); @@ -161,6 +161,11 @@ bool IsValidCertificate(X509Certificate2 certificate, DateTimeOffset currentDate GetCertificateVersion(certificate) >= AspNetHttpsCertificateVersion; } + protected virtual void PopulateCertificatesFromStore(X509Store store, List certificates) + { + certificates.AddRange(store.Certificates.OfType()); + } + public IList GetHttpsCertificates() => ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: true, requireExportable: true); @@ -411,13 +416,6 @@ internal ImportCertificateResult ImportCertificate(string certificatePath, strin public void CleanupHttpsCertificates() { - // On OS X we don't have a good way to manage trusted certificates in the system keychain - // so we do everything by invoking the native toolchain. - // This has some limitations, like for example not being able to identify our custom OID extension. For that - // matter, when we are cleaning up certificates on the machine, we start by removing the trusted certificates. - // To do this, we list the certificates that we can identify on the current user personal store and we invoke - // the native toolchain to remove them from the sytem keychain. Once we have removed the trusted certificates, - // we remove the certificates from the local user store to finish up the cleanup. var certificates = ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: false); var filteredCertificates = certificates.Where(c => c.Subject == Subject); @@ -430,6 +428,8 @@ public void CleanupHttpsCertificates() foreach (var certificate in filteredCertificates) { + // RemoveLocations.All will first remove from the trusted roots (e.g. keychain on + // macOS) and then from the local user store. RemoveCertificate(certificate, RemoveLocations.All); } } @@ -745,7 +745,7 @@ internal static void DisposeCertificates(IEnumerable disposabl } } - private static void RemoveCertificateFromUserStore(X509Certificate2 certificate) + protected virtual void RemoveCertificateFromUserStore(X509Certificate2 certificate) { try { @@ -753,14 +753,7 @@ private static void RemoveCertificateFromUserStore(X509Certificate2 certificate) { Log.RemoveCertificateFromUserStoreStart(GetDescription(certificate)); } - using var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); - store.Open(OpenFlags.ReadWrite); - var matching = store.Certificates - .OfType() - .Single(c => c.SerialNumber == certificate.SerialNumber); - - store.Remove(matching); - store.Close(); + RemoveCertificateFromUserStoreCore(certificate); Log.RemoveCertificateFromUserStoreEnd(); } catch (Exception ex) when (Log.IsEnabled()) @@ -770,6 +763,18 @@ private static void RemoveCertificateFromUserStore(X509Certificate2 certificate) } } + protected virtual void RemoveCertificateFromUserStoreCore(X509Certificate2 certificate) + { + using var store = new X509Store(StoreName.My, StoreLocation.CurrentUser); + store.Open(OpenFlags.ReadWrite); + var matching = store.Certificates + .OfType() + .Single(c => c.SerialNumber == certificate.SerialNumber); + + store.Remove(matching); + store.Close(); + } + internal static string ToCertificateDescription(IEnumerable certificates) { var list = certificates.ToList(); @@ -944,8 +949,8 @@ public sealed class CertificateManagerEventSource : EventSource [Event(55, Level = EventLevel.Verbose, Message = "Finished importing the certificate to the keychain.")] internal void MacOSAddCertificateToKeyChainEnd() => WriteEvent(55); - [Event(56, Level = EventLevel.Error, Message = "An error has occurred while importing the certificate to the keychain: {0}.")] - internal void MacOSAddCertificateToKeyChainError(int exitCode) => WriteEvent(56, exitCode); + [Event(56, Level = EventLevel.Error, Message = "An error has occurred while importing the certificate to the keychain: {0}, {1}")] + internal void MacOSAddCertificateToKeyChainError(int exitCode, string output) => WriteEvent(56, exitCode, output); [Event(57, Level = EventLevel.Verbose, Message = "Writing the certificate to: {0}.")] public void WritePemKeyToDisk(string path) => WriteEvent(57, path); @@ -970,6 +975,21 @@ public sealed class CertificateManagerEventSource : EventSource [Event(64, Level = EventLevel.Error, Message = "The provided certificate '{0}' is not a valid ASP.NET Core HTTPS development certificate.")] internal void NoHttpsDevelopmentCertificate(string description) => WriteEvent(64, description); + + [Event(65, Level = EventLevel.Verbose, Message = "The certificate is already trusted.")] + public void MacOSCertificateAlreadyTrusted() => WriteEvent(65); + + [Event(66, Level = EventLevel.Verbose, Message = "Saving the certificate {1} to the user profile folder '{0}'.")] + internal void MacOSAddCertificateToUserProfileDirStart(string directory, string certificate) => WriteEvent(66, directory, certificate); + + [Event(67, Level = EventLevel.Verbose, Message = "Finished saving the certificate to the user profile folder.")] + internal void MacOSAddCertificateToUserProfileDirEnd() => WriteEvent(67); + + [Event(68, Level = EventLevel.Error, Message = "An error has occurred while saving certificate '{0}' in the user profile folder: {1}.")] + internal void MacOSAddCertificateToUserProfileDirError(string certificateThumbprint, string errorMessage) => WriteEvent(68, certificateThumbprint, errorMessage); + + [Event(69, Level = EventLevel.Error, Message = "An error has occurred while removing certificate '{0}' from the user profile folder: {1}.")] + internal void MacOSRemoveCertificateFromUserProfileDirError(string certificateThumbprint, string errorMessage) => WriteEvent(69, certificateThumbprint, errorMessage); } internal sealed class UserCancelledTrustException : Exception diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index 28c971680b77..24ebb07c39dc 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -1,38 +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; using System.Globalization; -using System.IO; -using System.Linq; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; -using System.Text.RegularExpressions; namespace Microsoft.AspNetCore.Certificates.Generation; internal sealed class MacOSCertificateManager : CertificateManager { - private const string CertificateSubjectRegex = "CN=(.*[^,]+).*"; private static readonly string MacOSUserKeyChain = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "/Library/Keychains/login.keychain-db"; - private const string MacOSSystemKeyChain = "/Library/Keychains/System.keychain"; - private const string MacOSFindCertificateCommandLine = "security"; - private const string MacOSFindCertificateCommandLineArgumentsFormat = "find-certificate -c {0} -a -Z -p " + MacOSSystemKeyChain; - private const string MacOSFindCertificateOutputRegex = "SHA-1 hash: ([0-9A-Z]+)"; - private const string MacOSRemoveCertificateTrustCommandLine = "sudo"; - private const string MacOSRemoveCertificateTrustCommandLineArgumentsFormat = "security remove-trusted-cert -d {0}"; + + // Verify the certificate {0} for the SSL and X.509 Basic Policy. + private const string MacOSVerifyCertificateCommandLine = "security"; + private const string MacOSVerifyCertificateCommandLineArgumentsFormat = $"verify-cert -c {{0}} -p basic -p ssl"; + + // Delete a certificate with the specified SHA-256 (or SHA-1) hash {0} from keychain {1}. private const string MacOSDeleteCertificateCommandLine = "sudo"; private const string MacOSDeleteCertificateCommandLineArgumentsFormat = "security delete-certificate -Z {0} {1}"; - private const string MacOSTrustCertificateCommandLine = "sudo"; - private const string MacOSTrustCertificateCommandLineArguments = "security add-trusted-cert -d -r trustRoot -k " + MacOSSystemKeyChain + " "; + // Add a certificate to the per-user trust settings in the user keychain. The trust policy + // for the certificate will be set to be always trusted for SSL and X.509 Basic Policy. + // Note: This operation will require user authentication via a dialog. + private const string MacOSTrustCertificateCommandLine = "security"; + private static readonly string MacOSTrustCertificateCommandLineArguments = $"add-trusted-cert -p basic -p ssl -k {MacOSUserKeyChain} "; + + // Remove a certificate from the per-user trust settings. + // Note: This operation will require user authentication via a dialog. + private const string MacOSRemoveCertificateTrustCommandLine = "security"; + private const string MacOSRemoveCertificateTrustCommandLineArgumentsFormat = "remove-trusted-cert {0}"; + + // Well-known location on disk where dev-certs are stored. + private static readonly string MacOSUserHttpsCertificateLocation = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".aspnet", "dev-certs", "https"); + + // Import a pkcs12 certificate into the user keychain using the unwrapping passphrase {1}, and + // allow any application to access the imported key without warning. private const string MacOSAddCertificateToKeyChainCommandLine = "security"; private static readonly string MacOSAddCertificateToKeyChainCommandLineArgumentsFormat = "import {0} -k " + MacOSUserKeyChain + " -t cert -f pkcs12 -P {1} -A"; - public const string InvalidCertificateState = "The ASP.NET Core developer certificate is in an invalid state. " + - "To fix this issue, run the following commands 'dotnet dev-certs https --clean' and 'dotnet dev-certs https' to remove all existing ASP.NET Core development certificates " + + public const string InvalidCertificateState = + "The ASP.NET Core developer certificate is in an invalid state. " + + "To fix this issue, run 'dotnet dev-certs https --clean' and 'dotnet dev-certs https' " + + "to remove all existing ASP.NET Core development certificates " + "and create a new untrusted developer certificate. " + "On macOS or Windows, use 'dotnet dev-certs https --trust' to trust the new certificate."; @@ -41,8 +51,6 @@ internal sealed class MacOSCertificateManager : CertificateManager "A prompt might appear to ask for permission to access the key. " + "When that happens, select 'Always Allow' to grant 'dotnet' access to the certificate key in the future."; - private static readonly TimeSpan MaxRegexTimeout = TimeSpan.FromMinutes(1); - public MacOSCertificateManager() { } @@ -54,6 +62,12 @@ internal MacOSCertificateManager(string subject, int version) protected override void TrustCertificateCore(X509Certificate2 publicCertificate) { + if (IsTrusted(publicCertificate)) + { + Log.MacOSCertificateAlreadyTrusted(); + return; + } + var tmpFile = Path.GetTempFileName(); try { @@ -138,48 +152,41 @@ internal override void CorrectCertificateState(X509Certificate2 candidate) } } + // Use verify-cert to verify the certificate for the SSL and X.509 Basic Policy. public override bool IsTrusted(X509Certificate2 certificate) { - var subjectMatch = Regex.Match(certificate.Subject, CertificateSubjectRegex, RegexOptions.Singleline, MaxRegexTimeout); - if (!subjectMatch.Success) + var tmpFile = Path.GetTempFileName(); + try { - throw new InvalidOperationException($"Can't determine the subject for the certificate with subject '{certificate.Subject}'."); + ExportCertificate(certificate, tmpFile, includePrivateKey: false, password: null, CertificateKeyExportFormat.Pem); + + using var checkTrustProcess = Process.Start(new ProcessStartInfo( + MacOSVerifyCertificateCommandLine, + string.Format(CultureInfo.InvariantCulture, MacOSVerifyCertificateCommandLineArgumentsFormat, tmpFile)) + { + RedirectStandardOutput = true, + // Do this to avoid showing output to the console when the cert is not trusted. It is trivial to export + // the cert and replicate the command to see details. + RedirectStandardError = true, + }); + checkTrustProcess!.WaitForExit(); + return checkTrustProcess.ExitCode == 0; } - var subject = subjectMatch.Groups[1].Value; - using var checkTrustProcess = Process.Start(new ProcessStartInfo( - MacOSFindCertificateCommandLine, - string.Format(CultureInfo.InvariantCulture, MacOSFindCertificateCommandLineArgumentsFormat, subject)) + finally { - RedirectStandardOutput = true - }); - var output = checkTrustProcess!.StandardOutput.ReadToEnd(); - checkTrustProcess.WaitForExit(); - var matches = Regex.Matches(output, MacOSFindCertificateOutputRegex, RegexOptions.Multiline, MaxRegexTimeout); - var hashes = matches.OfType().Select(m => m.Groups[1].Value).ToList(); - return hashes.Any(h => string.Equals(h, certificate.Thumbprint, StringComparison.Ordinal)); + if (File.Exists(tmpFile)) + { + File.Delete(tmpFile); + } + } } protected override void RemoveCertificateFromTrustedRoots(X509Certificate2 certificate) { - if (IsTrusted(certificate)) // On OSX this check just ensures its on the system keychain + if (IsTrusted(certificate)) { - // A trusted certificate in OSX is installed into the system keychain and - // as a "trust rule" applied to it. - // To remove the certificate we first need to remove the "trust rule" and then - // remove the certificate from the keychain. - // We don't care if we fail to remove the trust rule if - // for some reason the certificate became untrusted. - // Trying to remove the certificate from the keychain will fail if the certificate is - // trusted. - try - { - RemoveCertificateTrustRule(certificate); - } - catch - { - } - - RemoveCertificateFromKeyChain(MacOSSystemKeyChain, certificate); + RemoveCertificateFromKeyChain(MacOSUserKeyChain, certificate); + RemoveCertificateFromUserStoreCore(certificate); } else { @@ -187,6 +194,55 @@ protected override void RemoveCertificateFromTrustedRoots(X509Certificate2 certi } } + private static void RemoveCertificateFromKeyChain(string keyChain, X509Certificate2 certificate) + { + try + { + // Trying to remove the certificate from the keychain will fail if the certificate + // has a "trust rule" applied to it. + RemoveCertificateTrustRule(certificate); + } + catch + { + // We don't care if we fail to remove the trust rule if for some reason the certificate + // became untrusted. + } + + var processInfo = new ProcessStartInfo( + MacOSDeleteCertificateCommandLine, + string.Format( + CultureInfo.InvariantCulture, + MacOSDeleteCertificateCommandLineArgumentsFormat, + certificate.Thumbprint.ToUpperInvariant(), + keyChain + )) + { + RedirectStandardOutput = true, + RedirectStandardError = true + }; + + if (Log.IsEnabled()) + { + Log.MacOSRemoveCertificateFromKeyChainStart(keyChain, GetDescription(certificate)); + } + + using (var process = Process.Start(processInfo)) + { + var output = process!.StandardOutput.ReadToEnd() + process.StandardError.ReadToEnd(); + process.WaitForExit(); + + if (process.ExitCode != 0) + { + Log.MacOSRemoveCertificateFromKeyChainError(process.ExitCode); + throw new InvalidOperationException($@"There was an error removing the certificate with thumbprint '{certificate.Thumbprint}'. + +{output}"); + } + } + + Log.MacOSRemoveCertificateFromKeyChainEnd(); + } + private static void RemoveCertificateTrustRule(X509Certificate2 certificate) { Log.MacOSRemoveCertificateTrustRuleStart(GetDescription(certificate)); @@ -226,49 +282,43 @@ private static void RemoveCertificateTrustRule(X509Certificate2 certificate) } } - private static void RemoveCertificateFromKeyChain(string keyChain, X509Certificate2 certificate) - { - var processInfo = new ProcessStartInfo( - MacOSDeleteCertificateCommandLine, - string.Format( - CultureInfo.InvariantCulture, - MacOSDeleteCertificateCommandLineArgumentsFormat, - certificate.Thumbprint.ToUpperInvariant(), - keyChain - )) - { - RedirectStandardOutput = true, - RedirectStandardError = true - }; + // We don't have a good way of checking on the underlying implementation if it is exportable, so just return true. + protected override bool IsExportable(X509Certificate2 c) => true; - if (Log.IsEnabled()) - { - Log.MacOSRemoveCertificateFromKeyChainStart(keyChain, GetDescription(certificate)); - } + protected override X509Certificate2 SaveCertificateCore(X509Certificate2 certificate, StoreName storeName, StoreLocation storeLocation) + { + // We do this for backwards compatibility with previous versions. .NET 7.0 and onwards will ignore the + // certificate on the keychain and load it directly from disk. + var keychainSaveSuccess = SaveCertificateToUserKeychain(certificate); + Debug.Assert(keychainSaveSuccess); - using (var process = Process.Start(processInfo)) + try { - var output = process!.StandardOutput.ReadToEnd() + process.StandardError.ReadToEnd(); - process.WaitForExit(); + var certBytes = certificate.Export(X509ContentType.Pfx); - if (process.ExitCode != 0) + if (Log.IsEnabled()) { - Log.MacOSRemoveCertificateFromKeyChainError(process.ExitCode); - throw new InvalidOperationException($@"There was an error removing the certificate with thumbprint '{certificate.Thumbprint}'. - -{output}"); + Log.MacOSAddCertificateToUserProfileDirStart(MacOSUserKeyChain, GetDescription(certificate)); } + + // Ensure that the directory exists before writing to the file. + Directory.CreateDirectory(MacOSUserHttpsCertificateLocation); + + File.WriteAllBytes(GetCertificateFilePath(certificate), certBytes); + } + catch (Exception ex) + { + Log.MacOSAddCertificateToUserProfileDirError(certificate.Thumbprint, ex.Message); } - Log.MacOSRemoveCertificateFromKeyChainEnd(); - } + Log.MacOSAddCertificateToKeyChainEnd(); + Log.MacOSAddCertificateToUserProfileDirEnd(); - // We don't have a good way of checking on the underlying implementation if ti is exportable, so just return true. - protected override bool IsExportable(X509Certificate2 c) => true; + return certificate; + } - protected override X509Certificate2 SaveCertificateCore(X509Certificate2 certificate, StoreName storeName, StoreLocation storeLocation) + private static bool SaveCertificateToUserKeychain(X509Certificate2 certificate) { - // security import https.pfx -k $loginKeyChain -t cert -f pkcs12 -P password -A; var passwordBytes = new byte[48]; RandomNumberGenerator.Fill(passwordBytes.AsSpan()[0..35]); var password = Convert.ToBase64String(passwordBytes, 0, 36); @@ -278,12 +328,7 @@ protected override X509Certificate2 SaveCertificateCore(X509Certificate2 certifi var processInfo = new ProcessStartInfo( MacOSAddCertificateToKeyChainCommandLine, - string.Format( - CultureInfo.InvariantCulture, - MacOSAddCertificateToKeyChainCommandLineArgumentsFormat, - certificatePath, - password - )) + string.Format(CultureInfo.InvariantCulture,MacOSAddCertificateToKeyChainCommandLineArgumentsFormat, certificatePath, password)) { RedirectStandardOutput = true, RedirectStandardError = true @@ -301,20 +346,65 @@ protected override X509Certificate2 SaveCertificateCore(X509Certificate2 certifi if (process.ExitCode != 0) { - Log.MacOSAddCertificateToKeyChainError(process.ExitCode); - throw new InvalidOperationException($@"There was an error importing the certificate into the user key chain '{certificate.Thumbprint}'. - -{output}"); + Log.MacOSAddCertificateToKeyChainError(process.ExitCode, output); + return false; } } Log.MacOSAddCertificateToKeyChainEnd(); - - return certificate; + return true; } + private static string GetCertificateFilePath(X509Certificate2 certificate) => + Path.Combine(MacOSUserHttpsCertificateLocation, $"aspnetcore-localhost-{certificate.Thumbprint}.pfx"); + protected override IList GetCertificatesToRemove(StoreName storeName, StoreLocation storeLocation) { return ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: false); } + + protected override void PopulateCertificatesFromStore(X509Store store, List certificates) + { + if (store.Name! != StoreName.My.ToString() || store.Location != StoreLocation.CurrentUser) + { + base.PopulateCertificatesFromStore(store, certificates); + } + else + { + if (Directory.Exists(MacOSUserHttpsCertificateLocation)) + { + var certificateFiles = Directory.EnumerateFiles(MacOSUserHttpsCertificateLocation, "aspnetcore-localhost-*.pfx"); + foreach (var file in certificateFiles) + { + try + { + var certificate = new X509Certificate2(file); + certificates.Add(certificate); + } + catch (Exception) + { + /////// Log exception + throw; + } + } + } + + } + } + + protected override void RemoveCertificateFromUserStoreCore(X509Certificate2 certificate) + { + try + { + var certificatePath = GetCertificateFilePath(certificate); + if (File.Exists(certificatePath)) + { + File.Delete(certificatePath); + } + } + catch (Exception ex) + { + Log.MacOSRemoveCertificateFromUserProfileDirError(certificate.Thumbprint, ex.Message); + } + } } diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index 06cd156aa944..c324e70aa8cd 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -74,8 +74,8 @@ public static int Main(string[] args) // We want to force generating a key without a password to not be an accident. var noPassword = c.Option("-np|--no-password", - "Explicitly request that you don't use a password for the key when exporting a certificate to a PEM format", - CommandOptionType.NoValue); + "Explicitly request that you don't use a password for the key when exporting a certificate to a PEM format", + CommandOptionType.NoValue); var check = c.Option( "-c|--check", @@ -170,10 +170,10 @@ public static int Main(string[] args) if (clean.HasValue()) { - var clean = CleanHttpsCertificates(reporter); - if (clean != Success || !import.HasValue()) + var cleanResult = CleanHttpsCertificates(reporter); + if (cleanResult != Success || !import.HasValue()) { - return clean; + return cleanResult; } return ImportCertificate(import, password, reporter); @@ -365,9 +365,9 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio { reporter.Warn("Trusting the HTTPS development certificate was requested. If the certificate is not " + "already trusted we will run the following command:" + Environment.NewLine + - "'sudo security add-trusted-cert -d -r trustRoot -k /Library/Keychains/System.keychain <>'" + + "'security add-trusted-cert -p basic -p ssl -k <> <>'" + Environment.NewLine + "This command might prompt you for your password to install the certificate " + - "on the system keychain. To undo these changes: 'sudo security remove-trusted-cert -d <>'"); + "on the keychain. To undo these changes: 'security remove-trusted-cert <>'"); } if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) From d7f6b51b122623dcc72d12e56de3e0a29fe51709 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 16 Jun 2022 11:59:17 -0700 Subject: [PATCH 02/22] Remove unnecessary trust checks for deletion. --- .../MacOSCertificateManager.cs | 103 +++++++++--------- 1 file changed, 53 insertions(+), 50 deletions(-) diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index 24ebb07c39dc..ecb96866a467 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -26,10 +26,10 @@ internal sealed class MacOSCertificateManager : CertificateManager private const string MacOSTrustCertificateCommandLine = "security"; private static readonly string MacOSTrustCertificateCommandLineArguments = $"add-trusted-cert -p basic -p ssl -k {MacOSUserKeyChain} "; - // Remove a certificate from the per-user trust settings. - // Note: This operation will require user authentication via a dialog. - private const string MacOSRemoveCertificateTrustCommandLine = "security"; - private const string MacOSRemoveCertificateTrustCommandLineArgumentsFormat = "remove-trusted-cert {0}"; + // // Remove a certificate from the per-user trust settings. + // // Note: This operation will require user authentication via a dialog. + // private const string MacOSRemoveCertificateTrustCommandLine = "security"; + // private const string MacOSRemoveCertificateTrustCommandLineArgumentsFormat = "remove-trusted-cert {0}"; // Well-known location on disk where dev-certs are stored. private static readonly string MacOSUserHttpsCertificateLocation = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".aspnet", "dev-certs", "https"); @@ -183,15 +183,15 @@ public override bool IsTrusted(X509Certificate2 certificate) protected override void RemoveCertificateFromTrustedRoots(X509Certificate2 certificate) { - if (IsTrusted(certificate)) - { + // if (IsTrusted(certificate)) //// necessary? + // { RemoveCertificateFromKeyChain(MacOSUserKeyChain, certificate); RemoveCertificateFromUserStoreCore(certificate); - } - else - { - Log.MacOSCertificateUntrusted(GetDescription(certificate)); - } + // } + // else + // { + // Log.MacOSCertificateUntrusted(GetDescription(certificate)); + // } } private static void RemoveCertificateFromKeyChain(string keyChain, X509Certificate2 certificate) @@ -200,7 +200,10 @@ private static void RemoveCertificateFromKeyChain(string keyChain, X509Certifica { // Trying to remove the certificate from the keychain will fail if the certificate // has a "trust rule" applied to it. - RemoveCertificateTrustRule(certificate); + // if (IsTrusted(certificate)) + // { + // RemoveCertificateTrustRule(certificate); + // } } catch { @@ -243,44 +246,44 @@ private static void RemoveCertificateFromKeyChain(string keyChain, X509Certifica Log.MacOSRemoveCertificateFromKeyChainEnd(); } - private static void RemoveCertificateTrustRule(X509Certificate2 certificate) - { - Log.MacOSRemoveCertificateTrustRuleStart(GetDescription(certificate)); - var certificatePath = Path.GetTempFileName(); - try - { - var certBytes = certificate.Export(X509ContentType.Cert); - File.WriteAllBytes(certificatePath, certBytes); - var processInfo = new ProcessStartInfo( - MacOSRemoveCertificateTrustCommandLine, - string.Format( - CultureInfo.InvariantCulture, - MacOSRemoveCertificateTrustCommandLineArgumentsFormat, - certificatePath - )); - using var process = Process.Start(processInfo); - process!.WaitForExit(); - if (process.ExitCode != 0) - { - Log.MacOSRemoveCertificateTrustRuleError(process.ExitCode); - } - Log.MacOSRemoveCertificateTrustRuleEnd(); - } - finally - { - try - { - if (File.Exists(certificatePath)) - { - File.Delete(certificatePath); - } - } - catch - { - // We don't care about failing to do clean-up on a temp file. - } - } - } + // private static void RemoveCertificateTrustRule(X509Certificate2 certificate) + // { + // Log.MacOSRemoveCertificateTrustRuleStart(GetDescription(certificate)); + // var certificatePath = Path.GetTempFileName(); + // try + // { + // var certBytes = certificate.Export(X509ContentType.Cert); + // File.WriteAllBytes(certificatePath, certBytes); + // var processInfo = new ProcessStartInfo( + // MacOSRemoveCertificateTrustCommandLine, + // string.Format( + // CultureInfo.InvariantCulture, + // MacOSRemoveCertificateTrustCommandLineArgumentsFormat, + // certificatePath + // )); + // using var process = Process.Start(processInfo); + // process!.WaitForExit(); + // if (process.ExitCode != 0) + // { + // Log.MacOSRemoveCertificateTrustRuleError(process.ExitCode); + // } + // Log.MacOSRemoveCertificateTrustRuleEnd(); + // } + // finally + // { + // try + // { + // if (File.Exists(certificatePath)) + // { + // File.Delete(certificatePath); + // } + // } + // catch + // { + // // We don't care about failing to do clean-up on a temp file. + // } + // } + // } // We don't have a good way of checking on the underlying implementation if it is exportable, so just return true. protected override bool IsExportable(X509Certificate2 c) => true; From 2f6d1f2b4704eaf2ef3c9062ffdc80c3572462f5 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 16 Jun 2022 15:21:58 -0700 Subject: [PATCH 03/22] Add specific return message for existing cert trusted. --- src/Shared/CertificateGeneration/CertificateManager.cs | 2 ++ src/Shared/CertificateGeneration/EnsureCertificateResult.cs | 1 + src/Tools/dotnet-dev-certs/src/Program.cs | 3 +++ 3 files changed, 6 insertions(+) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 44467ac9ae96..5ba0b202a99a 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -345,6 +345,8 @@ public EnsureCertificateResult EnsureAspNetCoreHttpsDevelopmentCertificate( result = EnsureCertificateResult.FailedToTrustTheCertificate; return result; } + + result = EnsureCertificateResult.ExistingHttpsCertificateTrusted; } DisposeCertificates(!isNewCertificate ? certificates : certificates.Append(certificate)); diff --git a/src/Shared/CertificateGeneration/EnsureCertificateResult.cs b/src/Shared/CertificateGeneration/EnsureCertificateResult.cs index eb1e3c0b0a66..6ed38b1cdfcf 100644 --- a/src/Shared/CertificateGeneration/EnsureCertificateResult.cs +++ b/src/Shared/CertificateGeneration/EnsureCertificateResult.cs @@ -13,5 +13,6 @@ internal enum EnsureCertificateResult FailedToTrustTheCertificate, UserCancelledTrustStep, FailedToMakeKeyAccessible, + ExistingHttpsCertificateTrusted } diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index c324e70aa8cd..fb6944e48b79 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -430,6 +430,9 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio case EnsureCertificateResult.UserCancelledTrustStep: reporter.Warn("The user cancelled the trust step."); return ErrorUserCancelledTrustPrompt; + case EnsureCertificateResult.ExistingHttpsCertificateTrusted: + reporter.Output("Successfully trusted the existing HTTPS certificate."); + return Success; default: reporter.Error("Something went wrong. The HTTPS developer certificate could not be created."); return CriticalError; From 765e1a768a77728598d1d9ef2ba0f9dbcfb55fef Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 16 Jun 2022 18:02:07 -0700 Subject: [PATCH 04/22] Clean up unnecessary code. --- .../CertificateManager.cs | 3 + .../MacOSCertificateManager.cs | 72 +------------------ 2 files changed, 6 insertions(+), 69 deletions(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 5ba0b202a99a..0bb05da2aea1 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -992,6 +992,9 @@ public sealed class CertificateManagerEventSource : EventSource [Event(69, Level = EventLevel.Error, Message = "An error has occurred while removing certificate '{0}' from the user profile folder: {1}.")] internal void MacOSRemoveCertificateFromUserProfileDirError(string certificateThumbprint, string errorMessage) => WriteEvent(69, certificateThumbprint, errorMessage); + + [Event(70, Level = EventLevel.Error, Message = "The file '{0}' is not a valid certificate.")] + internal void MacOSFileIsNotAValidCertificate(string path) => WriteEvent(70, path); } internal sealed class UserCancelledTrustException : Exception diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index ecb96866a467..60936f608a2f 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -26,11 +26,6 @@ internal sealed class MacOSCertificateManager : CertificateManager private const string MacOSTrustCertificateCommandLine = "security"; private static readonly string MacOSTrustCertificateCommandLineArguments = $"add-trusted-cert -p basic -p ssl -k {MacOSUserKeyChain} "; - // // Remove a certificate from the per-user trust settings. - // // Note: This operation will require user authentication via a dialog. - // private const string MacOSRemoveCertificateTrustCommandLine = "security"; - // private const string MacOSRemoveCertificateTrustCommandLineArgumentsFormat = "remove-trusted-cert {0}"; - // Well-known location on disk where dev-certs are stored. private static readonly string MacOSUserHttpsCertificateLocation = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".aspnet", "dev-certs", "https"); @@ -183,34 +178,12 @@ public override bool IsTrusted(X509Certificate2 certificate) protected override void RemoveCertificateFromTrustedRoots(X509Certificate2 certificate) { - // if (IsTrusted(certificate)) //// necessary? - // { - RemoveCertificateFromKeyChain(MacOSUserKeyChain, certificate); - RemoveCertificateFromUserStoreCore(certificate); - // } - // else - // { - // Log.MacOSCertificateUntrusted(GetDescription(certificate)); - // } + RemoveCertificateFromKeyChain(MacOSUserKeyChain, certificate); + RemoveCertificateFromUserStoreCore(certificate); } private static void RemoveCertificateFromKeyChain(string keyChain, X509Certificate2 certificate) { - try - { - // Trying to remove the certificate from the keychain will fail if the certificate - // has a "trust rule" applied to it. - // if (IsTrusted(certificate)) - // { - // RemoveCertificateTrustRule(certificate); - // } - } - catch - { - // We don't care if we fail to remove the trust rule if for some reason the certificate - // became untrusted. - } - var processInfo = new ProcessStartInfo( MacOSDeleteCertificateCommandLine, string.Format( @@ -246,45 +219,6 @@ private static void RemoveCertificateFromKeyChain(string keyChain, X509Certifica Log.MacOSRemoveCertificateFromKeyChainEnd(); } - // private static void RemoveCertificateTrustRule(X509Certificate2 certificate) - // { - // Log.MacOSRemoveCertificateTrustRuleStart(GetDescription(certificate)); - // var certificatePath = Path.GetTempFileName(); - // try - // { - // var certBytes = certificate.Export(X509ContentType.Cert); - // File.WriteAllBytes(certificatePath, certBytes); - // var processInfo = new ProcessStartInfo( - // MacOSRemoveCertificateTrustCommandLine, - // string.Format( - // CultureInfo.InvariantCulture, - // MacOSRemoveCertificateTrustCommandLineArgumentsFormat, - // certificatePath - // )); - // using var process = Process.Start(processInfo); - // process!.WaitForExit(); - // if (process.ExitCode != 0) - // { - // Log.MacOSRemoveCertificateTrustRuleError(process.ExitCode); - // } - // Log.MacOSRemoveCertificateTrustRuleEnd(); - // } - // finally - // { - // try - // { - // if (File.Exists(certificatePath)) - // { - // File.Delete(certificatePath); - // } - // } - // catch - // { - // // We don't care about failing to do clean-up on a temp file. - // } - // } - // } - // We don't have a good way of checking on the underlying implementation if it is exportable, so just return true. protected override bool IsExportable(X509Certificate2 c) => true; @@ -386,7 +320,7 @@ protected override void PopulateCertificatesFromStore(X509Store store, List Date: Thu, 16 Jun 2022 19:02:11 -0700 Subject: [PATCH 05/22] Throw on keychain add failure. --- .../CertificateGeneration/MacOSCertificateManager.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index 60936f608a2f..814923556002 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -224,10 +224,7 @@ private static void RemoveCertificateFromKeyChain(string keyChain, X509Certifica protected override X509Certificate2 SaveCertificateCore(X509Certificate2 certificate, StoreName storeName, StoreLocation storeLocation) { - // We do this for backwards compatibility with previous versions. .NET 7.0 and onwards will ignore the - // certificate on the keychain and load it directly from disk. - var keychainSaveSuccess = SaveCertificateToUserKeychain(certificate); - Debug.Assert(keychainSaveSuccess); + SaveCertificateToUserKeychain(certificate); try { @@ -284,7 +281,7 @@ private static bool SaveCertificateToUserKeychain(X509Certificate2 certificate) if (process.ExitCode != 0) { Log.MacOSAddCertificateToKeyChainError(process.ExitCode, output); - return false; + throw new InvalidOperationException("Failed to add the certificate to the keychain. Are you running in a non-interactive session perhaps?"); } } From a9a1808c39daa94c81410c68b2a7d6e1516a065e Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 16 Jun 2022 19:16:59 -0700 Subject: [PATCH 06/22] Fix handling of create+trust workflow. --- src/Shared/CertificateGeneration/CertificateManager.cs | 9 ++++++++- .../CertificateGeneration/EnsureCertificateResult.cs | 3 ++- src/Tools/dotnet-dev-certs/src/Program.cs | 3 +++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 0bb05da2aea1..771505cb2f5d 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -346,7 +346,14 @@ public EnsureCertificateResult EnsureAspNetCoreHttpsDevelopmentCertificate( return result; } - result = EnsureCertificateResult.ExistingHttpsCertificateTrusted; + if (result == EnsureCertificateResult.ValidCertificatePresent) + { + result = EnsureCertificateResult.ExistingHttpsCertificateTrusted; + } + else + { + result = EnsureCertificateResult.NewHttpsCertificateTrusted; + } } DisposeCertificates(!isNewCertificate ? certificates : certificates.Append(certificate)); diff --git a/src/Shared/CertificateGeneration/EnsureCertificateResult.cs b/src/Shared/CertificateGeneration/EnsureCertificateResult.cs index 6ed38b1cdfcf..842b84a3643d 100644 --- a/src/Shared/CertificateGeneration/EnsureCertificateResult.cs +++ b/src/Shared/CertificateGeneration/EnsureCertificateResult.cs @@ -13,6 +13,7 @@ internal enum EnsureCertificateResult FailedToTrustTheCertificate, UserCancelledTrustStep, FailedToMakeKeyAccessible, - ExistingHttpsCertificateTrusted + ExistingHttpsCertificateTrusted, + NewHttpsCertificateTrusted } diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index fb6944e48b79..024c61ccf511 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -433,6 +433,9 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio case EnsureCertificateResult.ExistingHttpsCertificateTrusted: reporter.Output("Successfully trusted the existing HTTPS certificate."); return Success; + case EnsureCertificateResult.NewHttpsCertificateTrusted: + reporter.Output("Successfully created and trusted a new HTTPS certificate."); + return Success; default: reporter.Error("Something went wrong. The HTTPS developer certificate could not be created."); return CriticalError; From df248a96e9a9d8372c64c1a1efbd4046a0f7275b Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Fri, 17 Jun 2022 16:48:49 -0700 Subject: [PATCH 07/22] Feedback. --- .../CertificateGeneration/CertificateManager.cs | 1 - .../MacOSCertificateManager.cs | 15 ++++----------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 771505cb2f5d..1ac8c2390a0a 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -781,7 +781,6 @@ protected virtual void RemoveCertificateFromUserStoreCore(X509Certificate2 certi .Single(c => c.SerialNumber == certificate.SerialNumber); store.Remove(matching); - store.Close(); } internal static string ToCertificateDescription(IEnumerable certificates) diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index 814923556002..dfd6d2ed134a 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -86,10 +86,7 @@ protected override void TrustCertificateCore(X509Certificate2 publicCertificate) { try { - if (File.Exists(tmpFile)) - { - File.Delete(tmpFile); - } + File.Delete(tmpFile); } catch { @@ -169,10 +166,7 @@ public override bool IsTrusted(X509Certificate2 certificate) } finally { - if (File.Exists(tmpFile)) - { - File.Delete(tmpFile); - } + File.Delete(tmpFile); } } @@ -251,7 +245,7 @@ protected override X509Certificate2 SaveCertificateCore(X509Certificate2 certifi return certificate; } - private static bool SaveCertificateToUserKeychain(X509Certificate2 certificate) + private static void SaveCertificateToUserKeychain(X509Certificate2 certificate) { var passwordBytes = new byte[48]; RandomNumberGenerator.Fill(passwordBytes.AsSpan()[0..35]); @@ -262,7 +256,7 @@ private static bool SaveCertificateToUserKeychain(X509Certificate2 certificate) var processInfo = new ProcessStartInfo( MacOSAddCertificateToKeyChainCommandLine, - string.Format(CultureInfo.InvariantCulture,MacOSAddCertificateToKeyChainCommandLineArgumentsFormat, certificatePath, password)) + string.Format(CultureInfo.InvariantCulture, MacOSAddCertificateToKeyChainCommandLineArgumentsFormat, certificatePath, password)) { RedirectStandardOutput = true, RedirectStandardError = true @@ -286,7 +280,6 @@ private static bool SaveCertificateToUserKeychain(X509Certificate2 certificate) } Log.MacOSAddCertificateToKeyChainEnd(); - return true; } private static string GetCertificateFilePath(X509Certificate2 certificate) => From dc5e8cf53e8f3166c07c51f11a709229ad20d817 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Fri, 17 Jun 2022 17:02:49 -0700 Subject: [PATCH 08/22] Add warning about changing exit codes. --- src/Tools/dotnet-dev-certs/src/Program.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index 024c61ccf511..cc2ac7c7b37c 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -15,6 +15,8 @@ namespace Microsoft.AspNetCore.DeveloperCertificates.Tools; internal sealed class Program { + // NOTE: Exercise caution when touching these exit codes, since existing tooling + // might depend on some of these values. private const int CriticalError = -1; private const int Success = 0; private const int ErrorCreatingTheCertificate = 1; From 0ff62c45d713bb760e5ddfbe1cf84fc427cea8d2 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Mon, 20 Jun 2022 18:28:05 -0700 Subject: [PATCH 09/22] Handle torn state cases. --- .../CertificateManager.cs | 6 + .../MacOSCertificateManager.cs | 212 +++++++++++++++--- 2 files changed, 193 insertions(+), 25 deletions(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 1ac8c2390a0a..0976ff0abcb9 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -1001,6 +1001,12 @@ public sealed class CertificateManagerEventSource : EventSource [Event(70, Level = EventLevel.Error, Message = "The file '{0}' is not a valid certificate.")] internal void MacOSFileIsNotAValidCertificate(string path) => WriteEvent(70, path); + + [Event(71, Level = EventLevel.Warning, Message = "The on-disk store and the keychain are not in a consistent state. This might occur if you've used an older version of this tool in the past. Running with the --clean argument will remove the development certificates from both locations and start clean.")] + internal void MacOSDiskAndKeychainInconsistent() => WriteEvent(71); + + [Event(72, Level = EventLevel.Warning, Message = "The on-disk store directory was not found.")] + internal void MacOSDiskStoreDoesNotExist() => WriteEvent(72); } internal sealed class UserCancelledTrustException : Exception diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index dfd6d2ed134a..a820a0cc8a2e 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -3,14 +3,24 @@ using System.Diagnostics; using System.Globalization; +using System.Linq; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using System.Text.RegularExpressions; namespace Microsoft.AspNetCore.Certificates.Generation; internal sealed class MacOSCertificateManager : CertificateManager { - private static readonly string MacOSUserKeyChain = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "/Library/Keychains/login.keychain-db"; + private static readonly string MacOSUserKeychain = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "/Library/Keychains/login.keychain-db"; + + // System keychain. We no longer store certificates or create trust rules in the system + // keychain, but check for their presence here so that we can clean up state left behind + // by pre-.NET 7 versions of this tool. + private const string MacOSSystemKeychain = "/Library/Keychains/System.keychain"; + + // Well-known location on disk where dev-certs are stored. + private static readonly string MacOSUserHttpsCertificateLocation = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".aspnet", "dev-certs", "https"); // Verify the certificate {0} for the SSL and X.509 Basic Policy. private const string MacOSVerifyCertificateCommandLine = "security"; @@ -22,17 +32,31 @@ internal sealed class MacOSCertificateManager : CertificateManager // Add a certificate to the per-user trust settings in the user keychain. The trust policy // for the certificate will be set to be always trusted for SSL and X.509 Basic Policy. - // Note: This operation will require user authentication via a dialog. + // Note: This operation will require user authentication. private const string MacOSTrustCertificateCommandLine = "security"; - private static readonly string MacOSTrustCertificateCommandLineArguments = $"add-trusted-cert -p basic -p ssl -k {MacOSUserKeyChain} "; - - // Well-known location on disk where dev-certs are stored. - private static readonly string MacOSUserHttpsCertificateLocation = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".aspnet", "dev-certs", "https"); + private static readonly string MacOSTrustCertificateCommandLineArguments = $"add-trusted-cert -p basic -p ssl -k {MacOSUserKeychain} "; // Import a pkcs12 certificate into the user keychain using the unwrapping passphrase {1}, and // allow any application to access the imported key without warning. private const string MacOSAddCertificateToKeyChainCommandLine = "security"; - private static readonly string MacOSAddCertificateToKeyChainCommandLineArgumentsFormat = "import {0} -k " + MacOSUserKeyChain + " -t cert -f pkcs12 -P {1} -A"; + private static readonly string MacOSAddCertificateToKeyChainCommandLineArgumentsFormat = "import {0} -k " + MacOSUserKeychain + " -t cert -f pkcs12 -P {1} -A"; + + // Remove a certificate from the admin trust settings. We no longer add certificates to the + // admin trust settings, but need this for cleaning up certs generated by pre-.NET 7 versions + // of this tool that used to create trust settings in the system keychain. + // Note: This operation will require user authentication. + private const string MacOSUntrustLegacyCertificateCommandLine = "sudo"; + private const string MacOSUntrustLegacyCertificateCommandLineArguments = "security remove-trusted-cert -d {0}"; + + // Find all matching certificates on the system keychain that have the name {0} and + // print their SHA-256 and SHA-1 hashes. + // We no longer store things in the system keychain but need this for identifying certs + // generated by pre-.NET 7 versions of this tool that used to store certs there. + private const string MacOSFindCertificateOnSystemKeychainCommandLine = "security"; + private const string MacOSFindCertificateOnSystemKeychainCommandLineArgumentsFormat = "find-certificate -c {0} -a -Z -p " + MacOSSystemKeychain; + + // Format used by the tool when printing SHA-1 hashes. + private const string MacOSFindCertificateOutputRegex = "SHA-1 hash: ([0-9A-Z]+)"; public const string InvalidCertificateState = "The ASP.NET Core developer certificate is in an invalid state. " + @@ -170,12 +194,97 @@ public override bool IsTrusted(X509Certificate2 certificate) } } + private static bool IsOnSystemKeychain(X509Certificate2 certificate) + { + TimeSpan MaxRegexTimeout = TimeSpan.FromMinutes(1); + const string CertificateSubjectRegex = "CN=(.*[^,]+).*"; + + var subjectMatch = Regex.Match(certificate.Subject, CertificateSubjectRegex, RegexOptions.Singleline, MaxRegexTimeout); + if (!subjectMatch.Success) + { + throw new InvalidOperationException($"Can't determine the subject for the certificate with subject '{certificate.Subject}'."); + } + + var subject = subjectMatch.Groups[1].Value; + + // Run the find-certificate command, and look for the cert's hash in the output + using var findCertificateProcess = Process.Start(new ProcessStartInfo( + MacOSFindCertificateOnSystemKeychainCommandLine, + string.Format(CultureInfo.InvariantCulture, MacOSFindCertificateOnSystemKeychainCommandLineArgumentsFormat, subject)) + { + RedirectStandardOutput = true + }); + + var output = findCertificateProcess!.StandardOutput.ReadToEnd(); + findCertificateProcess.WaitForExit(); + + var matches = Regex.Matches(output, MacOSFindCertificateOutputRegex, RegexOptions.Multiline, MaxRegexTimeout); + var hashes = matches.OfType().Select(m => m.Groups[1].Value).ToList(); + + return hashes.Any(h => string.Equals(h, certificate.Thumbprint, StringComparison.Ordinal)); + } + protected override void RemoveCertificateFromTrustedRoots(X509Certificate2 certificate) { - RemoveCertificateFromKeyChain(MacOSUserKeyChain, certificate); + if (IsOnSystemKeychain(certificate)) + { + // Pre-.NET 7 versions of this tool used to store certs and trust settings on the + // system keychain. Check if that's the case for this cert, and if so, remove the + // trust rule and the cert from the system keychain. + try + { + RemoveAdminTrustRule(certificate); + RemoveCertificateFromKeyChain(MacOSSystemKeychain, certificate); + } + catch + { + } + } + + RemoveCertificateFromKeyChain(MacOSUserKeychain, certificate); RemoveCertificateFromUserStoreCore(certificate); } + // Remove the certificate from the admin trust settings. + private static void RemoveAdminTrustRule(X509Certificate2 certificate) + { + Log.MacOSRemoveCertificateTrustRuleStart(GetDescription(certificate)); + var certificatePath = Path.GetTempFileName(); + try + { + var certBytes = certificate.Export(X509ContentType.Cert); + File.WriteAllBytes(certificatePath, certBytes); + var processInfo = new ProcessStartInfo( + MacOSUntrustLegacyCertificateCommandLine, + string.Format( + CultureInfo.InvariantCulture, + MacOSUntrustLegacyCertificateCommandLineArguments, + certificatePath + )); + + using var process = Process.Start(processInfo); + process!.WaitForExit(); + + if (process.ExitCode != 0) + { + Log.MacOSRemoveCertificateTrustRuleError(process.ExitCode); + } + + Log.MacOSRemoveCertificateTrustRuleEnd(); + } + finally + { + try + { + File.Delete(certificatePath); + } + catch + { + // We don't care if we can't delete the temp file. + } + } + } + private static void RemoveCertificateFromKeyChain(string keyChain, X509Certificate2 certificate) { var processInfo = new ProcessStartInfo( @@ -226,7 +335,7 @@ protected override X509Certificate2 SaveCertificateCore(X509Certificate2 certifi if (Log.IsEnabled()) { - Log.MacOSAddCertificateToUserProfileDirStart(MacOSUserKeyChain, GetDescription(certificate)); + Log.MacOSAddCertificateToUserProfileDirStart(MacOSUserKeychain, GetDescription(certificate)); } // Ensure that the directory exists before writing to the file. @@ -264,7 +373,7 @@ private static void SaveCertificateToUserKeychain(X509Certificate2 certificate) if (Log.IsEnabled()) { - Log.MacOSAddCertificateToKeyChainStart(MacOSUserKeyChain, GetDescription(certificate)); + Log.MacOSAddCertificateToKeyChainStart(MacOSUserKeychain, GetDescription(certificate)); } using (var process = Process.Start(processInfo)) @@ -292,31 +401,84 @@ protected override IList GetCertificatesToRemove(StoreName sto protected override void PopulateCertificatesFromStore(X509Store store, List certificates) { - if (store.Name! != StoreName.My.ToString() || store.Location != StoreLocation.CurrentUser) + bool useDiskStore = store.Name! == StoreName.My.ToString() && store.Location == StoreLocation.CurrentUser; + bool augmentWithCertsFromKeychain = false; + + if (useDiskStore) + { + var certsFromDisk = GetCertsFromDisk(); + certificates.AddRange(certsFromDisk); + + if (!CheckKeychainAndDiskConsistency(store)) + { + Log.MacOSDiskAndKeychainInconsistent(); + augmentWithCertsFromKeychain = true; + } + } + + // We may want to augment the certs we found on disk with the ones that are only in the + // keychain. We can end up in this state if previous versions of this tool left behind + // some state in the keychain. + if (!useDiskStore || augmentWithCertsFromKeychain) + { + var certsFromStore = new List(); + base.PopulateCertificatesFromStore(store, certsFromStore); + + foreach (var cert in certsFromStore) + { + if (!certificates.Contains(cert)) + { + certificates.Add(cert); + } + } + } + } + + private static ICollection GetCertsFromDisk() + { + var certsFromDisk = new List(); + if (!Directory.Exists(MacOSUserHttpsCertificateLocation)) { - base.PopulateCertificatesFromStore(store, certificates); + Log.MacOSDiskStoreDoesNotExist(); } else { - if (Directory.Exists(MacOSUserHttpsCertificateLocation)) + var certificateFiles = Directory.EnumerateFiles(MacOSUserHttpsCertificateLocation, "aspnetcore-localhost-*.pfx"); + foreach (var file in certificateFiles) { - var certificateFiles = Directory.EnumerateFiles(MacOSUserHttpsCertificateLocation, "aspnetcore-localhost-*.pfx"); - foreach (var file in certificateFiles) + try { - try - { - var certificate = new X509Certificate2(file); - certificates.Add(certificate); - } - catch (Exception) - { - Log.MacOSFileIsNotAValidCertificate(file); - throw; - } + var certificate = new X509Certificate2(file); + certsFromDisk.Add(certificate); + } + catch (Exception) + { + Log.MacOSFileIsNotAValidCertificate(file); + throw; } } + } + + return certsFromDisk; + } + // Ensure that the on-disk store and the keychain have the same set of certificates. + // We might be in a torn state if prior versions of this tool left certs behind on the + // keychain. + private bool CheckKeychainAndDiskConsistency(X509Store store) + { + var certsFromDisk = GetCertsFromDisk(); + + var certsFromStore = new List(); + base.PopulateCertificatesFromStore(store, certsFromStore); + + if (certsFromStore.Except(certsFromDisk).Any() || certsFromDisk.Except(certsFromStore).Any()) + { + Log.MacOSDiskAndKeychainInconsistent(); + return false; } + + return true; } protected override void RemoveCertificateFromUserStoreCore(X509Certificate2 certificate) From 77b079c0e494998fecb8cf6d1825ca5cc3cc6bc1 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Mon, 20 Jun 2022 19:02:13 -0700 Subject: [PATCH 10/22] Make --check also check for consistent state. --- src/Shared/CertificateGeneration/CertificateManager.cs | 6 ++++++ .../CertificateGeneration/MacOSCertificateManager.cs | 7 +++++++ src/Tools/dotnet-dev-certs/src/Program.cs | 10 ++++++++++ 3 files changed, 23 insertions(+) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 0976ff0abcb9..5985f889c873 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -66,6 +66,12 @@ public static bool IsHttpsDevelopmentCertificate(X509Certificate2 certificate) = certificate.Extensions.OfType() .Any(e => string.Equals(AspNetHttpsOid, e.Oid?.Value, StringComparison.Ordinal)); + // Check whether the world is in a consistent state or some cleanup is necessary. + public virtual bool IsSystemStateConsistent(StoreName storeName, StoreLocation location) + { + return true; + } + public IList ListCertificates( StoreName storeName, StoreLocation location, diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index a820a0cc8a2e..702047182753 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -399,6 +399,13 @@ protected override IList GetCertificatesToRemove(StoreName sto return ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: false); } + public override bool IsSystemStateConsistent(StoreName storeName, StoreLocation location) + { + using var store = new X509Store(storeName, location); + store.Open(OpenFlags.ReadOnly); + return CheckKeychainAndDiskConsistency(store); + } + protected override void PopulateCertificatesFromStore(X509Store store, List certificates) { bool useDiskStore = store.Name! == StoreName.My.ToString() && store.Location == StoreLocation.CurrentUser; diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index cc2ac7c7b37c..968af46ac62b 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -272,6 +272,16 @@ private static int CleanHttpsCertificates(IReporter reporter) private static int CheckHttpsCertificate(CommandOption trust, IReporter reporter) { var certificateManager = CertificateManager.Instance; + + if (!certificateManager.IsSystemStateConsistent(StoreName.My, StoreLocation.CurrentUser)) + { + reporter.Output("HTTPS development certificates are in an inconsistent state. This might occur if an older " + + "version of this tool was used in the past and left behind some certificates. "+ + "Running with the --clean argument will attempt to clean up the certificate state. " + + "Running with --verbose might help diagnose the issue."); + return InvalidCertificateState; + } + var certificates = certificateManager.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: true); var validCertificates = new List(); if (certificates.Count == 0) From 7a835ad8a4598d081b687a1fcaa3257315d8fb81 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Tue, 21 Jun 2022 18:46:24 -0700 Subject: [PATCH 11/22] Rev certificate version. --- src/Shared/CertificateGeneration/CertificateManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 5985f889c873..0ee52e3f5813 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Certificates.Generation; internal abstract class CertificateManager { - internal const int CurrentAspNetCoreCertificateVersion = 2; + internal const int CurrentAspNetCoreCertificateVersion = 3; internal const string AspNetHttpsOid = "1.3.6.1.4.1.311.84.1.1"; internal const string AspNetHttpsOidFriendlyName = "ASP.NET Core HTTPS development certificate"; From 6929323f0223e2908287f6f82b89a37773027305 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Wed, 22 Jun 2022 11:41:22 -0700 Subject: [PATCH 12/22] Revert the changes around handling the old cert model. --- .../CertificateManager.cs | 16 +- .../MacOSCertificateManager.cs | 164 +----------------- src/Tools/dotnet-dev-certs/src/Program.cs | 13 -- 3 files changed, 6 insertions(+), 187 deletions(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 0ee52e3f5813..9179d7042807 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -1,12 +1,9 @@ // 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; using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Tracing; -using System.IO; using System.Linq; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; @@ -66,12 +63,6 @@ public static bool IsHttpsDevelopmentCertificate(X509Certificate2 certificate) = certificate.Extensions.OfType() .Any(e => string.Equals(AspNetHttpsOid, e.Oid?.Value, StringComparison.Ordinal)); - // Check whether the world is in a consistent state or some cleanup is necessary. - public virtual bool IsSystemStateConsistent(StoreName storeName, StoreLocation location) - { - return true; - } - public IList ListCertificates( StoreName storeName, StoreLocation location, @@ -1008,11 +999,8 @@ public sealed class CertificateManagerEventSource : EventSource [Event(70, Level = EventLevel.Error, Message = "The file '{0}' is not a valid certificate.")] internal void MacOSFileIsNotAValidCertificate(string path) => WriteEvent(70, path); - [Event(71, Level = EventLevel.Warning, Message = "The on-disk store and the keychain are not in a consistent state. This might occur if you've used an older version of this tool in the past. Running with the --clean argument will remove the development certificates from both locations and start clean.")] - internal void MacOSDiskAndKeychainInconsistent() => WriteEvent(71); - - [Event(72, Level = EventLevel.Warning, Message = "The on-disk store directory was not found.")] - internal void MacOSDiskStoreDoesNotExist() => WriteEvent(72); + [Event(71, Level = EventLevel.Warning, Message = "The on-disk store directory was not found.")] + internal void MacOSDiskStoreDoesNotExist() => WriteEvent(71); } internal sealed class UserCancelledTrustException : Exception diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index 702047182753..e188244e1944 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -3,10 +3,8 @@ using System.Diagnostics; using System.Globalization; -using System.Linq; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; -using System.Text.RegularExpressions; namespace Microsoft.AspNetCore.Certificates.Generation; @@ -14,11 +12,6 @@ internal sealed class MacOSCertificateManager : CertificateManager { private static readonly string MacOSUserKeychain = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "/Library/Keychains/login.keychain-db"; - // System keychain. We no longer store certificates or create trust rules in the system - // keychain, but check for their presence here so that we can clean up state left behind - // by pre-.NET 7 versions of this tool. - private const string MacOSSystemKeychain = "/Library/Keychains/System.keychain"; - // Well-known location on disk where dev-certs are stored. private static readonly string MacOSUserHttpsCertificateLocation = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".aspnet", "dev-certs", "https"); @@ -41,23 +34,6 @@ internal sealed class MacOSCertificateManager : CertificateManager private const string MacOSAddCertificateToKeyChainCommandLine = "security"; private static readonly string MacOSAddCertificateToKeyChainCommandLineArgumentsFormat = "import {0} -k " + MacOSUserKeychain + " -t cert -f pkcs12 -P {1} -A"; - // Remove a certificate from the admin trust settings. We no longer add certificates to the - // admin trust settings, but need this for cleaning up certs generated by pre-.NET 7 versions - // of this tool that used to create trust settings in the system keychain. - // Note: This operation will require user authentication. - private const string MacOSUntrustLegacyCertificateCommandLine = "sudo"; - private const string MacOSUntrustLegacyCertificateCommandLineArguments = "security remove-trusted-cert -d {0}"; - - // Find all matching certificates on the system keychain that have the name {0} and - // print their SHA-256 and SHA-1 hashes. - // We no longer store things in the system keychain but need this for identifying certs - // generated by pre-.NET 7 versions of this tool that used to store certs there. - private const string MacOSFindCertificateOnSystemKeychainCommandLine = "security"; - private const string MacOSFindCertificateOnSystemKeychainCommandLineArgumentsFormat = "find-certificate -c {0} -a -Z -p " + MacOSSystemKeychain; - - // Format used by the tool when printing SHA-1 hashes. - private const string MacOSFindCertificateOutputRegex = "SHA-1 hash: ([0-9A-Z]+)"; - public const string InvalidCertificateState = "The ASP.NET Core developer certificate is in an invalid state. " + "To fix this issue, run 'dotnet dev-certs https --clean' and 'dotnet dev-certs https' " + @@ -194,97 +170,12 @@ public override bool IsTrusted(X509Certificate2 certificate) } } - private static bool IsOnSystemKeychain(X509Certificate2 certificate) - { - TimeSpan MaxRegexTimeout = TimeSpan.FromMinutes(1); - const string CertificateSubjectRegex = "CN=(.*[^,]+).*"; - - var subjectMatch = Regex.Match(certificate.Subject, CertificateSubjectRegex, RegexOptions.Singleline, MaxRegexTimeout); - if (!subjectMatch.Success) - { - throw new InvalidOperationException($"Can't determine the subject for the certificate with subject '{certificate.Subject}'."); - } - - var subject = subjectMatch.Groups[1].Value; - - // Run the find-certificate command, and look for the cert's hash in the output - using var findCertificateProcess = Process.Start(new ProcessStartInfo( - MacOSFindCertificateOnSystemKeychainCommandLine, - string.Format(CultureInfo.InvariantCulture, MacOSFindCertificateOnSystemKeychainCommandLineArgumentsFormat, subject)) - { - RedirectStandardOutput = true - }); - - var output = findCertificateProcess!.StandardOutput.ReadToEnd(); - findCertificateProcess.WaitForExit(); - - var matches = Regex.Matches(output, MacOSFindCertificateOutputRegex, RegexOptions.Multiline, MaxRegexTimeout); - var hashes = matches.OfType().Select(m => m.Groups[1].Value).ToList(); - - return hashes.Any(h => string.Equals(h, certificate.Thumbprint, StringComparison.Ordinal)); - } - protected override void RemoveCertificateFromTrustedRoots(X509Certificate2 certificate) { - if (IsOnSystemKeychain(certificate)) - { - // Pre-.NET 7 versions of this tool used to store certs and trust settings on the - // system keychain. Check if that's the case for this cert, and if so, remove the - // trust rule and the cert from the system keychain. - try - { - RemoveAdminTrustRule(certificate); - RemoveCertificateFromKeyChain(MacOSSystemKeychain, certificate); - } - catch - { - } - } - RemoveCertificateFromKeyChain(MacOSUserKeychain, certificate); RemoveCertificateFromUserStoreCore(certificate); } - // Remove the certificate from the admin trust settings. - private static void RemoveAdminTrustRule(X509Certificate2 certificate) - { - Log.MacOSRemoveCertificateTrustRuleStart(GetDescription(certificate)); - var certificatePath = Path.GetTempFileName(); - try - { - var certBytes = certificate.Export(X509ContentType.Cert); - File.WriteAllBytes(certificatePath, certBytes); - var processInfo = new ProcessStartInfo( - MacOSUntrustLegacyCertificateCommandLine, - string.Format( - CultureInfo.InvariantCulture, - MacOSUntrustLegacyCertificateCommandLineArguments, - certificatePath - )); - - using var process = Process.Start(processInfo); - process!.WaitForExit(); - - if (process.ExitCode != 0) - { - Log.MacOSRemoveCertificateTrustRuleError(process.ExitCode); - } - - Log.MacOSRemoveCertificateTrustRuleEnd(); - } - finally - { - try - { - File.Delete(certificatePath); - } - catch - { - // We don't care if we can't delete the temp file. - } - } - } - private static void RemoveCertificateFromKeyChain(string keyChain, X509Certificate2 certificate) { var processInfo = new ProcessStartInfo( @@ -399,45 +290,17 @@ protected override IList GetCertificatesToRemove(StoreName sto return ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: false); } - public override bool IsSystemStateConsistent(StoreName storeName, StoreLocation location) - { - using var store = new X509Store(storeName, location); - store.Open(OpenFlags.ReadOnly); - return CheckKeychainAndDiskConsistency(store); - } - protected override void PopulateCertificatesFromStore(X509Store store, List certificates) { bool useDiskStore = store.Name! == StoreName.My.ToString() && store.Location == StoreLocation.CurrentUser; - bool augmentWithCertsFromKeychain = false; - + if (useDiskStore) { - var certsFromDisk = GetCertsFromDisk(); - certificates.AddRange(certsFromDisk); - - if (!CheckKeychainAndDiskConsistency(store)) - { - Log.MacOSDiskAndKeychainInconsistent(); - augmentWithCertsFromKeychain = true; - } + certificates.AddRange(GetCertsFromDisk()); } - - // We may want to augment the certs we found on disk with the ones that are only in the - // keychain. We can end up in this state if previous versions of this tool left behind - // some state in the keychain. - if (!useDiskStore || augmentWithCertsFromKeychain) + else { - var certsFromStore = new List(); - base.PopulateCertificatesFromStore(store, certsFromStore); - - foreach (var cert in certsFromStore) - { - if (!certificates.Contains(cert)) - { - certificates.Add(cert); - } - } + base.PopulateCertificatesFromStore(store, certificates); } } @@ -469,25 +332,6 @@ private static ICollection GetCertsFromDisk() return certsFromDisk; } - // Ensure that the on-disk store and the keychain have the same set of certificates. - // We might be in a torn state if prior versions of this tool left certs behind on the - // keychain. - private bool CheckKeychainAndDiskConsistency(X509Store store) - { - var certsFromDisk = GetCertsFromDisk(); - - var certsFromStore = new List(); - base.PopulateCertificatesFromStore(store, certsFromStore); - - if (certsFromStore.Except(certsFromDisk).Any() || certsFromDisk.Except(certsFromStore).Any()) - { - Log.MacOSDiskAndKeychainInconsistent(); - return false; - } - - return true; - } - protected override void RemoveCertificateFromUserStoreCore(X509Certificate2 certificate) { try diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index 968af46ac62b..e94a52a423fc 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -1,9 +1,6 @@ // 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.IO; using System.Linq; using System.Runtime.InteropServices; using System.Security.Cryptography.X509Certificates; @@ -272,16 +269,6 @@ private static int CleanHttpsCertificates(IReporter reporter) private static int CheckHttpsCertificate(CommandOption trust, IReporter reporter) { var certificateManager = CertificateManager.Instance; - - if (!certificateManager.IsSystemStateConsistent(StoreName.My, StoreLocation.CurrentUser)) - { - reporter.Output("HTTPS development certificates are in an inconsistent state. This might occur if an older " + - "version of this tool was used in the past and left behind some certificates. "+ - "Running with the --clean argument will attempt to clean up the certificate state. " + - "Running with --verbose might help diagnose the issue."); - return InvalidCertificateState; - } - var certificates = certificateManager.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: true); var validCertificates = new List(); if (certificates.Count == 0) From b5f447e9ff7fa7ad265508ff57484c945b8b215b Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Wed, 22 Jun 2022 13:57:59 -0700 Subject: [PATCH 13/22] Update OID for dev cert. --- src/Shared/CertificateGeneration/CertificateManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 9179d7042807..97d5189361c9 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Certificates.Generation; internal abstract class CertificateManager { internal const int CurrentAspNetCoreCertificateVersion = 3; - internal const string AspNetHttpsOid = "1.3.6.1.4.1.311.84.1.1"; + internal const string AspNetHttpsOid = "1.3.6.1.4.1.311.84.1.3"; internal const string AspNetHttpsOidFriendlyName = "ASP.NET Core HTTPS development certificate"; private const string ServerAuthenticationEnhancedKeyUsageOid = "1.3.6.1.5.5.7.3.1"; From d51185084b54b87efb51de51c14f5b0ba3bb3943 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 23 Jun 2022 15:32:07 -0700 Subject: [PATCH 14/22] Make OID change on macOS only. --- .../CertificateManager.cs | 26 ++++++++++++++++++- .../test/CertificateManagerTests.cs | 13 +++++++--- src/Tools/dotnet-dev-certs/src/Program.cs | 7 ++++- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 97d5189361c9..bae909a02adf 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -16,7 +16,31 @@ namespace Microsoft.AspNetCore.Certificates.Generation; internal abstract class CertificateManager { internal const int CurrentAspNetCoreCertificateVersion = 3; - internal const string AspNetHttpsOid = "1.3.6.1.4.1.311.84.1.3"; + + // + // OIDs used for HTTPS certs + // + // Why do we have two different OIDs? + // + // As part of .NET 7 changes to how the dev-certs tool works on macOS, we needed + // to rev the OID for the HTTPS cert. Changing the OID means that apps expecting + // the older OID will not recognize the newer cert. Therefore, HTTPS certificate + // operations (creation, trust, etc.) for older apps will need to be done using + // an older, pre-change dev-certs tool (found in .NET 6 and earlier SDKs). + // + // Clearly, changing the OID causes some friction for people who work on multiple + // projects targeting pre- and post-change .NET versions. In order to reduce the + // blast radius of this change, we only use the new OID on macOS for now; other + // OS's will not be impacted by this change. If, in the future, there's some + // reason to change the OID version again, we might be able to reunify the OS's. + + // This was the OID used on all OS's pre-.NET 7 + internal const string AspNetHttpsOid1 = "1.3.6.1.4.1.311.84.1.1"; + + // This is a new OID only used on macOS for .NET 7+ + internal const string AspNetHttpsOid2 = "1.3.6.1.4.1.311.84.1.3"; + internal static readonly string AspNetHttpsOid = OperatingSystem.IsMacOS() ? AspNetHttpsOid2 : AspNetHttpsOid1; + internal const string AspNetHttpsOidFriendlyName = "ASP.NET Core HTTPS development certificate"; private const string ServerAuthenticationEnhancedKeyUsageOid = "1.3.6.1.5.5.7.3.1"; diff --git a/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs b/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs index f5970d8a1197..8ce5cc6b4145 100644 --- a/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs +++ b/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs @@ -25,7 +25,12 @@ public CertificateManagerTests(ITestOutputHelper output, CertFixture fixture) Output = output; } - public const string TestCertificateSubject = "CN=aspnet.test"; + private const string TestCertificateSubject = "CN=aspnet.test"; + + // See comment near the definition of AspNetHttpsOid in CertificateManager. + private const string AspNetHttpsOid1 = "1.3.6.1.4.1.311.84.1.1"; + private const string AspNetHttpsOid2 = "1.3.6.1.4.1.311.84.1.3"; + private static readonly string AspNetHttpsOid = OperatingSystem.IsMacOS() ? AspNetHttpsOid2 : AspNetHttpsOid1; public ITestOutputHelper Output { get; } @@ -94,7 +99,7 @@ public void EnsureCreateHttpsCertificate_CreatesACertificate_WhenThereAreNoHttps Assert.Contains( httpsCertificate.Extensions.OfType(), e => e.Critical == false && - e.Oid.Value == "1.3.6.1.4.1.311.84.1.1" && + e.Oid.Value == AspNetHttpsOid && e.RawData[0] == _manager.AspNetHttpsCertificateVersion); Assert.Equal(httpsCertificate.GetCertHashString(), exportedCertificate.GetCertHashString()); @@ -409,13 +414,13 @@ public void ListCertificates_AlwaysReturnsTheCertificate_WithHighestVersion() Assert.Contains( firstCertificate.Extensions.OfType(), e => e.Critical == false && - e.Oid.Value == "1.3.6.1.4.1.311.84.1.1" && + e.Oid.Value == AspNetHttpsOid && e.RawData[0] == 2); Assert.Contains( secondCertificate.Extensions.OfType(), e => e.Critical == false && - e.Oid.Value == "1.3.6.1.4.1.311.84.1.1" && + e.Oid.Value == AspNetHttpsOid && e.RawData[0] == 1); } } diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index e94a52a423fc..a26543ab7127 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -366,7 +366,12 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio "already trusted we will run the following command:" + Environment.NewLine + "'security add-trusted-cert -p basic -p ssl -k <> <>'" + Environment.NewLine + "This command might prompt you for your password to install the certificate " + - "on the keychain. To undo these changes: 'security remove-trusted-cert <>'"); + "on the keychain. To undo these changes: 'security remove-trusted-cert <>'" + Environment.NewLine); + + // See comment near the definition of AspNetHttpsOid in CertificateManager. + reporter.Warn("NOTE: If your app is targeting a version of .NET earlier than .NET 7, this version of " + + "the dev-certs tool will not install the correct certificate for your app. Please use the dev-certs " + + "tool from the .NET 6 SDK or the SDK version that matches your app instead."); } if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) From 42e9f11975c5cd4258488b8e87458718d35e8dda Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 23 Jun 2022 15:59:13 -0700 Subject: [PATCH 15/22] Revert version change that's no longer needed. --- src/Shared/CertificateGeneration/CertificateManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index bae909a02adf..0cf95b382fa6 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Certificates.Generation; internal abstract class CertificateManager { - internal const int CurrentAspNetCoreCertificateVersion = 3; + internal const int CurrentAspNetCoreCertificateVersion = 2; // // OIDs used for HTTPS certs From a96c04681f04aee73370c5c13436fdf232bc12e0 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 23 Jun 2022 16:47:22 -0700 Subject: [PATCH 16/22] Show warning on https, trust, and check commands. --- src/Tools/dotnet-dev-certs/src/Program.cs | 24 ++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index a26543ab7127..82415f69a61a 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -323,6 +323,12 @@ private static int CheckHttpsCertificate(CommandOption trust, IReporter reporter } } + if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + // Warn users here so they know that a successful --check doesn't mean that apps using the old OID will work. + PrintOidChangeWarning(reporter); + } + return Success; } @@ -367,11 +373,6 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio "'security add-trusted-cert -p basic -p ssl -k <> <>'" + Environment.NewLine + "This command might prompt you for your password to install the certificate " + "on the keychain. To undo these changes: 'security remove-trusted-cert <>'" + Environment.NewLine); - - // See comment near the definition of AspNetHttpsOid in CertificateManager. - reporter.Warn("NOTE: If your app is targeting a version of .NET earlier than .NET 7, this version of " - + "the dev-certs tool will not install the correct certificate for your app. Please use the dev-certs " - + "tool from the .NET 6 SDK or the SDK version that matches your app instead."); } if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) @@ -403,6 +404,11 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio password.Value(), exportFormat.HasValue() ? format : CertificateKeyExportFormat.Pfx); + if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) + { + PrintOidChangeWarning(reporter); + } + switch (result) { case EnsureCertificateResult.Succeeded: @@ -445,4 +451,12 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio return CriticalError; } } + + private static void PrintOidChangeWarning(IReporter reporter) + { + // See comment near the definition of AspNetHttpsOid in CertificateManager. + reporter.Warn("NOTE: If your app is targeting a version of .NET earlier than .NET 7, this version of " + + "the dev-certs tool will not install/check for the correct certificate for your app. Please use " + + "the dev-certs tool from the .NET 6 SDK or the SDK version that matches your app instead."); + } } From 5c5433436eb104c50107612211a34664d2dfa9b9 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 23 Jun 2022 17:03:12 -0700 Subject: [PATCH 17/22] Add aka.ms link to explainer doc for cert change. --- src/Tools/dotnet-dev-certs/src/Program.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index 82415f69a61a..a80b4c3cf2ec 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -457,6 +457,7 @@ private static void PrintOidChangeWarning(IReporter reporter) // See comment near the definition of AspNetHttpsOid in CertificateManager. reporter.Warn("NOTE: If your app is targeting a version of .NET earlier than .NET 7, this version of " + "the dev-certs tool will not install/check for the correct certificate for your app. Please use " - + "the dev-certs tool from the .NET 6 SDK or the SDK version that matches your app instead."); + + "the dev-certs tool from the .NET 6 SDK or the SDK version that matches your app instead. For " + + "more information, see https://aka.ms/aspnetcore-macos-cert-change".); } } From 58cdbae3d7bc8e50b5b3992be3e1000ef49adeea Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 23 Jun 2022 17:04:47 -0700 Subject: [PATCH 18/22] Oops. --- src/Tools/dotnet-dev-certs/src/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index a80b4c3cf2ec..4df6db626810 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -458,6 +458,6 @@ private static void PrintOidChangeWarning(IReporter reporter) reporter.Warn("NOTE: If your app is targeting a version of .NET earlier than .NET 7, this version of " + "the dev-certs tool will not install/check for the correct certificate for your app. Please use " + "the dev-certs tool from the .NET 6 SDK or the SDK version that matches your app instead. For " - + "more information, see https://aka.ms/aspnetcore-macos-cert-change".); + + "more information, see https://aka.ms/aspnetcore-macos-cert-change"); } } From 14a0f26d36fe5652bb7e8fc8957260452aee0731 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 23 Jun 2022 17:33:29 -0700 Subject: [PATCH 19/22] Feedback. --- .../CertificateGeneration/CertificateManager.cs | 6 +++--- .../test/CertificateManagerTests.cs | 11 +++-------- .../dotnet-dev-certs/src/dotnet-dev-certs.csproj | 4 ++++ 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 0cf95b382fa6..7843c7d758bc 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -35,11 +35,11 @@ internal abstract class CertificateManager // reason to change the OID version again, we might be able to reunify the OS's. // This was the OID used on all OS's pre-.NET 7 - internal const string AspNetHttpsOid1 = "1.3.6.1.4.1.311.84.1.1"; + internal const string AspNetHttpsOid_NotMacOS = "1.3.6.1.4.1.311.84.1.1"; // This is a new OID only used on macOS for .NET 7+ - internal const string AspNetHttpsOid2 = "1.3.6.1.4.1.311.84.1.3"; - internal static readonly string AspNetHttpsOid = OperatingSystem.IsMacOS() ? AspNetHttpsOid2 : AspNetHttpsOid1; + internal const string AspNetHttpsOid_MacOS = "1.3.6.1.4.1.311.84.1.3"; + internal static readonly string AspNetHttpsOid = OperatingSystem.IsMacOS() ? AspNetHttpsOid_MacOS : AspNetHttpsOid_NotMacOS; internal const string AspNetHttpsOidFriendlyName = "ASP.NET Core HTTPS development certificate"; diff --git a/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs b/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs index 8ce5cc6b4145..0dacfb93fffb 100644 --- a/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs +++ b/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs @@ -27,11 +27,6 @@ public CertificateManagerTests(ITestOutputHelper output, CertFixture fixture) private const string TestCertificateSubject = "CN=aspnet.test"; - // See comment near the definition of AspNetHttpsOid in CertificateManager. - private const string AspNetHttpsOid1 = "1.3.6.1.4.1.311.84.1.1"; - private const string AspNetHttpsOid2 = "1.3.6.1.4.1.311.84.1.3"; - private static readonly string AspNetHttpsOid = OperatingSystem.IsMacOS() ? AspNetHttpsOid2 : AspNetHttpsOid1; - public ITestOutputHelper Output { get; } [ConditionalFact] @@ -99,7 +94,7 @@ public void EnsureCreateHttpsCertificate_CreatesACertificate_WhenThereAreNoHttps Assert.Contains( httpsCertificate.Extensions.OfType(), e => e.Critical == false && - e.Oid.Value == AspNetHttpsOid && + e.Oid.Value == CertificateManager.AspNetHttpsOid && e.RawData[0] == _manager.AspNetHttpsCertificateVersion); Assert.Equal(httpsCertificate.GetCertHashString(), exportedCertificate.GetCertHashString()); @@ -414,13 +409,13 @@ public void ListCertificates_AlwaysReturnsTheCertificate_WithHighestVersion() Assert.Contains( firstCertificate.Extensions.OfType(), e => e.Critical == false && - e.Oid.Value == AspNetHttpsOid && + e.Oid.Value == CertificateManager.AspNetHttpsOid && e.RawData[0] == 2); Assert.Contains( secondCertificate.Extensions.OfType(), e => e.Critical == false && - e.Oid.Value == AspNetHttpsOid && + e.Oid.Value == CertificateManager.AspNetHttpsOid && e.RawData[0] == 1); } } diff --git a/src/Tools/dotnet-dev-certs/src/dotnet-dev-certs.csproj b/src/Tools/dotnet-dev-certs/src/dotnet-dev-certs.csproj index 922933eecd39..c644415502cb 100644 --- a/src/Tools/dotnet-dev-certs/src/dotnet-dev-certs.csproj +++ b/src/Tools/dotnet-dev-certs/src/dotnet-dev-certs.csproj @@ -18,4 +18,8 @@ + + + + From 5048398dd10678f582ff9dc8fcccfca30a2b2620 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Thu, 14 Jul 2022 20:40:06 -0700 Subject: [PATCH 20/22] Fix KestrelConfigurationLoaderTests to use new dev cert on macOS. --- .../test/KestrelConfigurationLoaderTests.cs | 7 ++++++- .../test/TestCertificates/aspnetdevcert_macos.pfx | Bin 0 -> 785 bytes 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 src/Servers/Kestrel/shared/test/TestCertificates/aspnetdevcert_macos.pfx diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index 04361234ee14..c5df5c977836 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -220,7 +220,12 @@ public void ConfigureEndpointDevelopmentCertificateGetsLoadedWhenPresent() try { var serverOptions = CreateServerOptions(); - var certificate = new X509Certificate2(TestResources.GetCertPath("aspnetdevcert.pfx"), "testPassword", X509KeyStorageFlags.Exportable); + + // We have two different versions of checked-in dev certs because macOS + // dev certs have a different OID starting in .NET 7. + var devCertFileName = OperatingSystem.IsMacOS() ? "aspnetdevcert_macos.pfx" : "aspnetdevcert.pfx"; + + var certificate = new X509Certificate2(TestResources.GetCertPath(devCertFileName), "testPassword", X509KeyStorageFlags.Exportable); var bytes = certificate.Export(X509ContentType.Pkcs12, "1234"); var path = GetCertificatePath(); Directory.CreateDirectory(Path.GetDirectoryName(path)); diff --git a/src/Servers/Kestrel/shared/test/TestCertificates/aspnetdevcert_macos.pfx b/src/Servers/Kestrel/shared/test/TestCertificates/aspnetdevcert_macos.pfx new file mode 100644 index 0000000000000000000000000000000000000000..be694c0ab9e0bb228157c817f4203e77389b88b1 GIT binary patch literal 785 zcmXqLV&*kyV*I**nTe5!iIbu8+SxA*{2%Wa@Un4gwRyCC=VfH%W@RuCF%&WoU}Fwt zVHW1h$xlwq$;dA*F_07IH8L_VH#9XcHZn7{hyrqrk+?W?H8Cn78_&qfz}&>h&j54= z7gG}>Bg2vURni6D+%`4^?-2WIaBkC%3diIt5*vHP+8B0SkC4wkmOA&fnu1YJNa=~Z z)lR`l7Tz)|U!92dI#_Yo@ZM+5)!!#gs!2Wms9nBp+INBLPdHz^J2D~cYhR4l2iL;g zQ6`T2+*@QEMfdzMZe@S8Xx&QFo; zvv^UA-nmmVrb}r4>(V+qS+?Ut^QAvCX7sxl3a-*wWI0_zsM;e|=5$&SP_yvIU6TIezWy%jKV&l+eV`ODzXJ#}I2Z;-U#CZ+4n~w47J!iq zj0HvpEAAb)(zKf&zOb>gsxoESc3G5T?mwZKt)E&7=e@IK+qX5fW``MN(_9pJltaK=to!>QthZvwqomhOw6`1fjx__l*P-I~svUa~>gcZGQeZ|Sy;Df($X zUu7+x-wP7VyArqV$ibcNT+$9z3mLlRb$MHh?TeUwVBh3xf^Xl>`(4iS?##h6qMO&9 z4>OGl-d?R^tL$BuxOsaGlfduidQA5|EO0nb@8O_Z*ZjRH?L)%R=-s}0O)~B$?cGcp zIRYiKlYaj2VSc*ZB4v_L(t_O27MFN$_g_-7;JG*F@t%9L|1yb*RLh%k-gaC3@Y<&I SYh@CZ!du?{Fn?0$T?zoT8#JW= literal 0 HcmV?d00001 From 0346d22abbf62a9bb9cf97b9956f93c1cf33dfc8 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Mon, 25 Jul 2022 18:49:14 -0700 Subject: [PATCH 21/22] Bring back 6.0 compat, prompt on https when updating state. --- .../Kestrel/Core/src/KestrelServerOptions.cs | 14 +- .../test/KestrelConfigurationLoaderTests.cs | 7 +- .../TestCertificates/aspnetdevcert_macos.pfx | Bin 785 -> 0 bytes .../CertificateManager.cs | 26 +-- .../MacOSCertificateManager.cs | 208 ++++++++++++++---- 5 files changed, 168 insertions(+), 87 deletions(-) delete mode 100644 src/Servers/Kestrel/shared/test/TestCertificates/aspnetdevcert_macos.pfx diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index 84e274202575..317edfdfd14a 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -285,18 +285,10 @@ private void EnsureDefaultCert() Debug.Assert(status.FailureMessage != null, "Status with a failure result must have a message."); logger.DeveloperCertificateFirstRun(status.FailureMessage); - // Now that we've displayed a warning in the logs so that the user gets a notification that a prompt might appear, try - // and access the certificate key, which might trigger a prompt. - status = CertificateManager.Instance.CheckCertificateState(DefaultCertificate, interactive: true); - if (!status.Success) - { - logger.BadDeveloperCertificateState(); - } + // Prevent binding to HTTPS if the certificate is not valid (avoid the prompt) + DefaultCertificate = null; } - - logger.LocatedDevelopmentCertificate(DefaultCertificate); - - if (!CertificateManager.Instance.IsTrusted(DefaultCertificate)) + else if (!CertificateManager.Instance.IsTrusted(DefaultCertificate)) { logger.DeveloperCertificateNotTrusted(); } diff --git a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs index c5df5c977836..04361234ee14 100644 --- a/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs +++ b/src/Servers/Kestrel/Kestrel/test/KestrelConfigurationLoaderTests.cs @@ -220,12 +220,7 @@ public void ConfigureEndpointDevelopmentCertificateGetsLoadedWhenPresent() try { var serverOptions = CreateServerOptions(); - - // We have two different versions of checked-in dev certs because macOS - // dev certs have a different OID starting in .NET 7. - var devCertFileName = OperatingSystem.IsMacOS() ? "aspnetdevcert_macos.pfx" : "aspnetdevcert.pfx"; - - var certificate = new X509Certificate2(TestResources.GetCertPath(devCertFileName), "testPassword", X509KeyStorageFlags.Exportable); + var certificate = new X509Certificate2(TestResources.GetCertPath("aspnetdevcert.pfx"), "testPassword", X509KeyStorageFlags.Exportable); var bytes = certificate.Export(X509ContentType.Pkcs12, "1234"); var path = GetCertificatePath(); Directory.CreateDirectory(Path.GetDirectoryName(path)); diff --git a/src/Servers/Kestrel/shared/test/TestCertificates/aspnetdevcert_macos.pfx b/src/Servers/Kestrel/shared/test/TestCertificates/aspnetdevcert_macos.pfx deleted file mode 100644 index be694c0ab9e0bb228157c817f4203e77389b88b1..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 785 zcmXqLV&*kyV*I**nTe5!iIbu8+SxA*{2%Wa@Un4gwRyCC=VfH%W@RuCF%&WoU}Fwt zVHW1h$xlwq$;dA*F_07IH8L_VH#9XcHZn7{hyrqrk+?W?H8Cn78_&qfz}&>h&j54= z7gG}>Bg2vURni6D+%`4^?-2WIaBkC%3diIt5*vHP+8B0SkC4wkmOA&fnu1YJNa=~Z z)lR`l7Tz)|U!92dI#_Yo@ZM+5)!!#gs!2Wms9nBp+INBLPdHz^J2D~cYhR4l2iL;g zQ6`T2+*@QEMfdzMZe@S8Xx&QFo; zvv^UA-nmmVrb}r4>(V+qS+?Ut^QAvCX7sxl3a-*wWI0_zsM;e|=5$&SP_yvIU6TIezWy%jKV&l+eV`ODzXJ#}I2Z;-U#CZ+4n~w47J!iq zj0HvpEAAb)(zKf&zOb>gsxoESc3G5T?mwZKt)E&7=e@IK+qX5fW``MN(_9pJltaK=to!>QthZvwqomhOw6`1fjx__l*P-I~svUa~>gcZGQeZ|Sy;Df($X zUu7+x-wP7VyArqV$ibcNT+$9z3mLlRb$MHh?TeUwVBh3xf^Xl>`(4iS?##h6qMO&9 z4>OGl-d?R^tL$BuxOsaGlfduidQA5|EO0nb@8O_Z*ZjRH?L)%R=-s}0O)~B$?cGcp zIRYiKlYaj2VSc*ZB4v_L(t_O27MFN$_g_-7;JG*F@t%9L|1yb*RLh%k-gaC3@Y<&I SYh@CZ!du?{Fn?0$T?zoT8#JW= diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs index 7843c7d758bc..af927545cfcf 100644 --- a/src/Shared/CertificateGeneration/CertificateManager.cs +++ b/src/Shared/CertificateGeneration/CertificateManager.cs @@ -17,30 +17,8 @@ internal abstract class CertificateManager { internal const int CurrentAspNetCoreCertificateVersion = 2; - // - // OIDs used for HTTPS certs - // - // Why do we have two different OIDs? - // - // As part of .NET 7 changes to how the dev-certs tool works on macOS, we needed - // to rev the OID for the HTTPS cert. Changing the OID means that apps expecting - // the older OID will not recognize the newer cert. Therefore, HTTPS certificate - // operations (creation, trust, etc.) for older apps will need to be done using - // an older, pre-change dev-certs tool (found in .NET 6 and earlier SDKs). - // - // Clearly, changing the OID causes some friction for people who work on multiple - // projects targeting pre- and post-change .NET versions. In order to reduce the - // blast radius of this change, we only use the new OID on macOS for now; other - // OS's will not be impacted by this change. If, in the future, there's some - // reason to change the OID version again, we might be able to reunify the OS's. - - // This was the OID used on all OS's pre-.NET 7 - internal const string AspNetHttpsOid_NotMacOS = "1.3.6.1.4.1.311.84.1.1"; - - // This is a new OID only used on macOS for .NET 7+ - internal const string AspNetHttpsOid_MacOS = "1.3.6.1.4.1.311.84.1.3"; - internal static readonly string AspNetHttpsOid = OperatingSystem.IsMacOS() ? AspNetHttpsOid_MacOS : AspNetHttpsOid_NotMacOS; - + // OID used for HTTPS certs + internal const string AspNetHttpsOid = "1.3.6.1.4.1.311.84.1.1"; internal const string AspNetHttpsOidFriendlyName = "ASP.NET Core HTTPS development certificate"; private const string ServerAuthenticationEnhancedKeyUsageOid = "1.3.6.1.5.5.7.3.1"; diff --git a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs index e188244e1944..e8d5e106a172 100644 --- a/src/Shared/CertificateGeneration/MacOSCertificateManager.cs +++ b/src/Shared/CertificateGeneration/MacOSCertificateManager.cs @@ -2,9 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Globalization; +using System.Linq; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using System.Text.RegularExpressions; namespace Microsoft.AspNetCore.Certificates.Generation; @@ -12,6 +15,11 @@ internal sealed class MacOSCertificateManager : CertificateManager { private static readonly string MacOSUserKeychain = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "/Library/Keychains/login.keychain-db"; + // System keychain. We no longer store certificates or create trust rules in the system + // keychain, but check for their presence here so that we can clean up state left behind + // by pre-.NET 7 versions of this tool. + private const string MacOSSystemKeychain = "/Library/Keychains/System.keychain"; + // Well-known location on disk where dev-certs are stored. private static readonly string MacOSUserHttpsCertificateLocation = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".aspnet", "dev-certs", "https"); @@ -34,6 +42,21 @@ internal sealed class MacOSCertificateManager : CertificateManager private const string MacOSAddCertificateToKeyChainCommandLine = "security"; private static readonly string MacOSAddCertificateToKeyChainCommandLineArgumentsFormat = "import {0} -k " + MacOSUserKeychain + " -t cert -f pkcs12 -P {1} -A"; + // Remove a certificate from the admin trust settings. We no longer add certificates to the + // admin trust settings, but need this for cleaning up certs generated by pre-.NET 7 versions + // of this tool that used to create trust settings in the system keychain. + // Note: This operation will require user authentication. + private const string MacOSUntrustLegacyCertificateCommandLine = "sudo"; + private const string MacOSUntrustLegacyCertificateCommandLineArguments = "security remove-trusted-cert -d {0}"; + + // Find all matching certificates on the keychain {1} that have the name {0} and print + // print their SHA-256 and SHA-1 hashes. + private const string MacOSFindCertificateOnKeychainCommandLine = "security"; + private const string MacOSFindCertificateOnKeychainCommandLineArgumentsFormat = "find-certificate -c {0} -a -Z -p {1}"; + + // Format used by the tool when printing SHA-1 hashes. + private const string MacOSFindCertificateOutputRegex = "SHA-1 hash: ([0-9A-Z]+)"; + public const string InvalidCertificateState = "The ASP.NET Core developer certificate is in an invalid state. " + "To fix this issue, run 'dotnet dev-certs https --clean' and 'dotnet dev-certs https' " + @@ -97,50 +120,24 @@ protected override void TrustCertificateCore(X509Certificate2 publicCertificate) internal override CheckCertificateStateResult CheckCertificateState(X509Certificate2 candidate, bool interactive) { - var sentinelPath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".dotnet", $"certificates.{candidate.GetCertHashString(HashAlgorithmName.SHA256)}.sentinel"); - if (!interactive && !File.Exists(sentinelPath)) - { - return new CheckCertificateStateResult(false, KeyNotAccessibleWithoutUserInteraction); - } + return File.Exists(GetCertificateFilePath(candidate)) ? + new CheckCertificateStateResult(true, null) : + new CheckCertificateStateResult(false, InvalidCertificateState); + } - // Tries to use the certificate key to validate it can't access it + internal override void CorrectCertificateState(X509Certificate2 candidate) + { try { - using var rsa = candidate.GetRSAPrivateKey(); - if (rsa == null) - { - return new CheckCertificateStateResult(false, InvalidCertificateState); - } - - // Encrypting a random value is the ultimate test for a key validity. - // Windows and Mac OS both return HasPrivateKey = true if there is (or there has been) a private key associated - // with the certificate at some point. - var value = new byte[32]; - RandomNumberGenerator.Fill(value); - rsa.Decrypt(rsa.Encrypt(value, RSAEncryptionPadding.Pkcs1), RSAEncryptionPadding.Pkcs1); - - // If we were able to access the key, create a sentinel so that we don't have to show a prompt - // on every kestrel run. - if (Directory.Exists(Path.GetDirectoryName(sentinelPath)) && !File.Exists(sentinelPath)) - { - File.WriteAllText(sentinelPath, "true"); - } + // Ensure that the directory exists before writing to the file. + Directory.CreateDirectory(MacOSUserHttpsCertificateLocation); - // Being able to encrypt and decrypt a payload is the strongest guarantee that the key is valid. - return new CheckCertificateStateResult(true, null); - } - catch (Exception) - { - return new CheckCertificateStateResult(false, InvalidCertificateState); + var certificatePath = GetCertificateFilePath(candidate); + ExportCertificate(candidate, certificatePath, includePrivateKey: true, null, CertificateKeyExportFormat.Pfx); } - } - - internal override void CorrectCertificateState(X509Certificate2 candidate) - { - var status = CheckCertificateState(candidate, true); - if (!status.Success) + catch (Exception ex) { - throw new InvalidOperationException(InvalidCertificateState); + Log.MacOSAddCertificateToUserProfileDirError(candidate.Thumbprint, ex.Message); } } @@ -172,11 +169,65 @@ public override bool IsTrusted(X509Certificate2 certificate) protected override void RemoveCertificateFromTrustedRoots(X509Certificate2 certificate) { - RemoveCertificateFromKeyChain(MacOSUserKeychain, certificate); + if (IsCertOnKeychain(MacOSSystemKeychain, certificate)) + { + // Pre-.NET 7 versions of this tool used to store certs and trust settings on the + // system keychain. Check if that's the case for this cert, and if so, remove the + // trust rule and the cert from the system keychain. + try + { + RemoveAdminTrustRule(certificate); + RemoveCertificateFromKeychain(MacOSSystemKeychain, certificate); + } + catch + { + } + } + RemoveCertificateFromUserStoreCore(certificate); } - private static void RemoveCertificateFromKeyChain(string keyChain, X509Certificate2 certificate) + // Remove the certificate from the admin trust settings. + private static void RemoveAdminTrustRule(X509Certificate2 certificate) + { + Log.MacOSRemoveCertificateTrustRuleStart(GetDescription(certificate)); + var certificatePath = Path.GetTempFileName(); + try + { + var certBytes = certificate.Export(X509ContentType.Cert); + File.WriteAllBytes(certificatePath, certBytes); + var processInfo = new ProcessStartInfo( + MacOSUntrustLegacyCertificateCommandLine, + string.Format( + CultureInfo.InvariantCulture, + MacOSUntrustLegacyCertificateCommandLineArguments, + certificatePath + )); + + using var process = Process.Start(processInfo); + process!.WaitForExit(); + + if (process.ExitCode != 0) + { + Log.MacOSRemoveCertificateTrustRuleError(process.ExitCode); + } + + Log.MacOSRemoveCertificateTrustRuleEnd(); + } + finally + { + try + { + File.Delete(certificatePath); + } + catch + { + // We don't care if we can't delete the temp file. + } + } + } + + private static void RemoveCertificateFromKeychain(string keychain, X509Certificate2 certificate) { var processInfo = new ProcessStartInfo( MacOSDeleteCertificateCommandLine, @@ -184,7 +235,7 @@ private static void RemoveCertificateFromKeyChain(string keyChain, X509Certifica CultureInfo.InvariantCulture, MacOSDeleteCertificateCommandLineArgumentsFormat, certificate.Thumbprint.ToUpperInvariant(), - keyChain + keychain )) { RedirectStandardOutput = true, @@ -193,7 +244,7 @@ private static void RemoveCertificateFromKeyChain(string keyChain, X509Certifica if (Log.IsEnabled()) { - Log.MacOSRemoveCertificateFromKeyChainStart(keyChain, GetDescription(certificate)); + Log.MacOSRemoveCertificateFromKeyChainStart(keychain, GetDescription(certificate)); } using (var process = Process.Start(processInfo)) @@ -213,6 +264,36 @@ private static void RemoveCertificateFromKeyChain(string keyChain, X509Certifica Log.MacOSRemoveCertificateFromKeyChainEnd(); } + private static bool IsCertOnKeychain(string keychain, X509Certificate2 certificate) + { + TimeSpan MaxRegexTimeout = TimeSpan.FromMinutes(1); + const string CertificateSubjectRegex = "CN=(.*[^,]+).*"; + + var subjectMatch = Regex.Match(certificate.Subject, CertificateSubjectRegex, RegexOptions.Singleline, MaxRegexTimeout); + if (!subjectMatch.Success) + { + throw new InvalidOperationException($"Can't determine the subject for the certificate with subject '{certificate.Subject}'."); + } + + var subject = subjectMatch.Groups[1].Value; + + // Run the find-certificate command, and look for the cert's hash in the output + using var findCertificateProcess = Process.Start(new ProcessStartInfo( + MacOSFindCertificateOnKeychainCommandLine, + string.Format(CultureInfo.InvariantCulture, MacOSFindCertificateOnKeychainCommandLineArgumentsFormat, subject, keychain)) + { + RedirectStandardOutput = true + }); + + var output = findCertificateProcess!.StandardOutput.ReadToEnd(); + findCertificateProcess.WaitForExit(); + + var matches = Regex.Matches(output, MacOSFindCertificateOutputRegex, RegexOptions.Multiline, MaxRegexTimeout); + var hashes = matches.OfType().Select(m => m.Groups[1].Value).ToList(); + + return hashes.Any(h => string.Equals(h, certificate.Thumbprint, StringComparison.Ordinal)); + } + // We don't have a good way of checking on the underlying implementation if it is exportable, so just return true. protected override bool IsExportable(X509Certificate2 c) => true; @@ -292,11 +373,28 @@ protected override IList GetCertificatesToRemove(StoreName sto protected override void PopulateCertificatesFromStore(X509Store store, List certificates) { - bool useDiskStore = store.Name! == StoreName.My.ToString() && store.Location == StoreLocation.CurrentUser; - - if (useDiskStore) + if (store.Name! == StoreName.My.ToString() && store.Location == store.Location && Directory.Exists(MacOSUserHttpsCertificateLocation)) { - certificates.AddRange(GetCertsFromDisk()); + var certsFromDisk = GetCertsFromDisk(); + + var certsFromStore = new List(); + base.PopulateCertificatesFromStore(store, certsFromStore); + + // Certs created by pre-.NET 7. + var onlyOnKeychain = certsFromStore.Except(certsFromDisk, ThumbprintComparer.Instance); + + // Certs created (or "upgraded") by .NET 7+. + // .NET 7+ installs the certificate on disk as well as on the user keychain (for backwards + // compatibility with pre-.NET 7). + var onDiskAndKeychain = certsFromDisk.Intersect(certsFromStore, ThumbprintComparer.Instance); + + // The only times we can find a certificate on the keychain and a certificate on keychain+disk + // are when the certificate on disk and keychain has expired and a pre-.NET 7 SDK has been + // used to create a new certificate, or when a pre-.NET 7 certificate has expired and .NET 7+ + // has been used to create a new certificate. In both cases, the caller filters the invalid + // certificates out, so only the valid certificate is selected. + certificates.AddRange(onlyOnKeychain); + certificates.AddRange(onDiskAndKeychain); } else { @@ -304,6 +402,19 @@ protected override void PopulateCertificatesFromStore(X509Store store, List + { + public static readonly IEqualityComparer Instance = new ThumbprintComparer(); + +#pragma warning disable CS8769 // Nullability of reference types in type of parameter doesn't match implemented member (possibly because of nullability attributes). + bool IEqualityComparer.Equals(X509Certificate2 x, X509Certificate2 y) => + EqualityComparer.Default.Equals(x?.Thumbprint, y?.Thumbprint); +#pragma warning restore CS8769 // Nullability of reference types in type of parameter doesn't match implemented member (possibly because of nullability attributes). + + int IEqualityComparer.GetHashCode([DisallowNull] X509Certificate2 obj) => + EqualityComparer.Default.GetHashCode(obj.Thumbprint); + } + private static ICollection GetCertsFromDisk() { var certsFromDisk = new List(); @@ -346,5 +457,10 @@ protected override void RemoveCertificateFromUserStoreCore(X509Certificate2 cert { Log.MacOSRemoveCertificateFromUserProfileDirError(certificate.Thumbprint, ex.Message); } + + if (IsCertOnKeychain(MacOSUserKeychain, certificate)) + { + RemoveCertificateFromKeychain(MacOSUserKeychain, certificate); + } } } From 983aee6db60843a390c6b7911b9d1313679fab56 Mon Sep 17 00:00:00 2001 From: Aditya Mandaleeka Date: Tue, 26 Jul 2022 13:18:59 -0700 Subject: [PATCH 22/22] Remove OID change warning. --- src/Tools/dotnet-dev-certs/src/Program.cs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs index 4df6db626810..642bdc3e5559 100644 --- a/src/Tools/dotnet-dev-certs/src/Program.cs +++ b/src/Tools/dotnet-dev-certs/src/Program.cs @@ -323,12 +323,6 @@ private static int CheckHttpsCertificate(CommandOption trust, IReporter reporter } } - if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) - { - // Warn users here so they know that a successful --check doesn't mean that apps using the old OID will work. - PrintOidChangeWarning(reporter); - } - return Success; } @@ -404,11 +398,6 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio password.Value(), exportFormat.HasValue() ? format : CertificateKeyExportFormat.Pfx); - if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) - { - PrintOidChangeWarning(reporter); - } - switch (result) { case EnsureCertificateResult.Succeeded: @@ -451,13 +440,4 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio return CriticalError; } } - - private static void PrintOidChangeWarning(IReporter reporter) - { - // See comment near the definition of AspNetHttpsOid in CertificateManager. - reporter.Warn("NOTE: If your app is targeting a version of .NET earlier than .NET 7, this version of " - + "the dev-certs tool will not install/check for the correct certificate for your app. Please use " - + "the dev-certs tool from the .NET 6 SDK or the SDK version that matches your app instead. For " - + "more information, see https://aka.ms/aspnetcore-macos-cert-change"); - } }