Skip to content

[Platform] Detect and fix certificates with potentially inaccessible keys on Mac OS (3.1) #17581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 129 additions & 3 deletions src/Shared/CertificateGeneration/CertificateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand All @@ -720,7 +744,8 @@ public DetailedEnsureCertificateResult EnsureValidCertificateExists(
bool trust,
bool includePrivateKey,
string password,
string subject)
string subject,
bool isInteractive)
{
if (purpose == CertificatePurpose.All)
{
Expand 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;
Expand Down Expand Up @@ -794,6 +848,11 @@ public DetailedEnsureCertificateResult EnsureValidCertificateExists(
result.ResultCode = EnsureCertificateResult.ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore;
return result;
}

if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && isInteractive)
{
MakeCertificateKeyAccessibleAcrossPartitions(certificate);
}
}
if (path != null)
{
Expand Down Expand Up @@ -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
{
}
Expand Down
3 changes: 2 additions & 1 deletion src/Shared/CertificateGeneration/EnsureCertificateResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ internal enum EnsureCertificateResult
ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore,
ErrorExportingTheCertificate,
FailedToTrustTheCertificate,
UserCancelledTrustStep
UserCancelledTrustStep,
FailedToMakeKeyAccessible,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
18 changes: 9 additions & 9 deletions src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}
Expand All @@ -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);

Expand Down
18 changes: 17 additions & 1 deletion src/Tools/dotnet-dev-certs/src/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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())
Expand All @@ -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/3.1/troubleshootcertissues");
}

if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && trust?.HasValue() == true)
{
reporter.Warn("Trusting the HTTPS development certificate was requested. If the certificate is not " +
Expand Down