Skip to content

Commit 638f0a5

Browse files
authored
Use a SafeHandle when duplicating a certificate context
CertDuplicateCertificateContext does not ensure the CERT_CONTEXT pointer it is incrementing has not been freed. If the duplicate and dispose race, duplicating a disposed handle will lead to unspecified behavior. For .NET 10, we can use the SafeHandle around the certificate before we duplicate the handle. For OOB, we don't have access to the internals, so we will continue to use the IntPtr Handle property.
1 parent 4c301c7 commit 638f0a5

File tree

4 files changed

+38
-1
lines changed

4 files changed

+38
-1
lines changed

src/libraries/Common/src/System/Security/Cryptography/X509Certificates/CertificateHelpers.Windows.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ internal static partial class CertificateHelpers
2525

2626
private static partial int GuessKeySpec(CngProvider provider, string keyName, bool machineKey, CngAlgorithmGroup? algorithmGroup);
2727

28+
private static partial SafeCertContextHandle DuplicateCertificateHandle(TCertificate certificate);
29+
2830
#if !SYSTEM_SECURITY_CRYPTOGRAPHY
2931
[SupportedOSPlatform("windows")]
3032
#endif
@@ -75,7 +77,7 @@ internal static TCertificate CopyWithPrivateKey(TCertificate certificate, MLDsa
7577
internal static T? GetPrivateKey<T>(TCertificate certificate, Func<CspParameters, T> createCsp, Func<CngKey, T?> createCng)
7678
where T : class, IDisposable
7779
{
78-
using (SafeCertContextHandle certContext = Interop.Crypt32.CertDuplicateCertificateContext(certificate.Handle))
80+
using (SafeCertContextHandle certContext = DuplicateCertificateHandle(certificate))
7981
{
8082
SafeNCryptKeyHandle? ncryptKey = TryAcquireCngPrivateKey(certContext, out CngKeyHandleOpenOptions cngHandleOptions);
8183
if (ncryptKey != null)

src/libraries/Microsoft.Bcl.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificateHelpers.Windows.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,10 @@ private static partial SafeNCryptKeyHandle CreateSafeNCryptKeyHandle(IntPtr hand
4242
return new SafeNCryptKeyHandle(handle, parentHandle);
4343
#endif
4444
}
45+
46+
private static partial SafeCertContextHandle DuplicateCertificateHandle(X509Certificate2 certificate)
47+
{
48+
return Interop.Crypt32.CertDuplicateCertificateContext(certificate.Handle);
49+
}
4550
}
4651
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificateHelpers.Windows.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,5 +166,33 @@ private static bool TryGuessDsaKeySpec(CspParameters cspParameters, out int keyS
166166
keySpec = 0;
167167
return false;
168168
}
169+
170+
private static partial SafeCertContextHandle DuplicateCertificateHandle(CertificatePal certificate)
171+
{
172+
SafeCertContextHandle? handle = certificate.SafeHandle;
173+
bool addedRef = false;
174+
175+
try
176+
{
177+
if (handle is not null)
178+
{
179+
handle.DangerousAddRef(ref addedRef);
180+
return Interop.Crypt32.CertDuplicateCertificateContext(handle.DangerousGetHandle());
181+
}
182+
}
183+
catch (ObjectDisposedException)
184+
{
185+
// Let this go to the invalid handle throw.
186+
}
187+
finally
188+
{
189+
if (addedRef)
190+
{
191+
handle!.DangerousRelease();
192+
}
193+
}
194+
195+
throw new CryptographicException(SR.Format(SR.Cryptography_InvalidHandle, nameof(handle)));
196+
}
169197
}
170198
}

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ public IntPtr Handle
5050
get { return _certContext.DangerousGetHandle(); }
5151
}
5252

53+
internal SafeCertContextHandle? SafeHandle => _certContext;
54+
5355
public string Issuer => GetIssuerOrSubject(issuer: true, reverse: true);
5456

5557
public string Subject => GetIssuerOrSubject(issuer: false, reverse: true);

0 commit comments

Comments
 (0)