Skip to content

Use a mutex to guard cert creation on windows #11546

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.

Sign up for GitHub

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 1 commit into from
Jun 25, 2019
Merged

Use a mutex to guard cert creation on windows #11546

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 24 additions & 6 deletions src/Servers/Kestrel/shared/test/TestResources.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
// 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;
using System.IO;
using System.Runtime.InteropServices;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using Xunit;

namespace Microsoft.AspNetCore.Testing
{
Expand All @@ -13,14 +17,28 @@ public static class TestResources
public static string TestCertificatePath { get; } = Path.Combine(_baseDir, "testCert.pfx");
public static string GetCertPath(string name) => Path.Combine(_baseDir, name);

public static X509Certificate2 GetTestCertificate()
{
return new X509Certificate2(TestCertificatePath, "testPassword");
}
private const int MutexTimeout = 120 * 1000;
private static readonly Mutex importPfxMutex = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ?
new Mutex(initiallyOwned: false, "Global\\KestrelTests.Certificates.LoadPfxCertificate") :
null;

public static X509Certificate2 GetTestCertificate(string certName)
public static X509Certificate2 GetTestCertificate(string certName = "testCert.pfx")
{
return new X509Certificate2(GetCertPath(certName), "testPassword");
// On Windows, applications should not import PFX files in parallel to avoid a known system-level
// race condition bug in native code which can cause crashes/corruption of the certificate state.
if (importPfxMutex != null)
{
Assert.True(importPfxMutex.WaitOne(MutexTimeout), "Cannot acquire the global certificate mutex.");
Copy link
Member Author

Choose a reason for hiding this comment

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

Hopefully, this won't cause more starvation right? 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Famous last words :)

Copy link
Member

Choose a reason for hiding this comment

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

The tread-safety issue is related to initialization on Windows, not usage right? I wonder if we should also cache the results for a given certName in a given process. VS is telling me this is called from 33 different places throughout the Kestrel test projects.

That way you wouldn't have to take a system-wide lock for all 33 calls. You could first acquire a normal process-wide lock and see if a cached cert has already been loaded by the process.

}

try
{
return new X509Certificate2(GetCertPath(certName), "testPassword");
}
finally
{
importPfxMutex?.ReleaseMutex();
}
}
}
}