Skip to content

Commit

Permalink
Ignore the private key handle cert property for persisted keys
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bartonjs authored May 12, 2020
1 parent 8ab6940 commit 14835fb
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,9 @@ public ICertificatePal CopyWithPrivateKey(RSA rsa)

IntPtr privateKeyPtr;

// If the certificate has a key handle instead of a key prov info, return the
// If the certificate has a key handle without a key prov info, return the
// ephemeral key
if (!certificateContext.HasPersistedPrivateKey)
{
int cbData = IntPtr.Size;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,5 +157,131 @@ public static void ExportAsPfxWithPrivateKey()
}
}
}

[Fact]
[PlatformSpecific(TestPlatforms.Windows)]
[OuterLoop("Modifies user-persisted state")]
public static void ExportDoesNotCorruptPrivateKeyMethods()
{
string keyName = $"clrtest.{Guid.NewGuid():D}";
X509Store cuMy = new X509Store(StoreName.My, StoreLocation.CurrentUser);
cuMy.Open(OpenFlags.ReadWrite);
X509Certificate2 createdCert = null;
X509Certificate2 foundCert = null;
X509Certificate2 foundCert2 = null;

try
{
string commonName = nameof(ExportDoesNotCorruptPrivateKeyMethods);
string subject = $"CN={commonName},OU=.NET";

using (ImportedCollection toClean = new ImportedCollection(cuMy.Certificates))
{
X509Certificate2Collection coll = toClean.Collection;

using (ImportedCollection matches =
new ImportedCollection(coll.Find(X509FindType.FindBySubjectName, commonName, false)))
{
foreach (X509Certificate2 cert in matches.Collection)
{
cuMy.Remove(cert);
}
}
}

foreach (X509Certificate2 cert in cuMy.Certificates)
{
if (subject.Equals(cert.Subject))
{
cuMy.Remove(cert);
}

cert.Dispose();
}

CngKeyCreationParameters options = new CngKeyCreationParameters
{
ExportPolicy = CngExportPolicies.AllowExport | CngExportPolicies.AllowPlaintextExport,
};

using (CngKey key = CngKey.Create(CngAlgorithm.Rsa, keyName, options))
using (RSACng rsaCng = new RSACng(key))
{
CertificateRequest certReq = new CertificateRequest(
subject,
rsaCng,
HashAlgorithmName.SHA256,
RSASignaturePadding.Pkcs1);

DateTimeOffset now = DateTimeOffset.UtcNow.AddMinutes(-5);
createdCert = certReq.CreateSelfSigned(now, now.AddDays(1));
}

cuMy.Add(createdCert);

using (ImportedCollection toClean = new ImportedCollection(cuMy.Certificates))
{
X509Certificate2Collection matches = toClean.Collection.Find(
X509FindType.FindBySubjectName,
commonName,
validOnly: false);

Assert.Equal(1, matches.Count);
foundCert = matches[0];
}

Assert.False(HasEphemeralKey(foundCert));
foundCert.Export(X509ContentType.Pfx, "");
Assert.False(HasEphemeralKey(foundCert));

using (ImportedCollection toClean = new ImportedCollection(cuMy.Certificates))
{
X509Certificate2Collection matches = toClean.Collection.Find(
X509FindType.FindBySubjectName,
commonName,
validOnly: false);

Assert.Equal(1, matches.Count);
foundCert2 = matches[0];
}

Assert.False(HasEphemeralKey(foundCert2));
}
finally
{
if (createdCert != null)
{
cuMy.Remove(createdCert);
createdCert.Dispose();
}

cuMy.Dispose();

foundCert?.Dispose();
foundCert2?.Dispose();

try
{
CngKey key = CngKey.Open(keyName);
key.Delete();
key.Dispose();
}
catch (Exception)
{
}
}

bool HasEphemeralKey(X509Certificate2 c)
{
using (RSA key = c.GetRSAPrivateKey())
{
// This code is not defensive against the type changing, because it
// is in the source tree with the code that produces the value.
// Don't blind-cast like this in library or application code.
RSACng rsaCng = (RSACng)key;
return rsaCng.Key.IsEphemeral;
}
}
}
}
}

0 comments on commit 14835fb

Please sign in to comment.