Create copies of mutable properties on X509Certificate2

Export(Cert) returned the original byte array from the PAL. If a
caller mutated the result of the export, they would be mutating the
underlying representation of RawData in the PAL. To be consistent
with Windows and Linux, we return a copy in the PAL.

Fix OpenSSL PAL to prevent side effects between mutable
SubjectName.RawData and Subject properties (similarly for Issuer)

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs
index 8e05f22..be5d106 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/AppleCertificatePal.cs
@@ -185,10 +185,23 @@ namespace Internal.Cryptography.Pal
             }
         }
 
+        public string Issuer
+        {
+            get
+            {
+                EnsureCertData();
+                return _certData.IssuerName;
+            }
+        }
 
-        public string Issuer => IssuerName.Name;
-
-        public string Subject => SubjectName.Name;
+        public string Subject
+        {
+            get
+            {
+                EnsureCertData();
+                return _certData.SubjectName;
+            }
+        }
 
         public string LegacyIssuer => IssuerName.Decode(X500DistinguishedNameFlags.None);
 
@@ -322,7 +335,7 @@ namespace Internal.Cryptography.Pal
             get
             {
                 EnsureCertData();
-                return _certData.RawData;
+                return _certData.RawData.CloneByteArray();
             }
         }
 

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/CertificateData.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/CertificateData.cs
index 2d3e96d..af1935e 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/CertificateData.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.OSX/CertificateData.cs
@@ -49,6 +49,8 @@ namespace Internal.Cryptography.Pal
         internal X500DistinguishedName Issuer;
         internal X500DistinguishedName Subject;
         internal List<X509Extension> Extensions;
+        internal string IssuerName;
+        internal string SubjectName;
 
         internal int Version => certificate.TbsCertificate.Version;
 
@@ -81,6 +83,8 @@ namespace Internal.Cryptography.Pal
             certificate.TbsCertificate.ValidateVersion();
             Issuer = new X500DistinguishedName(certificate.TbsCertificate.Issuer.ToArray());
             Subject = new X500DistinguishedName(certificate.TbsCertificate.Subject.ToArray());
+            IssuerName = Issuer.Name;
+            SubjectName = Subject.Name;
 
             AsnWriter writer = new AsnWriter(AsnEncodingRules.DER);
             certificate.TbsCertificate.SubjectPublicKeyInfo.Encode(writer);

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509CertificateReader.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509CertificateReader.cs
index 8d56f3a..880dd7e 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509CertificateReader.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509CertificateReader.cs
@@ -23,6 +23,8 @@ namespace Internal.Cryptography.Pal
         private SafeEvpPKeyHandle? _privateKey;
         private X500DistinguishedName? _subjectName;
         private X500DistinguishedName? _issuerName;
+        private string? _subject;
+        private string? _issuer;
 
         public static ICertificatePal FromHandle(IntPtr handle)
         {
@@ -258,9 +260,37 @@ namespace Internal.Cryptography.Pal
             get { return _cert; }
         }
 
-        public string Issuer => IssuerName.Name;
+        public string Issuer
+        {
+            get
+            {
+                if (_issuer == null)
+                {
+                    // IssuerName is mutable to callers in X509Certificate. We want to be
+                    // able to get the issuer even if IssuerName has been mutated, so we
+                    // don't use it here.
+                    _issuer = Interop.Crypto.LoadX500Name(Interop.Crypto.X509GetIssuerName(_cert)).Name;
+                }
+
+                return _issuer;
+            }
+        }
 
-        public string Subject => SubjectName.Name;
+        public string Subject
+        {
+            get
+            {
+                if (_subject == null)
+                {
+                    // SubjectName is mutable to callers in X509Certificate. We want to be
+                    // able to get the subject even if SubjectName has been mutated, so we
+                    // don't use it here.
+                    _subject = Interop.Crypto.LoadX500Name(Interop.Crypto.X509GetSubjectName(_cert)).Name;
+                }
+
+                return _subject;
+            }
+        }
 
         public string LegacyIssuer => IssuerName.Decode(X500DistinguishedNameFlags.None);
 

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
index b3f67b3..e16a28f 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
@@ -433,6 +433,39 @@ namespace System.Security.Cryptography.X509Certificates.Tests
             }
         }
 
+        [Fact]
+        public static void CopyResult_RawData()
+        {
+            using (X509Certificate2 cert = new X509Certificate2(TestData.MsCertificate))
+            {
+                byte[] first = cert.RawData;
+                byte[] second = cert.RawData;
+                Assert.NotSame(first, second);
+            }
+        }
+
+        [Fact]
+        public static void MutateDistinguishedName_IssuerName_DoesNotImpactIssuer()
+        {
+            using (X509Certificate2 cert = new X509Certificate2(TestData.MsCertificate))
+            {
+                byte[] issuerBytes = cert.IssuerName.RawData;
+                Array.Clear(issuerBytes, 0, issuerBytes.Length);
+                Assert.Equal("CN=Microsoft Code Signing PCA, O=Microsoft Corporation, L=Redmond, S=Washington, C=US", cert.Issuer);
+            }
+        }
+
+        [Fact]
+        public static void MutateDistinguishedName_SubjectName_DoesNotImpactSubject()
+        {
+            using (X509Certificate2 cert = new X509Certificate2(TestData.MsCertificate))
+            {
+                byte[] subjectBytes = cert.SubjectName.RawData;
+                Array.Clear(subjectBytes, 0, subjectBytes.Length);
+                Assert.Equal("CN=Microsoft Corporation, OU=MOPR, O=Microsoft Corporation, L=Redmond, S=Washington, C=US", cert.Subject);
+            }
+        }
+
         public static IEnumerable<object[]> StorageFlags => CollectionImportTests.StorageFlags;
     }
 }

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ExportTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ExportTests.cs
index e84bf7e..b508552 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/ExportTests.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/ExportTests.cs
@@ -8,6 +8,17 @@ namespace System.Security.Cryptography.X509Certificates.Tests
     public static class ExportTests
     {
         [Fact]
+        public static void ExportAsCert_CreatesCopy()
+        {
+            using (X509Certificate2 cert = new X509Certificate2(TestData.MsCertificate))
+            {
+                byte[] first = cert.Export(X509ContentType.Cert);
+                byte[] second = cert.Export(X509ContentType.Cert);
+                Assert.NotSame(first, second);
+            }
+        }
+
+        [Fact]
         public static void ExportAsCert()
         {
             using (X509Certificate2 c1 = new X509Certificate2(TestData.MsCertificate))