From: Kevin Jones Date: Fri, 10 Jul 2020 15:26:54 +0000 (-0400) Subject: Create copies of mutable properties on X509Certificate2 X-Git-Tag: submit/tizen/20210909.063632~6804 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=eda20d006dddb6e6e38c1b1139893472321b9d25;p=platform%2Fupstream%2Fdotnet%2Fruntime.git 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 --- 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 --- 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 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 --- 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 --- 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 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 --- 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))