Skip to content

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

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 17 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
3 changes: 3 additions & 0 deletions src/Servers/Kestrel/Core/src/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -518,4 +518,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
<data name="ConnectionTimedOutByServer" xml:space="preserve">
<value>The connection was timed out by the server.</value>
</data>
<data name="BadDeveloperCertificateState" xml:space="preserve">
<value>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 and create a new untrusted developer certificate. On macOS or Windows, use 'dotnet dev-certs https --trust' to trust the new certificate.</value>
</data>
</root>
22 changes: 20 additions & 2 deletions src/Servers/Kestrel/Core/src/Internal/HttpsConnectionAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Certificates.Generation;
using Microsoft.AspNetCore.Connections;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Server.Kestrel.Core;
Expand Down Expand Up @@ -177,8 +178,9 @@ private async Task<IAdaptedConnection> InnerOnConnectionAsync(ConnectionAdapterC
EnsureCertificateIsAllowedForServerAuth(serverCert);
}
}

await sslStream.AuthenticateAsServerAsync(serverCert, certificateRequired,
_options.SslProtocols, _options.CheckCertificateRevocation);
_options.SslProtocols, _options.CheckCertificateRevocation);
#endif
}
catch (OperationCanceledException)
Expand All @@ -187,12 +189,28 @@ await sslStream.AuthenticateAsServerAsync(serverCert, certificateRequired,
sslStream.Dispose();
return _closedAdaptedConnection;
}
catch (Exception ex) when (ex is IOException || ex is AuthenticationException)
catch (IOException ex)
{
_logger?.LogDebug(1, ex, CoreStrings.AuthenticationFailed);
sslStream.Dispose();
return _closedAdaptedConnection;
}
catch (AuthenticationException ex)
{
if (_serverCertificate != null &&
CertificateManager.IsHttpsDevelopmentCertificate(_serverCertificate) &&
!CertificateManager.CheckDeveloperCertificateKey(_serverCertificate))
{
_logger?.LogError(3, ex, CoreStrings.BadDeveloperCertificateState);
}
else
{
_logger?.LogDebug(1, ex, CoreStrings.AuthenticationFailed);
}

sslStream.Dispose();
return _closedAdaptedConnection;
}
finally
{
timeoutFeature.CancelTimeout();
Expand Down
14 changes: 14 additions & 0 deletions src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Servers/Kestrel/shared/test/TestResources.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,10 @@ public static X509Certificate2 GetTestCertificate(string certName)
{
return new X509Certificate2(GetCertPath(certName), "testPassword");
}

public static X509Certificate2 GetTestCertificate(string certName, string password)
{
return new X509Certificate2(GetCertPath(certName), password);
}
}
}
2 changes: 1 addition & 1 deletion src/Servers/Kestrel/test/FunctionalTests/HttpsTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand Down
169 changes: 164 additions & 5 deletions src/Shared/CertificateGeneration/CertificateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.ComTypes;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Text;
Expand Down Expand Up @@ -50,6 +51,8 @@ internal class CertificateManager
private static readonly string MacOSTrustCertificateCommandLineArguments = "security add-trusted-cert -d -r trustRoot -k " + MacOSSystemKeyChain + " ";
#endif
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;

public IList<X509Certificate2> ListCertificates(
CertificatePurpose purpose,
Expand Down Expand Up @@ -147,6 +150,39 @@ private void DisposeCertificates(IEnumerable<X509Certificate2> disposables)
}
}

internal static bool IsHttpsDevelopmentCertificate(X509Certificate2 certificate) =>
certificate.Extensions.OfType<X509Extension>()
.Any(e => string.Equals(AspNetHttpsOid, e.Oid.Value, StringComparison.Ordinal));

internal static bool CheckDeveloperCertificateKey(X509Certificate2 candidate)
{
// Tries to use the certificate key to validate it can't access it
try
{
var rsa = candidate.GetRSAPrivateKey();
if (rsa == null)
{
return false;
}

// 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];
using (var rng = RandomNumberGenerator.Create())
{
rsa.Decrypt(rsa.Encrypt(value, RSAEncryptionPadding.Pkcs1), RSAEncryptionPadding.Pkcs1);
}

// Being able to encrypt and decrypt a payload is the strongest guarantee that the key is valid.
return true;
}
catch (Exception)
{
return false;
}
}

#if NETCOREAPP2_0 || NETCOREAPP2_1

public X509Certificate2 CreateAspNetCoreHttpsDevelopmentCertificate(DateTimeOffset notBefore, DateTimeOffset notAfter, string subjectOverride)
Expand Down Expand Up @@ -192,6 +228,27 @@ public X509Certificate2 CreateAspNetCoreHttpsDevelopmentCertificate(DateTimeOffs
return certificate;
}

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 CreateApplicationTokenSigningDevelopmentCertificate(DateTimeOffset notBefore, DateTimeOffset notAfter, string subjectOverride)
{
var subject = new X500DistinguishedName(subjectOverride ?? IdentityDistinguishedName);
Expand Down Expand Up @@ -596,9 +653,10 @@ public EnsureCertificateResult EnsureAspNetCoreHttpsDevelopmentCertificate(
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 EnsureCertificateResult EnsureAspNetCoreApplicationTokensDevelopmentCertificate(
Expand All @@ -610,7 +668,7 @@ public EnsureCertificateResult EnsureAspNetCoreApplicationTokensDevelopmentCerti
string password = null,
string subject = IdentityDistinguishedName)
{
return EnsureValidCertificateExists(notBefore, notAfter, CertificatePurpose.Signing, path, trust, includePrivateKey, password, subject);
return EnsureValidCertificateExists(notBefore, notAfter, CertificatePurpose.Signing, path, trust, includePrivateKey, password, subject, isInteractive: true);
}

public EnsureCertificateResult EnsureValidCertificateExists(
Expand All @@ -621,7 +679,8 @@ public EnsureCertificateResult EnsureValidCertificateExists(
bool trust = false,
bool includePrivateKey = false,
string password = null,
string subjectOverride = null)
string subjectOverride = null,
bool isInteractive = true)
{
if (purpose == CertificatePurpose.All)
{
Expand All @@ -633,6 +692,33 @@ public EnsureCertificateResult EnsureValidCertificateExists(

certificates = subjectOverride == null ? certificates : certificates.Where(c => c.Subject == subjectOverride);

if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
foreach (var cert in certificates)
{
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)
{
return EnsureCertificateResult.FailedToMakeKeyAccessible;
}
}
}
}

var result = EnsureCertificateResult.Succeeded;

X509Certificate2 certificate = null;
Expand Down Expand Up @@ -672,6 +758,11 @@ public EnsureCertificateResult EnsureValidCertificateExists(
{
return EnsureCertificateResult.ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore;
}

if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && isInteractive)
{
MakeCertificateKeyAccessibleAcrossPartitions(certificate);
}
}
if (path != null)
{
Expand Down Expand Up @@ -704,6 +795,74 @@ public EnsureCertificateResult 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 All @@ -717,4 +876,4 @@ private enum RemoveLocations
}
#endif
}
}
}
5 changes: 3 additions & 2 deletions src/Shared/CertificateGeneration/EnsureCertificateResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ internal enum EnsureCertificateResult
ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore,
ErrorExportingTheCertificate,
FailedToTrustTheCertificate,
UserCancelledTrustStep
UserCancelledTrustStep,
FailedToMakeKeyAccessible,
}
}

#endif
#endif
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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bit that goes into the CLI.

At this point we don't want to try and setup the certificate key as that requires user interaction and the CLI first run experience must remain non-interactive.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this change, at which point would the dev certificate first be added/updated if not during the first run experience?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The certificate still gets created during the first run experience. We simply don't try and make it available across security partitions at that time as that's the bit that requires user interaction.

}
}
}
Loading