Skip to content

Commit ea2a26b

Browse files
Anipikbartonjs
andauthored
[release/5.0-preview5] Ignore the private key handle cert property for persisted keys (#36312)
* Ignore the private key handle cert property for persisted keys When a CERT_CONTEXT value has both property 2 (prov info) and property 78 (ncrypt key handle), prefer to load based on the property 2 state. This avoids a scenario where calling Get[Algorithm]PrivateKey sets the 'CLR IsEphemeral' property on a persisted key, preventing future loads of that key. An alternative approach of preferring the loaded key over the cold-load was not selected to avoid value contamination of ephemeral properties set on the CngKey object directly after the caller calls Get[Algorithm]PrivateKey. * unblocking build Co-authored-by: Jeremy Barton <jbarton@microsoft.com>
1 parent 9491e7a commit ea2a26b

File tree

3 files changed

+132
-3
lines changed

3 files changed

+132
-3
lines changed

src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.PrivateKey.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,9 @@ public ICertificatePal CopyWithPrivateKey(RSA rsa)
215215

216216
IntPtr privateKeyPtr;
217217

218-
// If the certificate has a key handle instead of a key prov info, return the
218+
// If the certificate has a key handle without a key prov info, return the
219219
// ephemeral key
220+
if (!certificateContext.HasPersistedPrivateKey)
220221
{
221222
int cbData = IntPtr.Size;
222223

src/libraries/System.Security.Cryptography.X509Certificates/tests/ExportTests.cs

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,5 +157,131 @@ public static void ExportAsPfxWithPrivateKey()
157157
}
158158
}
159159
}
160+
161+
[Fact]
162+
[PlatformSpecific(TestPlatforms.Windows)]
163+
[OuterLoop("Modifies user-persisted state")]
164+
public static void ExportDoesNotCorruptPrivateKeyMethods()
165+
{
166+
string keyName = $"clrtest.{Guid.NewGuid():D}";
167+
X509Store cuMy = new X509Store(StoreName.My, StoreLocation.CurrentUser);
168+
cuMy.Open(OpenFlags.ReadWrite);
169+
X509Certificate2 createdCert = null;
170+
X509Certificate2 foundCert = null;
171+
X509Certificate2 foundCert2 = null;
172+
173+
try
174+
{
175+
string commonName = nameof(ExportDoesNotCorruptPrivateKeyMethods);
176+
string subject = $"CN={commonName},OU=.NET";
177+
178+
using (ImportedCollection toClean = new ImportedCollection(cuMy.Certificates))
179+
{
180+
X509Certificate2Collection coll = toClean.Collection;
181+
182+
using (ImportedCollection matches =
183+
new ImportedCollection(coll.Find(X509FindType.FindBySubjectName, commonName, false)))
184+
{
185+
foreach (X509Certificate2 cert in matches.Collection)
186+
{
187+
cuMy.Remove(cert);
188+
}
189+
}
190+
}
191+
192+
foreach (X509Certificate2 cert in cuMy.Certificates)
193+
{
194+
if (subject.Equals(cert.Subject))
195+
{
196+
cuMy.Remove(cert);
197+
}
198+
199+
cert.Dispose();
200+
}
201+
202+
CngKeyCreationParameters options = new CngKeyCreationParameters
203+
{
204+
ExportPolicy = CngExportPolicies.AllowExport | CngExportPolicies.AllowPlaintextExport,
205+
};
206+
207+
using (CngKey key = CngKey.Create(CngAlgorithm.Rsa, keyName, options))
208+
using (RSACng rsaCng = new RSACng(key))
209+
{
210+
CertificateRequest certReq = new CertificateRequest(
211+
subject,
212+
rsaCng,
213+
HashAlgorithmName.SHA256,
214+
RSASignaturePadding.Pkcs1);
215+
216+
DateTimeOffset now = DateTimeOffset.UtcNow.AddMinutes(-5);
217+
createdCert = certReq.CreateSelfSigned(now, now.AddDays(1));
218+
}
219+
220+
cuMy.Add(createdCert);
221+
222+
using (ImportedCollection toClean = new ImportedCollection(cuMy.Certificates))
223+
{
224+
X509Certificate2Collection matches = toClean.Collection.Find(
225+
X509FindType.FindBySubjectName,
226+
commonName,
227+
validOnly: false);
228+
229+
Assert.Equal(1, matches.Count);
230+
foundCert = matches[0];
231+
}
232+
233+
Assert.False(HasEphemeralKey(foundCert));
234+
foundCert.Export(X509ContentType.Pfx, "");
235+
Assert.False(HasEphemeralKey(foundCert));
236+
237+
using (ImportedCollection toClean = new ImportedCollection(cuMy.Certificates))
238+
{
239+
X509Certificate2Collection matches = toClean.Collection.Find(
240+
X509FindType.FindBySubjectName,
241+
commonName,
242+
validOnly: false);
243+
244+
Assert.Equal(1, matches.Count);
245+
foundCert2 = matches[0];
246+
}
247+
248+
Assert.False(HasEphemeralKey(foundCert2));
249+
}
250+
finally
251+
{
252+
if (createdCert != null)
253+
{
254+
cuMy.Remove(createdCert);
255+
createdCert.Dispose();
256+
}
257+
258+
cuMy.Dispose();
259+
260+
foundCert?.Dispose();
261+
foundCert2?.Dispose();
262+
263+
try
264+
{
265+
CngKey key = CngKey.Open(keyName);
266+
key.Delete();
267+
key.Dispose();
268+
}
269+
catch (Exception)
270+
{
271+
}
272+
}
273+
274+
bool HasEphemeralKey(X509Certificate2 c)
275+
{
276+
using (RSA key = c.GetRSAPrivateKey())
277+
{
278+
// This code is not defensive against the type changing, because it
279+
// is in the source tree with the code that produces the value.
280+
// Don't blind-cast like this in library or application code.
281+
RSACng rsaCng = (RSACng)key;
282+
return rsaCng.Key.IsEphemeral;
283+
}
284+
}
285+
}
160286
}
161287
}

src/libraries/pkg/baseline/packageIndex.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,12 +543,14 @@
543543
"3.1.0",
544544
"3.1.1",
545545
"3.1.2",
546-
"3.1.3"
546+
"3.1.3",
547+
"3.1.4"
547548
],
548549
"BaselineVersion": "5.0.0",
549550
"InboxOn": {},
550551
"AssemblyVersionInPackageVersion": {
551-
"3.1.3.0": "5.0.0",
552+
"3.1.3.0": "3.1.3",
553+
"3.1.4.0": "3.1.4",
552554
"5.0.0.0": "5.0.0"
553555
}
554556
},

0 commit comments

Comments
 (0)