Skip to content

Commit 5059927

Browse files
committed
Make dev-certs import consistent with kestrel (dotnet#57014)
* Make dev-certs import consistent with kestrel Kestrel checks the subject name and our magic extension - import was only checking the extension. They can't easily share a method because import has a test hook. (cherry picked from commit 06155c0)
1 parent d603711 commit 5059927

File tree

3 files changed

+66
-22
lines changed

3 files changed

+66
-22
lines changed

src/Servers/Kestrel/Core/src/TlsConfigurationLoader.cs

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -176,20 +176,8 @@ public ListenOptions UseHttpsWithSni(
176176

177177
private static bool IsDevelopmentCertificate(X509Certificate2 certificate)
178178
{
179-
if (!string.Equals(certificate.Subject, "CN=localhost", StringComparison.Ordinal))
180-
{
181-
return false;
182-
}
183-
184-
foreach (var ext in certificate.Extensions)
185-
{
186-
if (string.Equals(ext.Oid?.Value, CertificateManager.AspNetHttpsOid, StringComparison.Ordinal))
187-
{
188-
return true;
189-
}
190-
}
191-
192-
return false;
179+
return string.Equals(certificate.Subject, CertificateManager.LocalhostHttpsDistinguishedName, StringComparison.Ordinal) &&
180+
CertificateManager.IsHttpsDevelopmentCertificate(certificate);
193181
}
194182

195183
private static bool TryGetCertificatePath(string applicationName, [NotNullWhen(true)] out string? path)

src/Shared/CertificateGeneration/CertificateManager.cs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ internal abstract class CertificateManager
2626
private const string ServerAuthenticationEnhancedKeyUsageOidFriendlyName = "Server Authentication";
2727

2828
private const string LocalhostHttpsDnsName = "localhost";
29-
private const string LocalhostHttpsDistinguishedName = "CN=" + LocalhostHttpsDnsName;
29+
internal const string LocalhostHttpsDistinguishedName = "CN=" + LocalhostHttpsDnsName;
3030

3131
public const int RSAMinimumKeySizeInBits = 2048;
3232

@@ -62,9 +62,21 @@ internal CertificateManager(string subject, int version)
6262
AspNetHttpsCertificateVersion = version;
6363
}
6464

65-
public static bool IsHttpsDevelopmentCertificate(X509Certificate2 certificate) =>
66-
certificate.Extensions.OfType<X509Extension>()
67-
.Any(e => string.Equals(AspNetHttpsOid, e.Oid?.Value, StringComparison.Ordinal));
65+
/// <remarks>
66+
/// This only checks if the certificate has the OID for ASP.NET Core HTTPS development certificates -
67+
/// it doesn't check the subject, validity, key usages, etc.
68+
/// </remarks>
69+
public static bool IsHttpsDevelopmentCertificate(X509Certificate2 certificate)
70+
{
71+
foreach (var extension in certificate.Extensions.OfType<X509Extension>())
72+
{
73+
if (string.Equals(AspNetHttpsOid, extension.Oid?.Value, StringComparison.Ordinal))
74+
{
75+
return true;
76+
}
77+
}
78+
return false;
79+
}
6880

6981
public IList<X509Certificate2> ListCertificates(
7082
StoreName storeName,
@@ -398,7 +410,10 @@ internal ImportCertificateResult ImportCertificate(string certificatePath, strin
398410
return ImportCertificateResult.InvalidCertificate;
399411
}
400412

401-
if (!IsHttpsDevelopmentCertificate(certificate))
413+
// Note that we're checking Subject, rather than LocalhostHttpsDistinguishedName,
414+
// because the tests use a different subject.
415+
if (!string.Equals(certificate.Subject, Subject, StringComparison.Ordinal) || // Kestrel requires this
416+
!IsHttpsDevelopmentCertificate(certificate))
402417
{
403418
if (Log.IsEnabled())
404419
{

src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ private void ListCertificates()
116116
var certificates = store.Certificates;
117117
foreach (var certificate in certificates)
118118
{
119-
Output.WriteLine($"Certificate: '{Convert.ToBase64String(certificate.Export(X509ContentType.Cert))}'.");
119+
Output.WriteLine($"Certificate: {certificate.Subject} '{Convert.ToBase64String(certificate.Export(X509ContentType.Cert))}'.");
120120
certificate.Dispose();
121121
}
122122

@@ -225,7 +225,7 @@ public void EnsureCreateHttpsCertificate_CanExportTheCertInPemFormat_WithoutKey(
225225
public void EnsureCreateHttpsCertificate_CanImport_ExportedPfx()
226226
{
227227
// Arrange
228-
const string CertificateName = nameof(EnsureCreateHttpsCertificate_DoesNotCreateACertificate_WhenThereIsAnExistingHttpsCertificates) + ".pfx";
228+
const string CertificateName = nameof(EnsureCreateHttpsCertificate_CanImport_ExportedPfx) + ".pfx";
229229
var certificatePassword = Guid.NewGuid().ToString();
230230

231231
_fixture.CleanupCertificates();
@@ -258,7 +258,7 @@ public void EnsureCreateHttpsCertificate_CanImport_ExportedPfx()
258258
public void EnsureCreateHttpsCertificate_CanImport_ExportedPfx_FailsIfThereAreCertificatesPresent()
259259
{
260260
// Arrange
261-
const string CertificateName = nameof(EnsureCreateHttpsCertificate_DoesNotCreateACertificate_WhenThereIsAnExistingHttpsCertificates) + ".pfx";
261+
const string CertificateName = nameof(EnsureCreateHttpsCertificate_CanImport_ExportedPfx_FailsIfThereAreCertificatesPresent) + ".pfx";
262262
var certificatePassword = Guid.NewGuid().ToString();
263263

264264
_fixture.CleanupCertificates();
@@ -280,6 +280,47 @@ public void EnsureCreateHttpsCertificate_CanImport_ExportedPfx_FailsIfThereAreCe
280280
Assert.Equal(ImportCertificateResult.ExistingCertificatesPresent, result);
281281
}
282282

283+
[ConditionalFact]
284+
[SkipOnHelix("https://github.com/dotnet/aspnetcore/issues/6720", Queues = "All.OSX")]
285+
public void EnsureCreateHttpsCertificate_CannotImportIfTheSubjectNameIsWrong()
286+
{
287+
// Arrange
288+
const string CertificateName = nameof(EnsureCreateHttpsCertificate_CannotImportIfTheSubjectNameIsWrong) + ".pfx";
289+
var certificatePassword = Guid.NewGuid().ToString();
290+
291+
_fixture.CleanupCertificates();
292+
293+
var now = DateTimeOffset.UtcNow;
294+
now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
295+
var creation = _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, isInteractive: false);
296+
Output.WriteLine(creation.ToString());
297+
ListCertificates();
298+
299+
var httpsCertificate = _manager.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: false).Single(c => c.Subject == TestCertificateSubject);
300+
301+
_manager.CleanupHttpsCertificates();
302+
303+
using var privateKey = httpsCertificate.GetRSAPrivateKey();
304+
var csr = new CertificateRequest(httpsCertificate.Subject + "Not", privateKey, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
305+
foreach (var extension in httpsCertificate.Extensions)
306+
{
307+
csr.CertificateExtensions.Add(extension);
308+
}
309+
var wrongSubjectCertificate = csr.CreateSelfSigned(httpsCertificate.NotBefore, httpsCertificate.NotAfter);
310+
311+
Assert.True(CertificateManager.IsHttpsDevelopmentCertificate(wrongSubjectCertificate));
312+
Assert.NotEqual(_manager.Subject, wrongSubjectCertificate.Subject);
313+
314+
File.WriteAllBytes(CertificateName, wrongSubjectCertificate.Export(X509ContentType.Pfx, certificatePassword));
315+
316+
// Act
317+
var result = _manager.ImportCertificate(CertificateName, certificatePassword);
318+
319+
// Assert
320+
Assert.Equal(ImportCertificateResult.NoDevelopmentHttpsCertificate, result);
321+
Assert.Empty(_manager.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: false));
322+
}
323+
283324
[ConditionalFact]
284325
[SkipOnHelix("https://github.com/dotnet/aspnetcore/issues/6720", Queues = "All.OSX")]
285326
public void EnsureCreateHttpsCertificate_CanExportTheCertInPemFormat_WithoutPassword()

0 commit comments

Comments
 (0)