Add support for reading files from disk to X509Certificate2 and X509Certificate2Collection on Unix

With the file support it was easy to add a test that verifies that the Windows and Unix implementations of X509Certificate2Collection.Import on a PFX result in enumerating the files in the same order.

The change uses OpenSSL's BIO structure to read the files, avoiding a File.ReadAllBytes approach to avoid cases wherein someone reads a very large file as a certificate.

Author: bartonjs

src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.NativeCrypto.cs

@@ -12,12 +12,21 @@ internal static partial class NativeCrypto
     {
         private delegate int NegativeSizeReadMethod<in THandle>(THandle handle, byte[] buf, int cBuf);
 
+        [DllImport(Libraries.CryptoInterop)]
+        internal static extern int BioTell(SafeBioHandle bio);
+
+        [DllImport(Libraries.CryptoInterop)]
+        internal static extern int BioSeek(SafeBioHandle bio, int pos);
+
         [DllImport(Libraries.CryptoInterop)]
         private static extern int GetX509Thumbprint(SafeX509Handle x509, byte[] buf, int cBuf);
 
         [DllImport(Libraries.CryptoInterop)]
         private static extern int GetX509NameRawBytes(IntPtr x509Name, byte[] buf, int cBuf);
 
+        [DllImport(Libraries.CryptoInterop)]
+        internal static extern SafeX509Handle ReadX509AsDerFromBio(SafeBioHandle bio);
+
         [DllImport(Libraries.CryptoInterop)]
         internal static extern IntPtr GetX509NotBefore(SafeX509Handle x509);
 

src/Common/src/Interop/Unix/libcrypto/Interop.BIO.cs

@@ -18,6 +18,9 @@ internal static partial class libcrypto
         [DllImport(Libraries.LibCrypto)]
         internal static extern SafeBioHandle BIO_new(IntPtr type);
 
+        [DllImport(Libraries.LibCrypto)]
+        internal static extern SafeBioHandle BIO_new_file(string filename, string mode);
+
         [DllImport(Libraries.LibCrypto)]
         internal static extern IntPtr BIO_s_mem();
 

src/Native/System.Security.Cryptography.Native/openssl.c

@@ -775,3 +775,76 @@ GetX509RootStorePath()
 
     return dir;
 }
+
+/*
+Function:
+ReadX509AsDerFromBio
+
+Used by System.Security.Cryptography.X509Certificates' OpenSslX509CertificateReader when attempting
+to turn the contents of a file into an ICertificatePal object.
+
+Return values:
+If bio containns a valid DER-encoded X509 object, a pointer to that X509 structure that was deserialized,
+otherwise NULL.
+*/
+X509*
+ReadX509AsDerFromBio(
+    BIO* bio)
+{
+    return d2i_X509_bio(bio, NULL);
+}
+
+/*
+Function:
+BioTell
+
+Used by System.Security.Cryptography.X509Certificates' OpenSslX509CertificateReader when attempting
+to turn the contents of a file into an ICertificatePal object to allow seeking back to the start point
+in the event of a deserialization failure.
+
+Return values:
+The current seek position of the BIO if it is a file-related BIO, -1 on NULL inputs, and has unspecified
+behavior on non-file, non-null BIO objects.
+
+See also:
+OpenSSL's BIO_tell
+*/
+int BioTell(
+    BIO* bio)
+{
+    if (!bio)
+    {
+        return -1;
+    }
+
+    return BIO_tell(bio);
+}
+
+/*
+Function:
+BioTell
+
+Used by System.Security.Cryptography.X509Certificates' OpenSslX509CertificateReader when attempting
+to turn the contents of a file into an ICertificatePal object to seek back to the start point
+in the event of a deserialization failure.
+
+Return values:
+-1 if bio is NULL
+-1 if bio is a file-related BIO and seek fails
+0 if bio is a file-related BIO and seek succeeds
+otherwise unspecified
+
+See also:
+OpenSSL's BIO_seek
+*/
+int BioSeek(
+    BIO* bio,
+    int ofs)
+{
+    if (!bio)
+    {
+        return -1;
+    }
+
+    return BIO_seek(bio, ofs);
+}

src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CertificatePal.cs

@@ -80,7 +80,13 @@ public static unsafe ICertificatePal FromBlob(byte[] rawData, string password, X
 
         public static ICertificatePal FromFile(string fileName, string password, X509KeyStorageFlags keyStorageFlags)
         {
-            throw new NotImplementedException();
+            // If we can't open the file, fail right away.
+            using (SafeBioHandle fileBio = Interop.libcrypto.BIO_new_file(fileName, "rb"))
+            {
+                Interop.libcrypto.CheckValidOpenSslHandle(fileBio);
+
+                return OpenSslX509CertificateReader.FromBio(fileBio, password);
+            }
         }
     }
 }

src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509CertificateReader.cs

@@ -35,6 +35,72 @@ internal OpenSslX509CertificateReader(SafeX509Handle handle)
 
             _cert = handle;
         }
+      
+        internal static ICertificatePal FromBio(SafeBioHandle bio, string password)
+        {
+            // Try reading the value as: PEM-X509, DER-X509, DER-PKCS12.
+            int bioPosition = Interop.NativeCrypto.BioTell(bio);
+
+            Debug.Assert(bioPosition >= 0);
+
+            SafeX509Handle cert = Interop.libcrypto.PEM_read_bio_X509_AUX(bio, IntPtr.Zero, IntPtr.Zero, IntPtr.Zero);
+
+            if (cert != null && !cert.IsInvalid)
+            {
+                return new OpenSslX509CertificateReader(cert);
+            }
+
+            // Rewind, try again.
+            Interop.NativeCrypto.BioSeek(bio, bioPosition);
+            cert = Interop.NativeCrypto.ReadX509AsDerFromBio(bio);
+
+            if (cert != null && !cert.IsInvalid)
+            {
+                return new OpenSslX509CertificateReader(cert);
+            }
+
+            // Rewind, try again.
+            Interop.NativeCrypto.BioSeek(bio, bioPosition);
+
+            OpenSslPkcs12Reader pfx;
+
+            if (OpenSslPkcs12Reader.TryRead(bio, out pfx))
+            {
+                using (pfx)
+                {
+                    pfx.Decrypt(password);
+
+                    ICertificatePal first = null;
+
+                    foreach (OpenSslX509CertificateReader certPal in pfx.ReadCertificates())
+                    {
+                        // When requesting an X509Certificate2 from a PFX only the first entry is
+                        // returned.  Other entries should be disposed.
+                        if (first == null)
+                        {
+                            first = certPal;
+                        }
+                        else
+                        {
+                            certPal.Dispose();
+                        }
+                    }
+
+                    return first;
+                }
+            }
+
+            // Since we aren't going to finish reading, leaving the buffer where it was when we got
+            // it seems better than leaving it in some arbitrary other position.
+            // 
+            // But, before seeking back to start, save the Exception representing the last reported
+            // OpenSSL error in case the last BioSeek would change it.
+            Exception openSslException = Interop.libcrypto.CreateOpenSslCryptographicException();
+
+            Interop.NativeCrypto.BioSeek(bio, bioPosition);
+
+            throw openSslException;
+        }
 
         public bool HasPrivateKey
         {

src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/StorePal.cs

@@ -34,7 +34,22 @@ public static IStorePal FromBlob(byte[] rawData, string password, X509KeyStorage
 
         public static IStorePal FromFile(string fileName, string password, X509KeyStorageFlags keyStorageFlags)
         {
-            throw new NotImplementedException();
+            using (SafeBioHandle fileBio = Interop.libcrypto.BIO_new_file(fileName, "rb"))
+            {
+                Interop.libcrypto.CheckValidOpenSslHandle(fileBio);
+
+                OpenSslPkcs12Reader pfx;
+
+                if (OpenSslPkcs12Reader.TryRead(fileBio, out pfx))
+                {
+                    using (pfx)
+                    {
+                        return PfxToCollection(pfx, password);
+                    }
+                }
+            }
+
+            return null;
         }
 
         public static IStorePal FromCertificate(ICertificatePal cert)

src/System.Security.Cryptography.X509Certificates/tests/CertTests.cs

@@ -9,16 +9,16 @@ namespace System.Security.Cryptography.X509Certificates.Tests
     public static class CertTests
     {
         [Fact]
-        [ActiveIssue(1993, PlatformID.AnyUnix)]
+        [ActiveIssue(1985, PlatformID.AnyUnix)]
         public static void X509CertTest()
         {
-            const string CertSubject =
-                @"CN=Microsoft Corporate Root Authority, OU=ITG, O=Microsoft, L=Redmond, S=WA, C=US, [email protected]";
+            string certSubject = TestData.NormalizeX500String(
+                @"CN=Microsoft Corporate Root Authority, OU=ITG, O=Microsoft, L=Redmond, S=WA, C=US, [email protected]");
 
             using (X509Certificate cert = new X509Certificate(Path.Combine("TestData", "microsoft.cer")))
             {
-                Assert.Equal(CertSubject, cert.Subject);
-                Assert.Equal(CertSubject, cert.Issuer);
+                Assert.Equal(certSubject, cert.Subject);
+                Assert.Equal(certSubject, cert.Issuer);
 
                 int snlen = cert.GetSerialNumber().Length;
                 Assert.Equal(16, snlen);
@@ -49,19 +49,19 @@ public static void X509CertTest()
         }
 
         [Fact]
-        [ActiveIssue(1993, PlatformID.AnyUnix)]
+        [ActiveIssue(1985, PlatformID.AnyUnix)]
         public static void X509Cert2Test()
         {
-            const string CertName =
-                @"[email protected], CN=ABA.ECOM Root CA, O=""ABA.ECOM, INC."", L=Washington, S=DC, C=US";
+            string certName = TestData.NormalizeX500String(
+                @"[email protected], CN=ABA.ECOM Root CA, O=""ABA.ECOM, INC."", L=Washington, S=DC, C=US");
 
             DateTime notBefore = new DateTime(1999, 7, 12, 17, 33, 53, DateTimeKind.Utc).ToLocalTime();
             DateTime notAfter = new DateTime(2009, 7, 9, 17, 33, 53, DateTimeKind.Utc).ToLocalTime();
 
             using (X509Certificate2 cert2 = new X509Certificate2(Path.Combine("TestData", "test.cer")))
             {
-                Assert.Equal(CertName, cert2.IssuerName.Name);
-                Assert.Equal(CertName, cert2.SubjectName.Name);
+                Assert.Equal(certName, cert2.IssuerName.Name);
+                Assert.Equal(certName, cert2.SubjectName.Name);
 
                 Assert.Equal("ABA.ECOM Root CA", cert2.GetNameInfo(X509NameType.DnsName, true));
 
@@ -147,7 +147,6 @@ public static void X509Cert2ToStringVerbose()
         }
 
         [Fact]
-        [ActiveIssue(1993, PlatformID.AnyUnix)]
         public static void X509Cert2CreateFromPfxFile()
         {
             using (X509Certificate2 cert2 = new X509Certificate2(Path.Combine("TestData", "DummyTcpServer.pfx")))
@@ -158,7 +157,6 @@ public static void X509Cert2CreateFromPfxFile()
         }
 
         [Fact]
-        [ActiveIssue(1993, PlatformID.AnyUnix)]
         public static void X509Cert2CreateFromPfxWithPassword()
         {
             using (X509Certificate2 cert2 = new X509Certificate2(Path.Combine("TestData", "test.pfx"), "test"))

src/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs

@@ -174,6 +174,43 @@ public static void ImportPfx()
             }
         }
 
+        [Fact]
+        public static void ImportFullChainPfx()
+        {
+            X509Certificate2Collection certs = new X509Certificate2Collection();
+            certs.Import(Path.Combine("TestData", "test.pfx"), "test", X509KeyStorageFlags.DefaultKeySet);
+            int count = certs.Count;
+            Assert.Equal(3, count);
+
+            // Verify that the read ordering is consistent across the platforms
+            string[] expectedSubjects =
+            {
+                "MS Passport Test Sub CA",
+                "MS Passport Test Root CA",
+                "test.local",
+            };
+
+            string[] actualSubjects = certs.OfType<X509Certificate2>().
+                Select(cert => cert.GetNameInfo(X509NameType.SimpleName, false)).
+                ToArray();
+
+            Assert.Equal(expectedSubjects, actualSubjects);
+            
+            // And verify that we have private keys when we expect them
+            bool[] expectedHasPrivateKeys =
+            {
+                false,
+                false,
+                true,
+            };
+
+            bool[] actualHasPrivateKeys = certs.OfType<X509Certificate2>().
+                Select(cert => cert.HasPrivateKey).
+                ToArray();
+
+            Assert.Equal(expectedHasPrivateKeys, actualHasPrivateKeys);
+        }
+
         [Fact]
         [ActiveIssue(1993, PlatformID.AnyUnix)]
         public static void ImportStoreSavedAsCerData()
@@ -267,7 +304,6 @@ public static void ImportStoreSavedAsPfxData()
         }
 
         [Fact]
-        [ActiveIssue(1993, PlatformID.AnyUnix)]
         public static void ImportFromFileTests()
         {
             using (var pfxCer = new X509Certificate2(TestData.PfxData, TestData.PfxDataPassword))