aspnetcore/modules/DataProtection

commit 2af13658fcd601f951ed9ff60446b7a5241efbab
Author: Nate McMaster <[email protected]>
Date:   Thu Jul 5 11:31:46 2018 -0700

    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.

diff --git a/.vscode/launch.json b/.vscode/launch.json
new file mode 100644
index 00000000000..f4fc2e3731e
--- /dev/null
+++ b/.vscode/launch.json
@@ -0,0 +1,10 @@
+{
+    "configurations": [
+        {
+            "name": ".NET Core Attach",
+            "type": "coreclr",
+            "request": "attach",
+            "processId": "${command:pickProcess}"
+        }
+    ]
+}
diff --git a/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/EncryptedXmlDecryptor.cs b/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/EncryptedXmlDecryptor.cs
index e020ac7bb00..fee981b2d7c 100644
--- a/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/EncryptedXmlDecryptor.cs
+++ b/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/EncryptedXmlDecryptor.cs
@@ -2,7 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
 using System.Security.Cryptography.Xml;
@@ -63,8 +62,7 @@ namespace Microsoft.AspNetCore.DataProtection.XmlEncryption
             var elementToDecrypt = (XmlElement)xmlDocument.DocumentElement.FirstChild;
 
             // Perform the decryption and update the document in-place.
-            var decryptionCerts = _options?.KeyDecryptionCertificates;
-            var encryptedXml = new EncryptedXmlWithCertificateKeys(decryptionCerts, xmlDocument);
+            var encryptedXml = new EncryptedXmlWithCertificateKeys(_options, xmlDocument);
             _decryptor.PerformPreDecryptionSetup(encryptedXml);
 
             encryptedXml.DecryptDocument();
@@ -83,48 +81,40 @@ namespace Microsoft.AspNetCore.DataProtection.XmlEncryption
         /// </summary>
         private class EncryptedXmlWithCertificateKeys : EncryptedXml
         {
-            private readonly IReadOnlyDictionary<string, X509Certificate2> _certificates;
+            private readonly XmlKeyDecryptionOptions _options;
 
-            public EncryptedXmlWithCertificateKeys(IReadOnlyDictionary<string, X509Certificate2> certificates, XmlDocument document)
+            public EncryptedXmlWithCertificateKeys(XmlKeyDecryptionOptions options, XmlDocument document)
                 : base(document)
             {
-                _certificates = certificates;
+                _options = options;
             }
 
             public override byte[] DecryptEncryptedKey(EncryptedKey encryptedKey)
             {
-                byte[] key = base.DecryptEncryptedKey(encryptedKey);
-                if (key != null)
+                if (_options != null && _options.KeyDecryptionCertificateCount > 0)
                 {
-                    return key;
-                }
-
-                if (_certificates == null || _certificates.Count == 0)
-                {
-                    return null;
-                }
-
-                var keyInfoEnum = encryptedKey.KeyInfo?.GetEnumerator();
-                if (keyInfoEnum == null)
-                {
-                    return null;
-                }
-
-                while (keyInfoEnum.MoveNext())
-                {
-                    if (!(keyInfoEnum.Current is KeyInfoX509Data kiX509Data))
+                    var keyInfoEnum = encryptedKey.KeyInfo?.GetEnumerator();
+                    if (keyInfoEnum == null)
                     {
-                        continue;
+                        return null;
                     }
 
-                    key = GetKeyFromCert(encryptedKey, kiX509Data);
-                    if (key != null)
+                    while (keyInfoEnum.MoveNext())
                     {
-                        return key;
+                        if (!(keyInfoEnum.Current is KeyInfoX509Data kiX509Data))
+                        {
+                            continue;
+                        }
+
+                        byte[] key = GetKeyFromCert(encryptedKey, kiX509Data);
+                        if (key != null)
+                        {
+                            return key;
+                        }
                     }
                 }
 
-                return null;
+                return base.DecryptEncryptedKey(encryptedKey);
             }
 
             private byte[] GetKeyFromCert(EncryptedKey encryptedKey, KeyInfoX509Data keyInfo)
@@ -142,17 +132,25 @@ namespace Microsoft.AspNetCore.DataProtection.XmlEncryption
                         continue;
                     }
 
-                    if (!_certificates.TryGetValue(certInfo.Thumbprint, out var certificate))
+                    if (!_options.TryGetKeyDecryptionCertificates(certInfo, out var keyDecryptionCerts))
                     {
                         continue;
                     }
 
-                    using (RSA privateKey = certificate.GetRSAPrivateKey())
+                    foreach (var keyDecryptionCert in keyDecryptionCerts)
                     {
-                        if (privateKey != null)
+                        if (!keyDecryptionCert.HasPrivateKey)
+                        {
+                            continue;
+                        }
+
+                        using (RSA privateKey = keyDecryptionCert.GetRSAPrivateKey())
                         {
-                            var useOAEP = encryptedKey.EncryptionMethod?.KeyAlgorithm == XmlEncRSAOAEPUrl;
-                            return DecryptKey(encryptedKey.CipherData.CipherValue, privateKey, useOAEP);
+                            if (privateKey != null)
+                            {
+                                var useOAEP = encryptedKey.EncryptionMethod?.KeyAlgorithm == XmlEncRSAOAEPUrl;
+                                return DecryptKey(encryptedKey.CipherData.CipherValue, privateKey, useOAEP);
+                            }
                         }
                     }
                 }
diff --git a/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/XmlKeyDecryptionOptions.cs b/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/XmlKeyDecryptionOptions.cs
index 01999c224db..7da598816f7 100644
--- a/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/XmlKeyDecryptionOptions.cs
+++ b/src/Microsoft.AspNetCore.DataProtection/XmlEncryption/XmlKeyDecryptionOptions.cs
@@ -12,16 +12,28 @@ namespace Microsoft.AspNetCore.DataProtection.XmlEncryption
     /// </summary>
     internal class XmlKeyDecryptionOptions
     {
-        private readonly Dictionary<string, X509Certificate2> _certs = new Dictionary<string, X509Certificate2>(StringComparer.Ordinal);
+        private readonly Dictionary<string, List<X509Certificate2>> _certs = new Dictionary<string, List<X509Certificate2>>(StringComparer.Ordinal);
 
-        /// <summary>
-        /// A mapping of key thumbprint to the X509Certificate2
-        /// </summary>
-        public IReadOnlyDictionary<string, X509Certificate2> KeyDecryptionCertificates => _certs;
+        public int KeyDecryptionCertificateCount => _certs.Count;
+
+        public bool TryGetKeyDecryptionCertificates(X509Certificate2 certInfo, out IReadOnlyList<X509Certificate2> keyDecryptionCerts)
+        {
+            var key = GetKey(certInfo);
+            var retVal = _certs.TryGetValue(key, out var keyDecryptionCertsRetVal);
+            keyDecryptionCerts = keyDecryptionCertsRetVal;
+            return retVal;
+        }
 
         public void AddKeyDecryptionCertificate(X509Certificate2 certificate)
         {
-            _certs[certificate.Thumbprint] = certificate;
+            var key = GetKey(certificate);
+            if (!_certs.TryGetValue(key, out var certificates))
+            {
+                certificates = _certs[key] = new List<X509Certificate2>();
+            }
+            certificates.Add(certificate);
         }
+
+        private string GetKey(X509Certificate2 cert) => cert.Thumbprint;
     }
 }
diff --git a/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/DataProtectionProviderTests.cs b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/DataProtectionProviderTests.cs
index d20332c1e29..a66ebec2e81 100644
--- a/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/DataProtectionProviderTests.cs
+++ b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/DataProtectionProviderTests.cs
@@ -3,7 +3,6 @@
 
 using System;
 using System.IO;
-using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
@@ -13,7 +12,6 @@ using Microsoft.AspNetCore.DataProtection.Repositories;
 using Microsoft.AspNetCore.DataProtection.Test.Shared;
 using Microsoft.AspNetCore.Testing.xunit;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Options;
 using Moq;
@@ -120,10 +118,12 @@ namespace Microsoft.AspNetCore.DataProtection
         public void System_UsesProvidedDirectoryAndCertificate()
         {
             var filePath = Path.Combine(GetTestFilesPath(), "TestCert.pfx");
-            var store = new X509Store(StoreName.My, StoreLocation.CurrentUser);
-            store.Open(OpenFlags.ReadWrite);
-            store.Add(new X509Certificate2(filePath, "password", X509KeyStorageFlags.Exportable));
-            store.Close();
+            using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser))
+            {
+                store.Open(OpenFlags.ReadWrite);
+                store.Add(new X509Certificate2(filePath, "password", X509KeyStorageFlags.Exportable));
+                store.Close();
+            }
 
             WithUniqueTempDirectory(directory =>
             {
@@ -139,7 +139,12 @@ namespace Microsoft.AspNetCore.DataProtection
 
                     // Step 2: instantiate the system and round-trip a payload
                     var protector = DataProtectionProvider.Create(directory, certificate).CreateProtector("purpose");
-                    Assert.Equal("payload", protector.Unprotect(protector.Protect("payload")));
+                    var data = protector.Protect("payload");
+
+                    // add a cert without the private key to ensure the decryption will still fallback to the cert store
+                    var certWithoutKey = new X509Certificate2(Path.Combine(GetTestFilesPath(), "TestCertWithoutPrivateKey.pfx"), "password");
+                    var unprotector = DataProtectionProvider.Create(directory, o => o.UnprotectKeysWithAnyCertificate(certWithoutKey)).CreateProtector("purpose");
+                    Assert.Equal("payload", unprotector.Unprotect(data));
 
                     // Step 3: validate that there's now a single key in the directory and that it's is protected using the certificate
                     var allFiles = directory.GetFiles();
@@ -157,6 +162,50 @@ namespace Microsoft.AspNetCore.DataProtection
             });
         }
 
+        [ConditionalFact]
+        [X509StoreIsAvailable(StoreName.My, StoreLocation.CurrentUser)]
+        public void System_UsesProvidedCertificateNotFromStore()
+        {
+            using (var store = new X509Store(StoreName.My, StoreLocation.CurrentUser))
+            {
+                store.Open(OpenFlags.ReadWrite);
+                var certWithoutKey = new X509Certificate2(Path.Combine(GetTestFilesPath(), "TestCert3WithoutPrivateKey.pfx"), "password3", X509KeyStorageFlags.Exportable);
+                Assert.False(certWithoutKey.HasPrivateKey, "Cert should not have private key");
+                store.Add(certWithoutKey);
+                store.Close();
+            }
+
+            WithUniqueTempDirectory(directory =>
+            {
+                using (var certificateStore = new X509Store(StoreName.My, StoreLocation.CurrentUser))
+                {
+                    certificateStore.Open(OpenFlags.ReadWrite);
+                    var certInStore = certificateStore.Certificates.Find(X509FindType.FindBySubjectName, "TestCert", false)[0];
+                    Assert.NotNull(certInStore);
+                    Assert.False(certInStore.HasPrivateKey);
+
+                    try
+                    {
+                        var certWithKey = new X509Certificate2(Path.Combine(GetTestFilesPath(), "TestCert3.pfx"), "password3");
+
+                        var protector = DataProtectionProvider.Create(directory, certWithKey).CreateProtector("purpose");
+                        var data = protector.Protect("payload");
+
+                        var keylessUnprotector = DataProtectionProvider.Create(directory).CreateProtector("purpose");
+                        Assert.Throws<CryptographicException>(() => keylessUnprotector.Unprotect(data));
+
+                        var unprotector = DataProtectionProvider.Create(directory, o => o.UnprotectKeysWithAnyCertificate(certInStore, certWithKey)).CreateProtector("purpose");
+                        Assert.Equal("payload", unprotector.Unprotect(data));
+                    }
+                    finally
+                    {
+                        certificateStore.Remove(certInStore);
+                        certificateStore.Close();
+                    }
+                }
+            });
+        }
+
         [Fact]
         public void System_UsesInMemoryCertificate()
         {
@@ -242,7 +291,7 @@ namespace Microsoft.AspNetCore.DataProtection
         /// </summary>
         private static void WithUniqueTempDirectory(Action<DirectoryInfo> testCode)
         {
-            string uniqueTempPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
+            string uniqueTempPath = Path.Combine(AppContext.BaseDirectory, Path.GetRandomFileName());
             var dirInfo = new DirectoryInfo(uniqueTempPath);
             try
             {
diff --git a/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3.pfx b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3.pfx
new file mode 100644
index 00000000000..364251ba09d
Binary files /dev/null and b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3.pfx differ
diff --git a/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3WithoutPrivateKey.pfx b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3WithoutPrivateKey.pfx
new file mode 100644
index 00000000000..9776e9006d7
Binary files /dev/null and b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCert3WithoutPrivateKey.pfx differ
diff --git a/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCertWithoutPrivateKey.pfx b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCertWithoutPrivateKey.pfx
new file mode 100644
index 00000000000..812374c50c3
Binary files /dev/null and b/test/Microsoft.AspNetCore.DataProtection.Extensions.Test/TestFiles/TestCertWithoutPrivateKey.pfx differ