diff --git a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignedCms.cs b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignedCms.cs index 8a88fb2b482bee..0b0700d2658410 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignedCms.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignedCms.cs @@ -686,7 +686,7 @@ public void AddCertificate(X509Certificate2 certificate) { foreach (CertificateChoiceAsn cert in _signedData.CertificateSet!) { - if (cert.Certificate!.Value.Span.SequenceEqual(rawData)) + if (cert.Certificate is not null && cert.Certificate.Value.Span.SequenceEqual(rawData)) { throw new CryptographicException(SR.Cryptography_Cms_CertificateAlreadyInCollection); } @@ -721,7 +721,7 @@ public void RemoveCertificate(X509Certificate2 certificate) foreach (CertificateChoiceAsn cert in _signedData.CertificateSet!) { - if (cert.Certificate!.Value.Span.SequenceEqual(rawData)) + if (cert.Certificate is not null && cert.Certificate.Value.Span.SequenceEqual(rawData)) { PkcsHelpers.RemoveAt(ref _signedData.CertificateSet, idx); Reencode(); diff --git a/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs b/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs index 255bc691b72d1e..054a07cd6e001f 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Formats.Asn1; using System.Linq; using System.Security.Cryptography.X509Certificates; using Test.Cryptography; @@ -604,6 +605,55 @@ public static void CreateSignature_Ecdsa_ThrowsWithRsaSignaturePadding() } } + [Fact] + public static void AddCertificate_CollectionContainsAttributeCertificate() + { + SignedCms signedCms = new SignedCms(); + signedCms.Decode(SignedDocuments.TstWithAttributeCertificate); + signedCms.CheckSignature(true); + + int countBefore = CountCertificateChoices(SignedDocuments.TstWithAttributeCertificate); + + using (X509Certificate2 cert = Certificates.RSA2048SignatureOnly.GetCertificate()) + { + signedCms.AddCertificate(cert); + byte[] reEncoded = signedCms.Encode(); + int countAfter = CountCertificateChoices(reEncoded); + Assert.Equal(countBefore + 1, countAfter); + + signedCms = new SignedCms(); + signedCms.Decode(reEncoded); + signedCms.CheckSignature(true); + } + } + + [Fact] + public static void RemoveCertificate_Existing_CollectionContainsAttributeCertificate() + { + SignedCms signedCms = new SignedCms(); + signedCms.Decode(SignedDocuments.TstWithAttributeCertificate); + int countBefore = CountCertificateChoices(SignedDocuments.TstWithAttributeCertificate); + + signedCms.RemoveCertificate(signedCms.Certificates[0]); + byte[] reEncoded = signedCms.Encode(); + int countAfter = CountCertificateChoices(reEncoded); + Assert.Equal(countBefore - 1, countAfter); + } + + [Fact] + public static void RemoveCertificate_NonExisting_CollectionContainsAttributeCertificate() + { + SignedCms signedCms = new SignedCms(); + signedCms.Decode(SignedDocuments.TstWithAttributeCertificate); + + using (X509Certificate2 cert = Certificates.RSA2048SignatureOnly.GetCertificate()) + { + // Remove a non-existing certificate so that we are forced to enumerate the entire 'certificates[0]' + // collection (including attribute certificates) looking for it. + Assert.Throws(() => signedCms.RemoveCertificate(cert)); + } + } + private static void VerifyWithExplicitPrivateKey(X509Certificate2 cert, AsymmetricAlgorithm key) { using (var pubCert = new X509Certificate2(cert.RawData)) @@ -664,5 +714,36 @@ private static void VerifyCounterSignatureWithExplicitPrivateKey(X509Certificate Assert.Equal(counterSignerPubCert, cms.SignerInfos[0].CounterSignerInfos[0].Certificate); } } + + private static int CountCertificateChoices(byte[] encoded) + { + AsnReader reader = new AsnReader(encoded, AsnEncodingRules.BER); + reader = reader.ReadSequence(); + reader.ReadObjectIdentifier(); + reader = reader.ReadSequence(new Asn1Tag(TagClass.ContextSpecific, 0)); + reader = reader.ReadSequence(); + + reader.ReadInteger(); // version + reader.ReadSetOf(); // digestAlgorithms + reader.ReadSequence(); // encapsulatedContentInfo + + Asn1Tag expectedTag = new Asn1Tag(TagClass.ContextSpecific, 0, true); // certificates[0] + + if (reader.PeekTag() == expectedTag) + { + AsnReader certs = reader.ReadSetOf(expectedTag); + int count = 0; + + while (certs.HasData) + { + certs.ReadEncodedValue(); + count++; + } + + return count; + } + + return 0; + } } }