Don't delete private keys detected from SerializedCert imports

dotnet · Mar 16, 2020 · 2bcf713 · 2bcf713
1 parent 4ceccbb
commit 2bcf713
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 2 deletions.
diff --git a/...ptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs b/...ptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs
@@ -35,7 +35,7 @@ private static ICertificatePal FromBlobOrFile(byte[]? rawData, string? fileName,
             bool loadFromFile = (fileName != null);
 
             PfxCertStoreFlags pfxCertStoreFlags = MapKeyStorageFlags(keyStorageFlags);
-            bool persistKeySet = (0 != (keyStorageFlags & X509KeyStorageFlags.PersistKeySet));
+            bool deleteKeyContainer = false;
 
             CertEncodingType msgAndCertEncodingType;
             ContentType contentType;
@@ -87,9 +87,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/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
@@ -372,6 +372,28 @@ public static void X509Certificate2WithT61String()
             }
         }
 
+        [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/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs
@@ -1397,6 +1397,35 @@ public static void X509ExtensionCollection_OidIndexer_NoMatchByFriendlyName()
             }
         }
 
+        [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)
         {
             using (var pfxCer = new X509Certificate2(TestData.PfxData, TestData.CreatePfxDataPasswordSecureString(), Cert.EphemeralIfPossible))