Skip to content

Commit 7f53f7e

Browse files
javiercndanroth27Tratcher
authored andcommitted
[Platform] Detect and fix certificates with potentially inaccessible keys on Mac OS (2.1) (#17560)
* [Https] Detects and fixes HTTPS certificates where the key is not guaranteed to be accessible across security partitions * Fix dotnet dev-certs https --check * Update logic for detecting missing certs * Fix security command * Update warning logic * Check that the key is accessible in Kestrel * Add correct link to docs * Update src/Tools/dotnet-dev-certs/src/Program.cs Co-Authored-By: Daniel Roth <[email protected]> * Update src/Tools/dotnet-dev-certs/src/Program.cs Co-Authored-By: Daniel Roth <[email protected]> * Add test for 2.1 * Update src/Tools/dotnet-dev-certs/src/Program.cs Co-Authored-By: Chris Ross <[email protected]> * Address feedback * Fix non-interctive path * Fix tests * Remove a couple of test from an unshipped product * Check only for certificates considered valid * Switch the exception being caught, remove invalid test Co-authored-by: Daniel Roth <[email protected]> Co-authored-by: Chris Ross <[email protected]>
1 parent 8211a1c commit 7f53f7e

File tree

10 files changed

+234
-119
lines changed

10 files changed

+234
-119
lines changed

src/Servers/Kestrel/Core/src/CoreStrings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,4 +518,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
518518
<data name="ConnectionTimedOutByServer" xml:space="preserve">
519519
<value>The connection was timed out by the server.</value>
520520
</data>
521+
<data name="BadDeveloperCertificateState" xml:space="preserve">
522+
<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>
523+
</data>
521524
</root>

src/Servers/Kestrel/Core/src/Internal/HttpsConnectionAdapter.cs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using System.Security.Cryptography.X509Certificates;
1010
using System.Threading;
1111
using System.Threading.Tasks;
12+
using Microsoft.AspNetCore.Certificates.Generation;
1213
using Microsoft.AspNetCore.Connections;
1314
using Microsoft.AspNetCore.Http.Features;
1415
using Microsoft.AspNetCore.Server.Kestrel.Core;
@@ -177,8 +178,9 @@ private async Task<IAdaptedConnection> InnerOnConnectionAsync(ConnectionAdapterC
177178
EnsureCertificateIsAllowedForServerAuth(serverCert);
178179
}
179180
}
181+
180182
await sslStream.AuthenticateAsServerAsync(serverCert, certificateRequired,
181-
_options.SslProtocols, _options.CheckCertificateRevocation);
183+
_options.SslProtocols, _options.CheckCertificateRevocation);
182184
#endif
183185
}
184186
catch (OperationCanceledException)
@@ -187,12 +189,28 @@ await sslStream.AuthenticateAsServerAsync(serverCert, certificateRequired,
187189
sslStream.Dispose();
188190
return _closedAdaptedConnection;
189191
}
190-
catch (Exception ex) when (ex is IOException || ex is AuthenticationException)
192+
catch (IOException ex)
191193
{
192194
_logger?.LogDebug(1, ex, CoreStrings.AuthenticationFailed);
193195
sslStream.Dispose();
194196
return _closedAdaptedConnection;
195197
}
198+
catch (AuthenticationException ex)
199+
{
200+
if (_serverCertificate != null &&
201+
CertificateManager.IsHttpsDevelopmentCertificate(_serverCertificate) &&
202+
!CertificateManager.CheckDeveloperCertificateKey(_serverCertificate))
203+
{
204+
_logger?.LogError(3, ex, CoreStrings.BadDeveloperCertificateState);
205+
}
206+
else
207+
{
208+
_logger?.LogDebug(1, ex, CoreStrings.AuthenticationFailed);
209+
}
210+
211+
sslStream.Dispose();
212+
return _closedAdaptedConnection;
213+
}
196214
finally
197215
{
198216
timeoutFeature.CancelTimeout();

src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Servers/Kestrel/shared/test/TestResources.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,10 @@ public static X509Certificate2 GetTestCertificate(string certName)
2222
{
2323
return new X509Certificate2(GetCertPath(certName), "testPassword");
2424
}
25+
26+
public static X509Certificate2 GetTestCertificate(string certName, string password)
27+
{
28+
return new X509Certificate2(GetCertPath(certName), password);
29+
}
2530
}
2631
}

src/Servers/Kestrel/test/FunctionalTests/HttpsTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;

src/Shared/CertificateGeneration/CertificateManager.cs

Lines changed: 164 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.IO;
88
using System.Linq;
99
using System.Runtime.InteropServices;
10+
using System.Runtime.InteropServices.ComTypes;
1011
using System.Security.Cryptography;
1112
using System.Security.Cryptography.X509Certificates;
1213
using System.Text;
@@ -50,6 +51,8 @@ internal class CertificateManager
5051
private static readonly string MacOSTrustCertificateCommandLineArguments = "security add-trusted-cert -d -r trustRoot -k " + MacOSSystemKeyChain + " ";
5152
#endif
5253
private const int UserCancelledErrorCode = 1223;
54+
private const string MacOSSetPartitionKeyPermissionsCommandLine = "sudo";
55+
private static readonly string MacOSSetPartitionKeyPermissionsCommandLineArguments = "security set-key-partition-list -D localhost -S unsigned:,teamid:UBF8T346G9 " + MacOSUserKeyChain;
5356

5457
public IList<X509Certificate2> ListCertificates(
5558
CertificatePurpose purpose,
@@ -147,6 +150,39 @@ private void DisposeCertificates(IEnumerable<X509Certificate2> disposables)
147150
}
148151
}
149152

153+
internal static bool IsHttpsDevelopmentCertificate(X509Certificate2 certificate) =>
154+
certificate.Extensions.OfType<X509Extension>()
155+
.Any(e => string.Equals(AspNetHttpsOid, e.Oid.Value, StringComparison.Ordinal));
156+
157+
internal static bool CheckDeveloperCertificateKey(X509Certificate2 candidate)
158+
{
159+
// Tries to use the certificate key to validate it can't access it
160+
try
161+
{
162+
var rsa = candidate.GetRSAPrivateKey();
163+
if (rsa == null)
164+
{
165+
return false;
166+
}
167+
168+
// Encrypting a random value is the ultimate test for a key validity.
169+
// Windows and Mac OS both return HasPrivateKey = true if there is (or there has been) a private key associated
170+
// with the certificate at some point.
171+
var value = new byte[32];
172+
using (var rng = RandomNumberGenerator.Create())
173+
{
174+
rsa.Decrypt(rsa.Encrypt(value, RSAEncryptionPadding.Pkcs1), RSAEncryptionPadding.Pkcs1);
175+
}
176+
177+
// Being able to encrypt and decrypt a payload is the strongest guarantee that the key is valid.
178+
return true;
179+
}
180+
catch (Exception)
181+
{
182+
return false;
183+
}
184+
}
185+
150186
#if NETCOREAPP2_0 || NETCOREAPP2_1
151187

152188
public X509Certificate2 CreateAspNetCoreHttpsDevelopmentCertificate(DateTimeOffset notBefore, DateTimeOffset notAfter, string subjectOverride)
@@ -192,6 +228,27 @@ public X509Certificate2 CreateAspNetCoreHttpsDevelopmentCertificate(DateTimeOffs
192228
return certificate;
193229
}
194230

231+
internal bool HasValidCertificateWithInnaccessibleKeyAcrossPartitions()
232+
{
233+
var certificates = GetHttpsCertificates();
234+
if (certificates.Count == 0)
235+
{
236+
return false;
237+
}
238+
239+
// We need to check all certificates as a new one might be created that hasn't been correctly setup.
240+
var result = false;
241+
foreach (var certificate in certificates)
242+
{
243+
result = result || !CanAccessCertificateKeyAcrossPartitions(certificate);
244+
}
245+
246+
return result;
247+
}
248+
249+
public IList<X509Certificate2> GetHttpsCertificates() =>
250+
ListCertificates(CertificatePurpose.HTTPS, StoreName.My, StoreLocation.CurrentUser, isValid: true, requireExportable: true);
251+
195252
public X509Certificate2 CreateApplicationTokenSigningDevelopmentCertificate(DateTimeOffset notBefore, DateTimeOffset notAfter, string subjectOverride)
196253
{
197254
var subject = new X500DistinguishedName(subjectOverride ?? IdentityDistinguishedName);
@@ -596,9 +653,10 @@ public EnsureCertificateResult EnsureAspNetCoreHttpsDevelopmentCertificate(
596653
bool trust = false,
597654
bool includePrivateKey = false,
598655
string password = null,
599-
string subject = LocalhostHttpsDistinguishedName)
656+
string subject = LocalhostHttpsDistinguishedName,
657+
bool isInteractive = true)
600658
{
601-
return EnsureValidCertificateExists(notBefore, notAfter, CertificatePurpose.HTTPS, path, trust, includePrivateKey, password, subject);
659+
return EnsureValidCertificateExists(notBefore, notAfter, CertificatePurpose.HTTPS, path, trust, includePrivateKey, password, subject, isInteractive);
602660
}
603661

604662
public EnsureCertificateResult EnsureAspNetCoreApplicationTokensDevelopmentCertificate(
@@ -610,7 +668,7 @@ public EnsureCertificateResult EnsureAspNetCoreApplicationTokensDevelopmentCerti
610668
string password = null,
611669
string subject = IdentityDistinguishedName)
612670
{
613-
return EnsureValidCertificateExists(notBefore, notAfter, CertificatePurpose.Signing, path, trust, includePrivateKey, password, subject);
671+
return EnsureValidCertificateExists(notBefore, notAfter, CertificatePurpose.Signing, path, trust, includePrivateKey, password, subject, isInteractive: true);
614672
}
615673

616674
public EnsureCertificateResult EnsureValidCertificateExists(
@@ -621,7 +679,8 @@ public EnsureCertificateResult EnsureValidCertificateExists(
621679
bool trust = false,
622680
bool includePrivateKey = false,
623681
string password = null,
624-
string subjectOverride = null)
682+
string subjectOverride = null,
683+
bool isInteractive = true)
625684
{
626685
if (purpose == CertificatePurpose.All)
627686
{
@@ -633,6 +692,33 @@ public EnsureCertificateResult EnsureValidCertificateExists(
633692

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

695+
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
696+
{
697+
foreach (var cert in certificates)
698+
{
699+
if (!CanAccessCertificateKeyAcrossPartitions(cert))
700+
{
701+
if (!isInteractive)
702+
{
703+
// If the process is not interactive (first run experience) bail out. We will simply create a certificate
704+
// in case there is none or report success during the first run experience.
705+
break;
706+
}
707+
try
708+
{
709+
// The command we run handles making keys for all localhost certificates accessible across partitions. If it can not run the
710+
// command safely (because there are other localhost certificates that were not created by asp.net core, it will throw.
711+
MakeCertificateKeyAccessibleAcrossPartitions(cert);
712+
break;
713+
}
714+
catch (Exception)
715+
{
716+
return EnsureCertificateResult.FailedToMakeKeyAccessible;
717+
}
718+
}
719+
}
720+
}
721+
636722
var result = EnsureCertificateResult.Succeeded;
637723

638724
X509Certificate2 certificate = null;
@@ -672,6 +758,11 @@ public EnsureCertificateResult EnsureValidCertificateExists(
672758
{
673759
return EnsureCertificateResult.ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore;
674760
}
761+
762+
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && isInteractive)
763+
{
764+
MakeCertificateKeyAccessibleAcrossPartitions(certificate);
765+
}
675766
}
676767
if (path != null)
677768
{
@@ -704,6 +795,74 @@ public EnsureCertificateResult EnsureValidCertificateExists(
704795
return result;
705796
}
706797

798+
private void MakeCertificateKeyAccessibleAcrossPartitions(X509Certificate2 certificate)
799+
{
800+
if (OtherNonAspNetCoreHttpsCertificatesPresent())
801+
{
802+
throw new InvalidOperationException("Unable to make HTTPS ceritificate key trusted across security partitions.");
803+
}
804+
using (var process = Process.Start(MacOSSetPartitionKeyPermissionsCommandLine, MacOSSetPartitionKeyPermissionsCommandLineArguments))
805+
{
806+
process.WaitForExit();
807+
if (process.ExitCode != 0)
808+
{
809+
throw new InvalidOperationException("Error making the key accessible across partitions.");
810+
}
811+
}
812+
813+
var certificateSentinelPath = GetCertificateSentinelPath(certificate);
814+
File.WriteAllText(certificateSentinelPath, "true");
815+
}
816+
817+
private static string GetCertificateSentinelPath(X509Certificate2 certificate) =>
818+
Path.Combine(Environment.GetEnvironmentVariable("HOME"), ".dotnet", $"certificate.{certificate.GetCertHashString(HashAlgorithmName.SHA256)}.sentinel");
819+
820+
private bool OtherNonAspNetCoreHttpsCertificatesPresent()
821+
{
822+
var certificates = new List<X509Certificate2>();
823+
try
824+
{
825+
using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser))
826+
{
827+
store.Open(OpenFlags.ReadOnly);
828+
certificates.AddRange(store.Certificates.OfType<X509Certificate2>());
829+
IEnumerable<X509Certificate2> matchingCertificates = certificates;
830+
// Ensure the certificate hasn't expired, has a private key and its exportable
831+
// (for container/unix scenarios).
832+
var now = DateTimeOffset.Now;
833+
matchingCertificates = matchingCertificates
834+
.Where(c => c.NotBefore <= now &&
835+
now <= c.NotAfter && c.Subject == LocalhostHttpsDistinguishedName);
836+
837+
// We need to enumerate the certificates early to prevent dispoisng issues.
838+
matchingCertificates = matchingCertificates.ToList();
839+
840+
var certificatesToDispose = certificates.Except(matchingCertificates);
841+
DisposeCertificates(certificatesToDispose);
842+
843+
store.Close();
844+
845+
return matchingCertificates.All(c => !HasOid(c, AspNetHttpsOid));
846+
}
847+
}
848+
catch
849+
{
850+
DisposeCertificates(certificates);
851+
certificates.Clear();
852+
return true;
853+
}
854+
855+
bool HasOid(X509Certificate2 certificate, string oid) =>
856+
certificate.Extensions.OfType<X509Extension>()
857+
.Any(e => string.Equals(oid, e.Oid.Value, StringComparison.Ordinal));
858+
}
859+
860+
private bool CanAccessCertificateKeyAcrossPartitions(X509Certificate2 certificate)
861+
{
862+
var certificateSentinelPath = GetCertificateSentinelPath(certificate);
863+
return File.Exists(certificateSentinelPath);
864+
}
865+
707866
private class UserCancelledTrustException : Exception
708867
{
709868
}
@@ -717,4 +876,4 @@ private enum RemoveLocations
717876
}
718877
#endif
719878
}
720-
}
879+
}

src/Shared/CertificateGeneration/EnsureCertificateResult.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ internal enum EnsureCertificateResult
1313
ErrorSavingTheCertificateIntoTheCurrentUserPersonalStore,
1414
ErrorExportingTheCertificate,
1515
FailedToTrustTheCertificate,
16-
UserCancelledTrustStep
16+
UserCancelledTrustStep,
17+
FailedToMakeKeyAccessible,
1718
}
1819
}
1920

20-
#endif
21+
#endif

src/Tools/FirstRunCertGenerator/src/CertificateGenerator.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ public static void GenerateAspNetHttpsCertificate()
99
{
1010
var manager = new CertificateManager();
1111
var now = DateTimeOffset.Now;
12-
manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1));
12+
manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), isInteractive: false);
1313
}
1414
}
1515
}

0 commit comments

Comments
 (0)