Skip to content

Commit 53fab4f

Browse files
committed
Improve the accuracy of the check to avoid false positives
1 parent b972c6b commit 53fab4f

File tree

2 files changed

+42
-3
lines changed

2 files changed

+42
-3
lines changed

src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Net.Security;
99
using System.Runtime.InteropServices;
1010
using System.Security.Authentication;
11+
using System.Security.Cryptography;
1112
using System.Security.Cryptography.X509Certificates;
1213
using System.Threading;
1314
using System.Threading.Tasks;
@@ -218,7 +219,9 @@ private async Task InnerOnConnectionAsync(ConnectionContext context)
218219
catch (AuthenticationException ex)
219220
{
220221
_logger?.LogDebug(1, ex, CoreStrings.AuthenticationFailed);
221-
if(_serverCertificate != null && CertificateManager.IsHttpsDevelopmentCertificate(_serverCertificate))
222+
if (_serverCertificate != null &&
223+
CertificateManager.IsHttpsDevelopmentCertificate(_serverCertificate) &&
224+
!CertificateManager.CheckDeveloperCertificateKey(_serverCertificate))
222225
{
223226
_logger?.LogError(2, CoreStrings.BadDeveloperCertificateState);
224227
}

src/Shared/CertificateGeneration/CertificateManager.cs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,13 @@ internal class CertificateManager
4444

4545
// Setting to 0 means we don't append the version byte,
4646
// which is what all machines currently have.
47-
public int AspNetHttpsCertificateVersion { get; set; } = 1;
47+
public static int AspNetHttpsCertificateVersion { get; set; } = 1;
4848

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

53-
public IList<X509Certificate2> ListCertificates(
53+
public static IList<X509Certificate2> ListCertificates(
5454
CertificatePurpose purpose,
5555
StoreName storeName,
5656
StoreLocation location,
@@ -232,6 +232,42 @@ public X509Certificate2 CreateAspNetCoreHttpsDevelopmentCertificate(DateTimeOffs
232232
return certificate;
233233
}
234234

235+
internal static bool CheckDeveloperCertificateKey(X509Certificate2 candidate)
236+
{
237+
// Tries to use the certificate key to validate it can't access it
238+
try
239+
{
240+
// Search for the 'active' developer certificate, being very conservative about the possibility that there
241+
// might be multiple repeated certificates and multiple HTTPS certificates that have been rolled over.
242+
var devCert = ListCertificates(CertificatePurpose.HTTPS, StoreName.My,StoreLocation.CurrentUser, isValid:false)
243+
.OrderByDescending(c => c.NotAfter)
244+
.FirstOrDefault(c => candidate.Thumbprint == c.Thumbprint && c.HasPrivateKey);
245+
246+
if (devCert == null)
247+
{
248+
return false;
249+
}
250+
251+
var rsa = devCert.GetRSAPrivateKey();
252+
if (rsa == null)
253+
{
254+
return false;
255+
}
256+
257+
// Encrypting a random value is the ultimate test for a key validity.
258+
// Windows and Mac OS both return HasPrivateKey = true if there is (or there has been) a private key associated
259+
// with the certificate at some point.
260+
var value = new byte[32];
261+
RandomNumberGenerator.Fill(value);
262+
rsa.Encrypt(value, RSAEncryptionPadding.Pkcs1);
263+
return true;
264+
}
265+
catch (Exception)
266+
{
267+
return false;
268+
}
269+
}
270+
235271
public X509Certificate2 CreateSelfSignedCertificate(
236272
X500DistinguishedName subject,
237273
IEnumerable<X509Extension> extensions,

0 commit comments

Comments
 (0)