Skip to content

Commit

Permalink
Make dev-certs import consistent with kestrel (dotnet#57014)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
amcasey authored Jul 26, 2024
1 parent f85deee commit 06155c0
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 22 deletions.
16 changes: 2 additions & 14 deletions src/Servers/Kestrel/Core/src/TlsConfigurationLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -176,20 +176,8 @@ public ListenOptions UseHttpsWithSni(

private static bool IsDevelopmentCertificate(X509Certificate2 certificate)
{
if (!string.Equals(certificate.Subject, "CN=localhost", StringComparison.Ordinal))
{
return false;
}

foreach (var ext in certificate.Extensions)
{
if (string.Equals(ext.Oid?.Value, CertificateManager.AspNetHttpsOid, StringComparison.Ordinal))
{
return true;
}
}

return false;
return string.Equals(certificate.Subject, CertificateManager.LocalhostHttpsDistinguishedName, StringComparison.Ordinal) &&
CertificateManager.IsHttpsDevelopmentCertificate(certificate);
}

private static bool TryGetCertificatePath(string applicationName, [NotNullWhen(true)] out string? path)
Expand Down
25 changes: 20 additions & 5 deletions src/Shared/CertificateGeneration/CertificateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal abstract class CertificateManager
private const string ServerAuthenticationEnhancedKeyUsageOidFriendlyName = "Server Authentication";

private const string LocalhostHttpsDnsName = "localhost";
private const string LocalhostHttpsDistinguishedName = "CN=" + LocalhostHttpsDnsName;
internal const string LocalhostHttpsDistinguishedName = "CN=" + LocalhostHttpsDnsName;

public const int RSAMinimumKeySizeInBits = 2048;

Expand Down Expand Up @@ -62,9 +62,21 @@ internal CertificateManager(string subject, int version)
AspNetHttpsCertificateVersion = version;
}

public static bool IsHttpsDevelopmentCertificate(X509Certificate2 certificate) =>
certificate.Extensions.OfType<X509Extension>()
.Any(e => string.Equals(AspNetHttpsOid, e.Oid?.Value, StringComparison.Ordinal));
/// <remarks>
/// This only checks if the certificate has the OID for ASP.NET Core HTTPS development certificates -
/// it doesn't check the subject, validity, key usages, etc.
/// </remarks>
public static bool IsHttpsDevelopmentCertificate(X509Certificate2 certificate)
{
foreach (var extension in certificate.Extensions.OfType<X509Extension>())
{
if (string.Equals(AspNetHttpsOid, extension.Oid?.Value, StringComparison.Ordinal))
{
return true;
}
}
return false;
}

public IList<X509Certificate2> ListCertificates(
StoreName storeName,
Expand Down Expand Up @@ -398,7 +410,10 @@ internal ImportCertificateResult ImportCertificate(string certificatePath, strin
return ImportCertificateResult.InvalidCertificate;
}

if (!IsHttpsDevelopmentCertificate(certificate))
// Note that we're checking Subject, rather than LocalhostHttpsDistinguishedName,
// because the tests use a different subject.
if (!string.Equals(certificate.Subject, Subject, StringComparison.Ordinal) || // Kestrel requires this
!IsHttpsDevelopmentCertificate(certificate))
{
if (Log.IsEnabled())
{
Expand Down
47 changes: 44 additions & 3 deletions src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ private void ListCertificates()
var certificates = store.Certificates;
foreach (var certificate in certificates)
{
Output.WriteLine($"Certificate: '{Convert.ToBase64String(certificate.Export(X509ContentType.Cert))}'.");
Output.WriteLine($"Certificate: {certificate.Subject} '{Convert.ToBase64String(certificate.Export(X509ContentType.Cert))}'.");
certificate.Dispose();
}

Expand Down Expand Up @@ -225,7 +225,7 @@ public void EnsureCreateHttpsCertificate_CanExportTheCertInPemFormat_WithoutKey(
public void EnsureCreateHttpsCertificate_CanImport_ExportedPfx()
{
// Arrange
const string CertificateName = nameof(EnsureCreateHttpsCertificate_DoesNotCreateACertificate_WhenThereIsAnExistingHttpsCertificates) + ".pfx";
const string CertificateName = nameof(EnsureCreateHttpsCertificate_CanImport_ExportedPfx) + ".pfx";
var certificatePassword = Guid.NewGuid().ToString();

_fixture.CleanupCertificates();
Expand Down Expand Up @@ -258,7 +258,7 @@ public void EnsureCreateHttpsCertificate_CanImport_ExportedPfx()
public void EnsureCreateHttpsCertificate_CanImport_ExportedPfx_FailsIfThereAreCertificatesPresent()
{
// Arrange
const string CertificateName = nameof(EnsureCreateHttpsCertificate_DoesNotCreateACertificate_WhenThereIsAnExistingHttpsCertificates) + ".pfx";
const string CertificateName = nameof(EnsureCreateHttpsCertificate_CanImport_ExportedPfx_FailsIfThereAreCertificatesPresent) + ".pfx";
var certificatePassword = Guid.NewGuid().ToString();

_fixture.CleanupCertificates();
Expand All @@ -280,6 +280,47 @@ public void EnsureCreateHttpsCertificate_CanImport_ExportedPfx_FailsIfThereAreCe
Assert.Equal(ImportCertificateResult.ExistingCertificatesPresent, result);
}

[ConditionalFact]
[SkipOnHelix("https://github.com/dotnet/aspnetcore/issues/6720", Queues = "All.OSX")]
public void EnsureCreateHttpsCertificate_CannotImportIfTheSubjectNameIsWrong()
{
// Arrange
const string CertificateName = nameof(EnsureCreateHttpsCertificate_CannotImportIfTheSubjectNameIsWrong) + ".pfx";
var certificatePassword = Guid.NewGuid().ToString();

_fixture.CleanupCertificates();

var now = DateTimeOffset.UtcNow;
now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
var creation = _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, isInteractive: false);
Output.WriteLine(creation.ToString());
ListCertificates();

var httpsCertificate = _manager.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: false).Single(c => c.Subject == TestCertificateSubject);

_manager.CleanupHttpsCertificates();

using var privateKey = httpsCertificate.GetRSAPrivateKey();
var csr = new CertificateRequest(httpsCertificate.Subject + "Not", privateKey, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
foreach (var extension in httpsCertificate.Extensions)
{
csr.CertificateExtensions.Add(extension);
}
var wrongSubjectCertificate = csr.CreateSelfSigned(httpsCertificate.NotBefore, httpsCertificate.NotAfter);

Assert.True(CertificateManager.IsHttpsDevelopmentCertificate(wrongSubjectCertificate));
Assert.NotEqual(_manager.Subject, wrongSubjectCertificate.Subject);

File.WriteAllBytes(CertificateName, wrongSubjectCertificate.Export(X509ContentType.Pfx, certificatePassword));

// Act
var result = _manager.ImportCertificate(CertificateName, certificatePassword);

// Assert
Assert.Equal(ImportCertificateResult.NoDevelopmentHttpsCertificate, result);
Assert.Empty(_manager.ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: false));
}

[ConditionalFact]
[SkipOnHelix("https://github.com/dotnet/aspnetcore/issues/6720", Queues = "All.OSX")]
public void EnsureCreateHttpsCertificate_CanExportTheCertInPemFormat_WithoutPassword()
Expand Down

0 comments on commit 06155c0

Please sign in to comment.