From e99d21c51202a204cb7b69f54254f07b6e0e3c7e Mon Sep 17 00:00:00 2001
From: Jeremy Barton <jbarton@microsoft.com>
Date: Wed, 18 Mar 2020 09:12:43 -0700
Subject: [PATCH] Don't delete private keys detected from SerializedCert
 imports

Port of https://github.com/dotnet/runtime/commit/2bcf71309065044c291de38809912b14437ee804
---
 .../Pal.Windows/CertificatePal.Import.cs      | 11 +++++--
 .../tests/CertTests.cs                        | 22 ++++++++++++++
 .../tests/CollectionTests.cs                  | 29 +++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs b/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs
index c5aa53f58378..8c8bac17981f 100644
--- a/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs
+++ b/src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs
@@ -34,7 +34,7 @@ private static ICertificatePal FromBlobOrFile(byte[] rawData, string fileName, S
             bool loadFromFile = (fileName != null);
 
             PfxCertStoreFlags pfxCertStoreFlags = MapKeyStorageFlags(keyStorageFlags);
-            bool persistKeySet = (0 != (keyStorageFlags & X509KeyStorageFlags.PersistKeySet));
+            bool deleteKeyContainer = false;
 
             CertEncodingType msgAndCertEncodingType;
             ContentType contentType;
@@ -86,9 +86,16 @@ out pCertContext
                         if (loadFromFile)
                             rawData = File.ReadAllBytes(fileName);
                         pCertContext = FilterPFXStore(rawData, password, pfxCertStoreFlags);
+
+                        // If PersistKeySet is set we don't delete the key, so that it persists.
+                        // If EphemeralKeySet is set we don't delete the key, because there's no file, so it's a wasteful call.
+                        const X509KeyStorageFlags DeleteUnless =
+                            X509KeyStorageFlags.PersistKeySet | X509KeyStorageFlags.EphemeralKeySet;
+
+                        deleteKeyContainer = ((keyStorageFlags & DeleteUnless) == 0);
                     }
 
-                    CertificatePal pal = new CertificatePal(pCertContext, deleteKeyContainer: !persistKeySet);
+                    CertificatePal pal = new CertificatePal(pCertContext, deleteKeyContainer);
                     pCertContext = null;
                     return pal;
                 }
diff --git a/src/System.Security.Cryptography.X509Certificates/tests/CertTests.cs b/src/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
index 559c576c5a70..0f63b7324131 100644
--- a/src/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
+++ b/src/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
@@ -354,6 +354,28 @@ public static void ExportPublicKeyAsPkcs12()
             }
         }
 
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public static void SerializedCertDisposeDoesNotRemoveKeyFile()
+        {
+            using (X509Certificate2 fromPfx = new X509Certificate2(TestData.PfxData, TestData.PfxDataPassword))
+            {
+                Assert.True(fromPfx.HasPrivateKey, "fromPfx.HasPrivateKey - before");
+
+                byte[] serializedCert = fromPfx.Export(X509ContentType.SerializedCert);
+
+                using (X509Certificate2 fromSerialized = new X509Certificate2(serializedCert))
+                {
+                    Assert.True(fromSerialized.HasPrivateKey, "fromSerialized.HasPrivateKey");
+                }
+
+                using (RSA key = fromPfx.GetRSAPrivateKey())
+                {
+                    key.SignData(serializedCert, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
+                }
+            }
+        }
+
         public static IEnumerable<object> StorageFlags => CollectionImportTests.StorageFlags;
     }
 }
diff --git a/src/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs b/src/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs
index 0891532760df..ffffab610618 100644
--- a/src/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs
+++ b/src/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs
@@ -1431,6 +1431,35 @@ public static void X509ExtensionCollection_OidIndexer_NoMatchByFriendlyName()
                 Assert.Null(byFriendlyName);
             }
         }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public static void SerializedCertDisposeDoesNotRemoveKeyFile()
+        {
+            using (X509Certificate2 fromPfx = new X509Certificate2(TestData.PfxData, TestData.PfxDataPassword))
+            {
+                Assert.True(fromPfx.HasPrivateKey, "fromPfx.HasPrivateKey - before");
+
+                byte[] serializedCert = fromPfx.Export(X509ContentType.SerializedCert);
+
+                X509Certificate2 fromSerialized;
+
+                using (ImportedCollection imported = Cert.Import(serializedCert))
+                {
+                    fromSerialized = imported.Collection[0];
+                    Assert.True(fromSerialized.HasPrivateKey, "fromSerialized.HasPrivateKey");
+                    Assert.NotEqual(IntPtr.Zero, fromSerialized.Handle);
+                }
+
+                // The certificate got disposed by the collection holder.
+                Assert.Equal(IntPtr.Zero, fromSerialized.Handle);
+
+                using (RSA key = fromPfx.GetRSAPrivateKey())
+                {
+                    key.SignData(serializedCert, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
+                }
+            }
+        }
         
         private static void TestExportSingleCert_SecureStringPassword(X509ContentType ct)
         {