Skip to content
This repository was archived by the owner on Oct 17, 2018. It is now read-only.

Commit 2af1365

Browse files
natemcmasternatemcmaster
authored
Unprotect key material with the local cache of certificates before checking the cert store
In some cases, private keys for certificates is not completely available. When attempting to decrypt key material, this can cause 'CryptographicException: Keyset does not exist'. This changes the order in which key material decryption looks up private keys to first key the certificate options provided explicitly to the API, and then falling back to the cert store for decryption keys.
1 parent a5c86af commit 2af1365

File tree

7 files changed

+118
-49
lines changed

7 files changed

+118
-49
lines changed

‎.vscode/launch.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"configurations": [
3+
{
4+
"name": ".NET Core Attach",
5+
"type": "coreclr",
6+
"request": "attach",
7+
"processId": "${command:pickProcess}"
8+
}
9+
]
10+
}

‎src/Microsoft.AspNetCore.DataProtection/XmlEncryption/EncryptedXmlDecryptor.cs

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Collections.Generic;
65
using System.Security.Cryptography;
76
using System.Security.Cryptography.X509Certificates;
87
using System.Security.Cryptography.Xml;
@@ -63,8 +62,7 @@ public XElement Decrypt(XElement encryptedElement)
6362
var elementToDecrypt = (XmlElement)xmlDocument.DocumentElement.FirstChild;
6463

6564
// Perform the decryption and update the document in-place.
66-
var decryptionCerts = _options?.KeyDecryptionCertificates;
67-
var encryptedXml = new EncryptedXmlWithCertificateKeys(decryptionCerts, xmlDocument);
65+
var encryptedXml = new EncryptedXmlWithCertificateKeys(_options, xmlDocument);
6866
_decryptor.PerformPreDecryptionSetup(encryptedXml);
6967

7068
encryptedXml.DecryptDocument();
@@ -83,48 +81,40 @@ void IInternalEncryptedXmlDecryptor.PerformPreDecryptionSetup(EncryptedXml encry
8381
/// </summary>
8482
private class EncryptedXmlWithCertificateKeys : EncryptedXml
8583
{
86-
private readonly IReadOnlyDictionary<string, X509Certificate2> _certificates;
84+
private readonly XmlKeyDecryptionOptions _options;
8785

88-
public EncryptedXmlWithCertificateKeys(IReadOnlyDictionary<string, X509Certificate2> certificates, XmlDocument document)
86+
public EncryptedXmlWithCertificateKeys(XmlKeyDecryptionOptions options, XmlDocument document)
8987
: base(document)
9088
{
91-
_certificates = certificates;
89+
_options = options;
9290
}
9391

9492
public override byte[] DecryptEncryptedKey(EncryptedKey encryptedKey)
9593
{
96-
byte[] key = base.DecryptEncryptedKey(encryptedKey);
97-
if (key != null)
94+
if (_options != null && _options.KeyDecryptionCertificateCount > 0)
9895
{
99-
return key;
100-
}
101-
102-
if (_certificates == null || _certificates.Count == 0)
103-
{
104-
return null;
105-
}
106-
107-
var keyInfoEnum = encryptedKey.KeyInfo?.GetEnumerator();
108-
if (keyInfoEnum == null)
109-
{
110-
return null;
111-
}
112-
113-
while (keyInfoEnum.MoveNext())
114-
{
115-
if (!(keyInfoEnum.Current is KeyInfoX509Data kiX509Data))
96+
var keyInfoEnum = encryptedKey.KeyInfo?.GetEnumerator();
97+
if (keyInfoEnum == null)
11698
{
117-
continue;
99+
return null;
118100
}
119101

120-
key = GetKeyFromCert(encryptedKey, kiX509Data);
121-
if (key != null)
102+
while (keyInfoEnum.MoveNext())
122103
{
123-
return key;
104+
if (!(keyInfoEnum.Current is KeyInfoX509Data kiX509Data))
105+
{
106+
continue;
107+
}
108+
109+
byte[] key = GetKeyFromCert(encryptedKey, kiX509Data);
110+
if (key != null)
111+
{
112+
return key;
113+
}
124114
}
125115
}
126116

127-
return null;
117+
return base.DecryptEncryptedKey(encryptedKey);
128118
}
129119

130120
private byte[] GetKeyFromCert(EncryptedKey encryptedKey, KeyInfoX509Data keyInfo)
@@ -142,17 +132,25 @@ private byte[] GetKeyFromCert(EncryptedKey encryptedKey, KeyInfoX509Data keyInfo
142132
continue;
143133
}
144134

145-
if (!_certificates.TryGetValue(certInfo.Thumbprint, out var certificate))
135+
if (!_options.TryGetKeyDecryptionCertificates(certInfo, out var keyDecryptionCerts))
146136
{
147137
continue;
148138
}
149139

150-
using (RSA privateKey = certificate.GetRSAPrivateKey())
140+
foreach (var keyDecryptionCert in keyDecryptionCerts)
151141
{
152-
if (privateKey != null)
142+
if (!keyDecryptionCert.HasPrivateKey)
143+
{
144+
continue;
145+
}
146+
147+
using (RSA privateKey = keyDecryptionCert.GetRSAPrivateKey())
153148
{
154-
var useOAEP = encryptedKey.EncryptionMethod?.KeyAlgorithm == XmlEncRSAOAEPUrl;
155-
return DecryptKey(encryptedKey.CipherData.CipherValue, privateKey, useOAEP);
149+
if (privateKey != null)
150+
{
151+
var useOAEP = encryptedKey.EncryptionMethod?.KeyAlgorithm == XmlEncRSAOAEPUrl;
152+
return DecryptKey(encryptedKey.CipherData.CipherValue, privateKey, useOAEP);
153+
}
156154
}
157155
}
158156
}

‎src/Microsoft.AspNetCore.DataProtection/XmlEncryption/XmlKeyDecryptionOptions.cs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,28 @@ namespace Microsoft.AspNetCore.DataProtection.XmlEncryption
1212
/// </summary>
1313
internal class XmlKeyDecryptionOptions
1414
{
15-
private readonly Dictionary<string, X509Certificate2> _certs = new Dictionary<string, X509Certificate2>(StringComparer.Ordinal);
15+
private readonly Dictionary<string, List<X509Certificate2>> _certs = new Dictionary<string, List<X509Certificate2>>(StringComparer.Ordinal);
1616

17-
/// <summary>
18-
/// A mapping of key thumbprint to the X509Certificate2
19-
/// </summary>
20-
public IReadOnlyDictionary<string, X509Certificate2> KeyDecryptionCertificates => _certs;
17+
public int KeyDecryptionCertificateCount => _certs.Count;
18+
19+
public bool TryGetKeyDecryptionCertificates(X509Certificate2 certInfo, out IReadOnlyList<X509Certificate2> keyDecryptionCerts)
20+
{
21+
var key = GetKey(certInfo);
22+
var retVal = _certs.TryGetValue(key, out var keyDecryptionCertsRetVal);
23+
keyDecryptionCerts = keyDecryptionCertsRetVal;
24+
return retVal;
25+
}
2126

2227
public void AddKeyDecryptionCertificate(X509Certificate2 certificate)
2328
{
24-
_certs[certificate.Thumbprint] = certificate;
29+
var key = GetKey(certificate);
30+
if (!_certs.TryGetValue(key, out var certificates))
31+
{
32+
certificates = _certs[key] = new List<X509Certificate2>();
33+
}
34+
certificates.Add(certificate);
2535
}
36+
37+
private string GetKey(X509Certificate2 cert) => cert.Thumbprint;
2638
}
2739
}

‎test/Microsoft.AspNetCore.DataProtection.Extensions.Test/DataProtectionProviderTests.cs

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System;
55
using System.IO;
6-
using System.Reflection;
76
using System.Runtime.InteropServices;
87
using System.Security.Cryptography;
98
using System.Security.Cryptography.X509Certificates;
@@ -13,7 +12,6 @@
1312
using Microsoft.AspNetCore.DataProtection.Test.Shared;
1413
using Microsoft.AspNetCore.Testing.xunit;
1514
using Microsoft.Extensions.DependencyInjection;
16-
using Microsoft.Extensions.Logging;
1715
using Microsoft.Extensions.Logging.Abstractions;
1816
using Microsoft.Extensions.Options;
1917
using Moq;
@@ -120,10 +118,12 @@ public void System_UsesProvidedDirectory_WithConfigurationCallback()
120118
public void System_UsesProvidedDirectoryAndCertificate()
121119
{
122120
var filePath = Path.Combine(GetTestFilesPath(), "TestCert.pfx");
123-
var store = new X509Store(StoreName.My, StoreLocation.CurrentUser);
124-
store.Open(OpenFlags.ReadWrite);
125-
store.Add(new X509Certificate2(filePath, "password", X509KeyStorageFlags.Exportable));
126-
store.Close();
121+
using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser))
122+
{
123+
store.Open(OpenFlags.ReadWrite);
124+
store.Add(new X509Certificate2(filePath, "password", X509KeyStorageFlags.Exportable));
125+
store.Close();
126+
}
127127

128128
WithUniqueTempDirectory(directory =>
129129
{
@@ -139,7 +139,12 @@ public void System_UsesProvidedDirectoryAndCertificate()
139139

140140
// Step 2: instantiate the system and round-trip a payload
141141
var protector = DataProtectionProvider.Create(directory, certificate).CreateProtector("purpose");
142-
Assert.Equal("payload", protector.Unprotect(protector.Protect("payload")));
142+
var data = protector.Protect("payload");
143+
144+
// add a cert without the private key to ensure the decryption will still fallback to the cert store
145+
var certWithoutKey = new X509Certificate2(Path.Combine(GetTestFilesPath(), "TestCertWithoutPrivateKey.pfx"), "password");
146+
var unprotector = DataProtectionProvider.Create(directory, o => o.UnprotectKeysWithAnyCertificate(certWithoutKey)).CreateProtector("purpose");
147+
Assert.Equal("payload", unprotector.Unprotect(data));
143148

144149
// Step 3: validate that there's now a single key in the directory and that it's is protected using the certificate
145150
var allFiles = directory.GetFiles();
@@ -157,6 +162,50 @@ public void System_UsesProvidedDirectoryAndCertificate()
157162
});
158163
}
159164

165+
[ConditionalFact]
166+
[X509StoreIsAvailable(StoreName.My, StoreLocation.CurrentUser)]
167+
public void System_UsesProvidedCertificateNotFromStore()
168+
{
169+
using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser))
170+
{
171+
store.Open(OpenFlags.ReadWrite);
172+
var certWithoutKey = new X509Certificate2(Path.Combine(GetTestFilesPath(), "TestCert3WithoutPrivateKey.pfx"), "password3", X509KeyStorageFlags.Exportable);
173+
Assert.False(certWithoutKey.HasPrivateKey, "Cert should not have private key");
174+
store.Add(certWithoutKey);
175+
store.Close();
176+
}
177+
178+
WithUniqueTempDirectory(directory =>
179+
{
180+
using (var certificateStore = new X509Store(StoreName.My, StoreLocation.CurrentUser))
181+
{
182+
certificateStore.Open(OpenFlags.ReadWrite);
183+
var certInStore = certificateStore.Certificates.Find(X509FindType.FindBySubjectName, "TestCert", false)[0];
184+
Assert.NotNull(certInStore);
185+
Assert.False(certInStore.HasPrivateKey);
186+
187+
try
188+
{
189+
var certWithKey = new X509Certificate2(Path.Combine(GetTestFilesPath(), "TestCert3.pfx"), "password3");
190+
191+
var protector = DataProtectionProvider.Create(directory, certWithKey).CreateProtector("purpose");
192+
var data = protector.Protect("payload");
193+
194+
var keylessUnprotector = DataProtectionProvider.Create(directory).CreateProtector("purpose");
195+
Assert.Throws<CryptographicException>(() => keylessUnprotector.Unprotect(data));
196+
197+
var unprotector = DataProtectionProvider.Create(directory, o => o.UnprotectKeysWithAnyCertificate(certInStore, certWithKey)).CreateProtector("purpose");
198+
Assert.Equal("payload", unprotector.Unprotect(data));
199+
}
200+
finally
201+
{
202+
certificateStore.Remove(certInStore);
203+
certificateStore.Close();
204+
}
205+
}
206+
});
207+
}
208+
160209
[Fact]
161210
public void System_UsesInMemoryCertificate()
162211
{
@@ -242,7 +291,7 @@ public void System_CanUnprotectWithCert()
242291
/// </summary>
243292
private static void WithUniqueTempDirectory(Action<DirectoryInfo> testCode)
244293
{
245-
string uniqueTempPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
294+
string uniqueTempPath = Path.Combine(AppContext.BaseDirectory, Path.GetRandomFileName());
246295
var dirInfo = new DirectoryInfo(uniqueTempPath);
247296
try
248297
{

‎test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3.pfx

2.37 KB
Binary file not shown.

‎test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3WithoutPrivateKey.pfx

1.02 KB
Binary file not shown.

‎test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCertWithoutPrivateKey.pfx

968 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)