[Platform] Add logic to dotnet-dev-certs to detect and fix certificates with inaccessible keys on Mac OS

javiercn · javiercn · commit 36bdd51a74d3 · 2019-12-04T08:30:13.000-08:00
diff --git a/src/Shared/CertificateGeneration/CertificateManager.cs b/src/Shared/CertificateGeneration/CertificateManager.cs
@@ -41,6 +41,8 @@ internal class CertificateManager
         private const string MacOSTrustCertificateCommandLine = "sudo";
         private static readonly string MacOSTrustCertificateCommandLineArguments = "security add-trusted-cert -d -r trustRoot -k " + MacOSSystemKeyChain + " ";
         private const int UserCancelledErrorCode = 1223;
+        private const string MacOSSetPartitionKeyPermissionsCommandLine = "sudo";
+        private static readonly string MacOSSetPartitionKeyPermissionsCommandLineArguments = "security set-key-partition-list -D localhost -S unsigned:,teamid:UBF8T346G9 " + MacOSUserKeyChain;
 
         // Setting to 0 means we don't append the version byte,
         // which is what all machines currently have.
@@ -177,6 +179,27 @@ private static void DisposeCertificates(IEnumerable<X509Certificate2> disposable
             }
         }
 
+        internal bool HasValidCertificateWithInnaccessibleKeyAcrossPartitions()
+        {
+            var certificates = GetHttpsCertificates();
+            if (certificates.Count == 0)
+            {
+                return false;
+            }
+
+            // We need to check all certificates as a new one might be created that hasn't been correctly setup.
+            var result = false;
+            foreach (var certificate in certificates)
+            {
+                result = result || !CanAccessCertificateKeyAcrossPartitions(certificate);
+            }
+
+            return result;
+        }
+
+        public IList<X509Certificate2> GetHttpsCertificates() =>
+            ListCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true, requireExportable: true);
+
         public X509Certificate2 CreateAspNetCoreHttpsDevelopmentCertificate(DateTimeOffset notBefore, DateTimeOffset notAfter, string subjectOverride, DiagnosticInformation diagnostics = null)
         {
             var subject = new X500DistinguishedName(subjectOverride ?? LocalhostHttpsDistinguishedName);
@@ -707,9 +730,10 @@ public DetailedEnsureCertificateResult EnsureAspNetCoreHttpsDevelopmentCertifica
             bool trust = false,
             bool includePrivateKey = false,
             string password = null,
-            string subject = LocalhostHttpsDistinguishedName)
+            string subject = LocalhostHttpsDistinguishedName,
+            bool isInteractive = true)
         {
-            return EnsureValidCertificateExists(notBefore, notAfter, CertificatePurpose.HTTPS, path, trust, includePrivateKey, password, subject);
+            return EnsureValidCertificateExists(notBefore, notAfter, CertificatePurpose.HTTPS, path, trust, includePrivateKey, password, subject, isInteractive);
         }
 
         public DetailedEnsureCertificateResult EnsureValidCertificateExists(
@@ -720,7 +744,8 @@ public DetailedEnsureCertificateResult EnsureValidCertificateExists(
             bool trust,
             bool includePrivateKey,
             string password,
-            string subject)
+            string subject,
+            bool isInteractive)
         {
             if (purpose == CertificatePurpose.All)
             {
@@ -747,6 +772,35 @@ public DetailedEnsureCertificateResult EnsureValidCertificateExists(
                 result.Diagnostics.Debug("Skipped filtering certificates by subject.");
             }
 
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
+            {
+                foreach (var cert in filteredCertificates)
+                {
+                    if (!CanAccessCertificateKeyAcrossPartitions(cert))
+                    {
+                        if (!isInteractive)
+                        {
+                            // If the process is not interactive (first run experience) bail out. We will simply create a certificate
+                            // in case there is none or report success during the first run experience.
+                            break;
+                        }
+                        try
+                        {
+                            // The command we run handles making keys for all localhost certificates accessible across partitions. If it can not run the
+                            // command safely (because there are other localhost certificates that were not created by asp.net core, it will throw.
+                            MakeCertificateKeyAccessibleAcrossPartitions(cert);
+                            break;
+                        }
+                        catch (Exception ex)
+                        {
+                            result.Diagnostics.Error("Failed to make certificate key accessible", ex);
+                            result.ResultCode = EnsureCertificateResult.FailedToMakeKeyAccessible;
+                            return result;
+                        }
+                    }
+                }
+            }
+
             certificates = filteredCertificates;
 
             result.ResultCode = EnsureCertificateResult.Succeeded;
@@ -794,6 +848,11 @@ public DetailedEnsureCertificateResult EnsureValidCertificateExists(
                     result.ResultCode = EnsureCertificateResult.ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore;
                     return result;
                 }
+
+                if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && isInteractive)
+                {
+                    MakeCertificateKeyAccessibleAcrossPartitions(certificate);
+                }
             }
             if (path != null)
             {
@@ -835,6 +894,73 @@ public DetailedEnsureCertificateResult EnsureValidCertificateExists(
             return result;
         }
 
+        private void MakeCertificateKeyAccessibleAcrossPartitions(X509Certificate2 certificate)                {
+            if (OtherNonAspNetCoreHttpsCertificatesPresent())
+            {
+                throw new InvalidOperationException("Unable to make HTTPS ceritificate key trusted across security partitions.");
+            }
+            using (var process = Process.Start(MacOSSetPartitionKeyPermissionsCommandLine, MacOSSetPartitionKeyPermissionsCommandLineArguments))
+            {
+                process.WaitForExit();
+                if (process.ExitCode != 0)
+                {
+                    throw new InvalidOperationException("Error making the key accessible across partitions.");
+                }
+            }
+
+            var certificateSentinelPath = GetCertificateSentinelPath(certificate);
+            File.WriteAllText(certificateSentinelPath, "true");
+        }
+
+        private static string GetCertificateSentinelPath(X509Certificate2 certificate) =>
+            Path.Combine(Environment.GetEnvironmentVariable("HOME"), ".dotnet", $"certificate.{certificate.GetCertHashString(HashAlgorithmName.SHA256)}.sentinel");
+
+        private bool OtherNonAspNetCoreHttpsCertificatesPresent()
+        {
+            var certificates = new List<X509Certificate2>();
+            try
+            {
+                using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser))
+                {
+                    store.Open(OpenFlags.ReadOnly);
+                    certificates.AddRange(store.Certificates.OfType<X509Certificate2>());
+                    IEnumerable<X509Certificate2> matchingCertificates = certificates;
+                        // Ensure the certificate hasn't expired, has a private key and its exportable
+                        // (for container/unix scenarios).
+                        var now = DateTimeOffset.Now;
+                        matchingCertificates = matchingCertificates
+                            .Where(c => c.NotBefore <= now &&
+                                now <= c.NotAfter && c.Subject == LocalhostHttpsDistinguishedName);
+
+                    // We need to enumerate the certificates early to prevent dispoisng issues.
+                    matchingCertificates = matchingCertificates.ToList();
+
+                    var certificatesToDispose = certificates.Except(matchingCertificates);
+                    DisposeCertificates(certificatesToDispose);
+
+                    store.Close();
+
+                    return matchingCertificates.All(c => !HasOid(c, AspNetHttpsOid));
+                }
+            }
+            catch
+            {
+                DisposeCertificates(certificates);
+                certificates.Clear();
+                return true;
+            }
+
+            bool HasOid(X509Certificate2 certificate, string oid) =>
+                certificate.Extensions.OfType<X509Extension>()
+                .Any(e => string.Equals(oid, e.Oid.Value, StringComparison.Ordinal));
+        }
+
+        private bool CanAccessCertificateKeyAcrossPartitions(X509Certificate2 certificate)
+        {
+            var certificateSentinelPath = GetCertificateSentinelPath(certificate);
+            return File.Exists(certificateSentinelPath);
+        }
+
         private class UserCancelledTrustException : Exception
         {
         }
diff --git a/src/Shared/CertificateGeneration/EnsureCertificateResult.cs b/src/Shared/CertificateGeneration/EnsureCertificateResult.cs
@@ -11,6 +11,7 @@ internal enum EnsureCertificateResult
         ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore,
         ErrorExportingTheCertificate,
         FailedToTrustTheCertificate,
-        UserCancelledTrustStep
+        UserCancelledTrustStep,
+        FailedToMakeKeyAccessible,
     }
 }
diff --git a/src/Tools/FirstRunCertGenerator/src/CertificateGenerator.cs b/src/Tools/FirstRunCertGenerator/src/CertificateGenerator.cs
@@ -9,7 +9,7 @@ public static void GenerateAspNetHttpsCertificate()
         {
             var manager = new CertificateManager();
             var now = DateTimeOffset.Now;
-            manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1));
+            manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), isInteractive: false);
         }
     }
 }
diff --git a/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs b/src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs
@@ -42,7 +42,7 @@ public void EnsureCreateHttpsCertificate_CreatesACertificate_WhenThereAreNoHttps
                     // Act
                 DateTimeOffset now = DateTimeOffset.UtcNow;
                 now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
-                var result = _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), CertificateName, trust: false, subject: TestCertificateSubject);
+                var result = _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), CertificateName, trust: false, subject: TestCertificateSubject, isInteractive: false);
 
                 // Assert
                 Assert.Equal(EnsureCertificateResult.Succeeded, result.ResultCode);
@@ -135,12 +135,12 @@ public void EnsureCreateHttpsCertificate_DoesNotCreateACertificate_WhenThereIsAn
 
             DateTimeOffset now = DateTimeOffset.UtcNow;
             now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
-            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject);
+            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject, isInteractive: false);
 
             var httpsCertificate = CertificateManager.ListCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: false).Single(c => c.Subject == TestCertificateSubject);
 
             // Act
-            var result = _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), CertificateName, trust: false, includePrivateKey: true, password: certificatePassword, subject: TestCertificateSubject);
+            var result = _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), CertificateName, trust: false, includePrivateKey: true, password: certificatePassword, subject: TestCertificateSubject, isInteractive: false);
 
             // Assert
             Assert.Equal(EnsureCertificateResult.ValidCertificatePresent, result.ResultCode);
@@ -162,7 +162,7 @@ public void EnsureCreateHttpsCertificate_ReturnsExpiredCertificateIfVersionIsInc
 
             DateTimeOffset now = DateTimeOffset.UtcNow;
             now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
-            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject);
+            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject, isInteractive: false);
 
             CertificateManager.AspNetHttpsCertificateVersion = 2;
 
@@ -179,7 +179,7 @@ public void EnsureCreateHttpsCertificate_ReturnsExpiredCertificateForEmptyVersio
             DateTimeOffset now = DateTimeOffset.UtcNow;
             now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
             CertificateManager.AspNetHttpsCertificateVersion = 0;
-            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject);
+            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject, isInteractive: false);
 
             CertificateManager.AspNetHttpsCertificateVersion = 1;
 
@@ -196,7 +196,7 @@ public void EnsureCreateHttpsCertificate_ReturnsValidIfVersionIsZero()
             DateTimeOffset now = DateTimeOffset.UtcNow;
             now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
             CertificateManager.AspNetHttpsCertificateVersion = 0;
-            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject);
+            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject, isInteractive: false);
 
             var httpsCertificateList = CertificateManager.ListCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true);
             Assert.NotEmpty(httpsCertificateList);
@@ -211,7 +211,7 @@ public void EnsureCreateHttpsCertificate_ReturnValidIfCertIsNewer()
             DateTimeOffset now = DateTimeOffset.UtcNow;
             now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
             CertificateManager.AspNetHttpsCertificateVersion = 2;
-            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject);
+            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, subject: TestCertificateSubject, isInteractive: false);
 
             CertificateManager.AspNetHttpsCertificateVersion = 1;
             var httpsCertificateList = CertificateManager.ListCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true);
@@ -225,7 +225,7 @@ public void EnsureAspNetCoreHttpsDevelopmentCertificate_ReturnsCorrectResult_Whe
 
             DateTimeOffset now = DateTimeOffset.UtcNow;
             now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
-            var trustFailed = _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: true, subject: TestCertificateSubject);
+            var trustFailed = _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: true, subject: TestCertificateSubject, isInteractive: false);
 
             Assert.Equal(EnsureCertificateResult.UserCancelledTrustStep, trustFailed.ResultCode);
         }
@@ -237,7 +237,7 @@ public void EnsureAspNetCoreHttpsDevelopmentCertificate_CanRemoveCertificates()
 
             DateTimeOffset now = DateTimeOffset.UtcNow;
             now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
-            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: true, subject: TestCertificateSubject);
+            _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: true, subject: TestCertificateSubject, isInteractive: false);
 
             _manager.CleanupHttpsCertificates(TestCertificateSubject);
 
diff --git a/src/Tools/dotnet-dev-certs/src/Program.cs b/src/Tools/dotnet-dev-certs/src/Program.cs
@@ -24,6 +24,7 @@ internal class Program
         private const int ErrorNoValidCertificateFound = 6;
         private const int ErrorCertificateNotTrusted = 7;
         private const int ErrorCleaningUpCertificates = 8;
+        private const int ErrorMacOsCertificateKeyCouldNotBeAccessible = 9;
 
         public static readonly TimeSpan HttpsCertificateValidity = TimeSpan.FromDays(365);
 
@@ -158,7 +159,15 @@ private static int CheckHttpsCertificate(CommandOption trust, IReporter reporter
             }
             else
             {
-                reporter.Output("A valid certificate was found.");
+                if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && certificateManager.HasValidCertificateWithInnaccessibleKeyAcrossPartitions())
+                {
+                    reporter.Warn($"A valid HTTPS certificate was found but it may not be accessible across security partitions. Run dotnet dev-certs https to ensure it will be accessible during development.");
+                    return ErrorMacOsCertificateKeyCouldNotBeAccessible;
+                }
+                else
+                {
+                    reporter.Verbose("A valid certificate was found.");
+                }
             }
 
             if (trust != null && trust.HasValue())
@@ -185,6 +194,13 @@ private static int EnsureHttpsCertificate(CommandOption exportPath, CommandOptio
             var now = DateTimeOffset.Now;
             var manager = new CertificateManager();
 
+            if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && manager.HasValidCertificateWithInnaccessibleKeyAcrossPartitions() || manager.GetHttpsCertificates().Count == 0)
+            {
+                reporter.Warn($"A valid HTTPS certificate with a key accessible across security partitions was not found. The following command will run to fix it:" + Environment.NewLine +
+                    "'sudo security set-key-partition-list -D localhost -S unsigned:,teamid:UBF8T346G9'" + Environment.NewLine +
+                    "This command will make the certificate key accessible across security partitions and might prompt you for your password. For more information see: https://aka.ms/aspnetcore/2.1/troubleshootcertissues");
+            }
+
             if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && trust?.HasValue() == true)
             {
                 reporter.Warn("Trusting the HTTPS development certificate was requested. If the certificate is not " +

Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,7 @@ internal enum EnsureCertificateResult`
`11`	`11`	`ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore,`
`12`	`12`	`ErrorExportingTheCertificate,`
`13`	`13`	`FailedToTrustTheCertificate,`
`14`		`- UserCancelledTrustStep`
	`14`	`+ UserCancelledTrustStep,`
	`15`	`+ FailedToMakeKeyAccessible,`
`15`	`16`	`}`
`16`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ public static void GenerateAspNetHttpsCertificate()`
`9`	`9`	`{`
`10`	`10`	`var manager = new CertificateManager();`
`11`	`11`	`var now = DateTimeOffset.Now;`
`12`		`- manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1));`
	`12`	`+ manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), isInteractive: false);`
`13`	`13`	`}`
`14`	`14`	`}`
`15`	`15`	`}`