Adding CRL checks during BuildCertificateChain

Include intermediate certificate resolution.
Adding tests to ensure CRLs are resolved outside of a revocation test.  A new nochain certificate was created where the cert does not contain the whole certificate chain.  This way the resolution can be exercised.

Author: JoeShook

MimeKit/Cryptography/BouncyCastleSecureMimeContext.cs

@@ -694,20 +694,41 @@ X509Certificate GetCertificate (IStore<X509Certificate> store, SignerID signer)
 		/// <returns>The certificate chain, including the specified certificate.</returns>
 		protected IList<X509Certificate> BuildCertificateChain (X509Certificate certificate)
 		{
-			var selector = new X509CertStoreSelector {
-				Certificate = certificate
-			};
+			var selector = new X509CertStoreSelector ();
 
-			var intermediates = new X509CertificateStore ();
-			intermediates.Add (certificate);
+			var userCertificateStore = new X509CertificateStore ();
+			userCertificateStore.Add (certificate);
 
-			var parameters = new PkixBuilderParameters (GetTrustedAnchors (), selector) {
+			var issurerStore = GetTrustedAnchors ();
+			var anchorStore = new X509CertificateStore ();
+			
+			foreach (var anchor in issurerStore) {
+				anchorStore.Add (anchor.TrustedCert);
+			}
+
+			var parameters = new PkixBuilderParameters (issurerStore, selector) {
 				ValidityModel = PkixParameters.PkixValidityModel,
-				IsRevocationEnabled = false,
+				IsRevocationEnabled = CheckCertificateRevocation,
 				Date = DateTime.UtcNow
 			};
-			parameters.AddStoreCert (intermediates);
-			parameters.AddStoreCert (GetIntermediateCertificates ());
+			parameters.AddStoreCert (userCertificateStore);
+
+			if (CheckCertificateRevocation) {
+				DownloadCrls (certificate);
+			}
+			
+			var intermediateStore = GetIntermediateCertificates ();
+
+			foreach (var intermediate in intermediateStore.EnumerateMatches (new X509CertStoreSelector ())) {
+				anchorStore.Add (intermediate);
+				if (CheckCertificateRevocation)
+					DownloadCrls (intermediate);
+			}
+
+			parameters.AddStoreCert (anchorStore);
+
+			if (CheckCertificateRevocation)
+				parameters.AddStoreCrl (GetCertificateRevocationLists ());
 
 			var builder = new PkixCertPathBuilder ();
 			var result = builder.Build (parameters);
@@ -995,7 +1016,7 @@ static IEnumerable<string> EnumerateCrlDistributionPointUrls (X509Certificate ce
 			}
 		}
 
-		void DownloadCrls (X509Certificate certificate, CancellationToken cancellationToken)
+		void DownloadCrls (X509Certificate certificate, CancellationToken cancellationToken = default)
 		{
 			var nextUpdate = GetNextCertificateRevocationListUpdate (certificate.IssuerDN);
 			var now = DateTime.UtcNow;
@@ -1125,10 +1146,15 @@ DigitalSignatureCollection GetDigitalSignatures (CmsSignedDataParser parser, Can
 				}
 
 				var anchors = GetTrustedAnchors ();
+				var intermediates = GetIntermediateCertificates ();
 
 				if (CheckCertificateRevocation) {
 					foreach (var anchor in anchors)
 						DownloadCrls (anchor.TrustedCert, cancellationToken);
+
+					foreach (X509Certificate intermediate in intermediates.EnumerateMatches(new X509CertStoreSelector())) {
+						DownloadCrls (intermediate, cancellationToken);
+					}
 				}
 
 				try {

MimeKit/Cryptography/DefaultSecureMimeContext.cs

@@ -432,20 +432,19 @@ protected override ISet<TrustAnchor> GetTrustedAnchors ()
 		/// <returns>The intermediate certificates.</returns>
 		protected override IStore<X509Certificate> GetIntermediateCertificates ()
 		{
-			//var intermediates = new X509CertificateStore ();
-			//var selector = new X509CertStoreSelector ();
-			//var keyUsage = new bool[9];
+			var intermediates = new X509CertificateStore ();
+			var selector = new X509CertStoreSelector ();
+			var keyUsage = new bool[9];
 
-			//keyUsage[(int) X509KeyUsageBits.KeyCertSign] = true;
-			//selector.KeyUsage = keyUsage;
+			keyUsage[(int) X509KeyUsageBits.KeyCertSign] = true;
+			selector.KeyUsage = keyUsage;
 
-			//foreach (var record in dbase.Find (selector, false, X509CertificateRecordFields.Certificate)) {
-			//	if (!record.Certificate.IsSelfSigned ())
-			//		intermediates.Add (record.Certificate);
-			//}
+			foreach (var record in dbase.Find (selector, false, X509CertificateRecordFields.Certificate)) {
+				if (!record.Certificate.IsSelfSigned ())
+					intermediates.Add (record.Certificate);
+			}
 
-			//return intermediates;
-			return dbase;
+			return intermediates;
 		}
 
 		/// <summary>

UnitTests/Cryptography/SecureMimeTests.cs

@@ -25,6 +25,7 @@
 //
 
 using System.Net;
+using System.Reflection;
 using System.Text;
 using System.Security.Cryptography.X509Certificates;
 
@@ -79,7 +80,7 @@ public abstract class SecureMimeTestsBase
 		public const string ThunderbirdName = "[email protected]";
 
 		public static readonly string[] RelativeConfigFilePaths = {
-			"certificate-authority.cfg", "intermediate1.cfg", "intermediate2.cfg", "dnsnames/smime.cfg", "dsa/smime.cfg", "ec/smime.cfg", "revoked/smime.cfg", "rsa/smime.cfg"
+			"certificate-authority.cfg", "intermediate1.cfg", "intermediate2.cfg", "dnsnames/smime.cfg", "dsa/smime.cfg", "ec/smime.cfg", "nochain/smime.cfg", "revoked/smime.cfg", "rsa/smime.cfg"
 		};
 
 		public static readonly string[] StartComCertificates = {
@@ -260,7 +261,7 @@ protected static Mock<HttpMessageHandler> CreateMockHttpMessageHandler ()
 			};
 
 			var mockHttpMessageHandler = new Mock<HttpMessageHandler> (MockBehavior.Strict);
-
+			
 			for (int i = 0; i < CrlRequestUris.Length; i++) {
 				var requestUri = CrlRequestUris[i];
 				var response = responses[i];
@@ -343,11 +344,11 @@ protected void ImportTestCertificates (SecureMimeContext ctx)
 				Assert.DoesNotThrow (() => ctx.Import (mimekitCertificate.FileName, "no.secret"));
 			}
 
-			if (windows is null) {
-				// Import the obsolete CRLs (we want the S/MIME context to download the current CRLs)
-				foreach (var crl in ObsoleteCrls)
-					ctx.Import (crl);
-			}
+			// if (windows is null) {
+			// 	// Import the obsolete CRLs (we want the S/MIME context to download the current CRLs)
+			// 	foreach (var crl in ObsoleteCrls)
+			// 		ctx.Import (crl);
+			// }
 		}
 
 		protected SecureMimeTestsBase ()
@@ -3000,6 +3001,116 @@ protected async Task VerifyRevokedCertificateAsync (BouncyCastleSecureMimeContex
 				Assert.That (innerException.Message, Does.EndWith (", reason: keyCompromise"));
 			}
 		}
+
+		protected void VerifyCrlsResolvedWithBuildCertificateChain (BouncyCastleSecureMimeContext ctx,
+			Mock<HttpMessageHandler> mockHttpMessageHandler)
+		{
+			var body = new TextPart ("plain") { Text = "This is some cleartext that we'll end up signing..." };
+			var certificate = SupportedCertificates.Single(c => c.EmailAddress == "[email protected]");
+
+			var signer = new CmsSigner (certificate.FileName, "no.secret");
+			var multipart = MultipartSigned.Create (ctx, signer, body);
+
+			Assert.That (multipart.Count, Is.EqualTo (2), "The multipart/signed has an unexpected number of children.");
+
+			var protocol = multipart.ContentType.Parameters["protocol"];
+			Assert.That (protocol, Is.EqualTo (ctx.SignatureProtocol), "The multipart/signed protocol does not match.");
+
+			Assert.That (multipart[0], Is.InstanceOf<TextPart> (), "The first child is not a text part.");
+			Assert.That (multipart[1], Is.InstanceOf<ApplicationPkcs7Signature> (), "The second child is not a detached signature.");
+
+			var signatures = multipart.Verify (ctx);
+			Assert.That (signatures.Count, Is.EqualTo (1), "Verify returned an unexpected number of signatures.");
+
+			AssertCrlsRequested (mockHttpMessageHandler);
+
+			AssertValidSignatures (ctx, signatures);
+		}
+
+		protected void VerifyCrlsResolved (BouncyCastleSecureMimeContext ctx,
+			Mock<HttpMessageHandler> mockHttpMessageHandler)
+		{
+			var body = new TextPart ("plain") { Text = "This is some cleartext that we'll end up signing..." };
+			var certificate = SupportedCertificates.Single (c => c.EmailAddress == "[email protected]");
+
+			var signer = new CmsSigner (certificate.FileName, "no.secret");
+			var multipart = MultipartSigned.Create (ctx, signer, body);
+
+			Assert.That (multipart.Count, Is.EqualTo (2), "The multipart/signed has an unexpected number of children.");
+
+			var protocol = multipart.ContentType.Parameters["protocol"];
+			Assert.That (protocol, Is.EqualTo (ctx.SignatureProtocol), "The multipart/signed protocol does not match.");
+
+			Assert.That (multipart[0], Is.InstanceOf<TextPart> (), "The first child is not a text part.");
+			Assert.That (multipart[1], Is.InstanceOf<ApplicationPkcs7Signature> (), "The second child is not a detached signature.");
+
+			var signatures = multipart.Verify (ctx);
+			Assert.That (signatures.Count, Is.EqualTo (1), "Verify returned an unexpected number of signatures.");
+
+			AssertCrlsRequested (mockHttpMessageHandler);
+
+			AssertValidSignatures (ctx, signatures);
+		}
+
+		protected void VerifyCrlMissing (BouncyCastleSecureMimeContext ctx, string errorContent)
+		{
+			var body = new TextPart ("plain") { Text = "This is some cleartext that we'll end up signing..." };
+			var certificate = SupportedCertificates.Single (c => c.EmailAddress == "[email protected]");
+			
+			var signer = new CmsSigner (certificate.FileName, "no.secret");
+			using var multipart = MultipartSigned.Create (ctx, signer, body);
+			
+			Assert.That (multipart.Count, Is.EqualTo (2), "The multipart/signed has an unexpected number of children.");
+
+			var protocol = multipart.ContentType.Parameters["protocol"];
+			Assert.That (protocol, Is.EqualTo (ctx.SignatureProtocol), "The multipart/signed protocol does not match.");
+
+			Assert.That (multipart[0], Is.InstanceOf<TextPart> (), "The first child is not a text part.");
+			Assert.That (multipart[1], Is.InstanceOf<ApplicationPkcs7Signature> (), "The second child is not a detached signature.");
+
+			var signatures = multipart.Verify (ctx);
+			Assert.That (signatures.Count, Is.EqualTo (1), "Verify returned an unexpected number of signatures.");
+			
+			var ex = Assert.Throws<DigitalSignatureVerifyException> (() => signatures.Single ().Verify ());
+			Assert.That (ex.Message, Does.StartWith ("Failed to verify digital signature chain: Certification path could not be validated."));
+			Assert.That (ex.InnerException?.Message, Does.StartWith ("Certification path could not be validated."));
+			Assert.That(ex.InnerException?.InnerException?.Message, Is.EquivalentTo ($"No CRLs found for issuer \"{errorContent}\""));
+		}
+
+
+		protected void VerifyMimeEncapsulatedSigningWithContext (BouncyCastleSecureMimeContext ctx,
+			Mock<HttpMessageHandler> mockHttpMessageHandler)
+		{
+			var cleartext = new TextPart ("plain") { Text = "This is some text that we'll end up signing..." };
+
+
+			var certificate = SupportedCertificates.Single (c => c.EmailAddress == "[email protected]");
+
+			var self = new MailboxAddress ("MimeKit UnitTests", certificate.EmailAddress);
+			var signed = ApplicationPkcs7Mime.Sign (ctx, self, DigestAlgorithm.Sha1, cleartext);
+			MimeEntity extracted;
+
+			Assert.That (signed.SecureMimeType, Is.EqualTo (SecureMimeType.SignedData), "S/MIME type did not match.");
+
+			var signatures = signed.Verify (ctx, out extracted);
+
+			Assert.That (extracted, Is.InstanceOf<TextPart> (), "Extracted part is not the expected type.");
+			Assert.That (((TextPart) extracted).Text, Is.EqualTo (cleartext.Text), "Extracted content is not the same as the original.");
+
+			Assert.That (signatures.Count, Is.EqualTo (1), "Verify returned an unexpected number of signatures.");
+			AssertValidSignatures (ctx, signatures);
+
+			using (var signedData = signed.Content.Open ()) {
+				using (var stream = ctx.Verify (signedData, out signatures))
+					extracted = MimeEntity.Load (stream);
+
+				Assert.That (extracted, Is.InstanceOf<TextPart> (), "Extracted part is not the expected type.");
+				Assert.That (((TextPart) extracted).Text, Is.EqualTo (cleartext.Text), "Extracted content is not the same as the original.");
+
+				Assert.That (signatures.Count, Is.EqualTo (1), "Verify returned an unexpected number of signatures.");
+				AssertValidSignatures (ctx, signatures);
+			}
+		}
 	}
 
 	[TestFixture]
@@ -3010,11 +3121,11 @@ class MyTemporarySecureMimeContext : TemporarySecureMimeContext
 			public readonly Mock<HttpMessageHandler> MockHttpMessageHandler;
 			readonly HttpClient client;
 
-			public MyTemporarySecureMimeContext () : base (new SecureRandom (new CryptoApiRandomGenerator ()))
+			public MyTemporarySecureMimeContext (Mock<HttpMessageHandler>? mockHttpMessageHandler = null) : base (new SecureRandom (new CryptoApiRandomGenerator ()))
 			{
 				CheckCertificateRevocation = false;
 
-				MockHttpMessageHandler = CreateMockHttpMessageHandler ();
+				MockHttpMessageHandler = mockHttpMessageHandler ?? CreateMockHttpMessageHandler ();
 				client = new HttpClient (MockHttpMessageHandler.Object);
 			}
 
@@ -3055,6 +3166,92 @@ public async Task TestVerifyRevokedCertificateAsync ()
 				await VerifyRevokedCertificateAsync (ctx, ctx.MockHttpMessageHandler);
 			}
 		}
+
+		[Test]
+		public void TestVerifyCrlsResolved ()
+		{
+			using (var ctx = new MyTemporarySecureMimeContext () { CheckCertificateRevocation = true }) {
+				ImportTestCertificates (ctx);
+
+				VerifyCrlsResolved (ctx, ctx.MockHttpMessageHandler);
+			}
+		}
+
+		[Test]
+		public void TestMissingRootCrl ()
+		{
+			var responses = new HttpResponseMessage[]
+			{
+			new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(CurrentCrls[1].GetEncoded()) },
+			new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(CurrentCrls[2].GetEncoded()) }
+			};
+			var crlUrlIndexes = new[] { 1, 2 };
+			var errorContent = RootCertificate.SubjectDN.ToString ();
+
+			VerifyCrlMissingTest (responses, crlUrlIndexes, errorContent);
+		}
+
+		[Test]
+		public void TestMissingPrimaryIntermediateCrl ()
+		{
+			var responses = new HttpResponseMessage[]
+			{
+			new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(CurrentCrls[0].GetEncoded()) },
+			new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(CurrentCrls[2].GetEncoded()) }
+			};
+			var crlUrlIndexes = new[] { 0, 2 };
+			var errorContent = IntermediateCertificate1.SubjectDN.ToString ();
+
+			VerifyCrlMissingTest (responses, crlUrlIndexes, errorContent);
+		}
+
+		[Test]
+		public void TestMissingSecondaryIntermediateCrl ()
+		{
+			var responses = new HttpResponseMessage[]
+			{
+			new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(CurrentCrls[0].GetEncoded()) },
+			new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(CurrentCrls[1].GetEncoded()) }
+			};
+			var crlUrlIndexes = new[] { 0, 1 };
+			var errorContent = IntermediateCertificate2.SubjectDN.ToString ();
+
+			VerifyCrlMissingTest (responses, crlUrlIndexes, errorContent);
+		}
+
+		private void VerifyCrlMissingTest (HttpResponseMessage[] responses, int[] crlUrlIndexes, string errorContent)
+		{
+			var mockHttpMessageHandler = new Mock<HttpMessageHandler> (MockBehavior.Strict);
+
+			for (int i = 0; i < crlUrlIndexes.Length; i++) {
+				var requestUri = CrlRequestUris[crlUrlIndexes[i]];
+				var response = responses[i];
+
+				mockHttpMessageHandler
+					.Protected ()
+					.Setup<Task<HttpResponseMessage>> (
+						"SendAsync",
+						ItExpr.Is<HttpRequestMessage> (m => m.Method == HttpMethod.Get && m.RequestUri == requestUri),
+						ItExpr.IsAny<CancellationToken> ())
+					.ReturnsAsync (response);
+			}
+
+			using (var ctx = new MyTemporarySecureMimeContext (mockHttpMessageHandler) { CheckCertificateRevocation = true }) {
+				ImportTestCertificates (ctx);
+
+				VerifyCrlMissing (ctx, errorContent);
+			}
+		}
+
+		[Test]
+		public virtual void TestVerifyCrlsResolvedWiithSecureMimeEncapsulatedSigningWithContext ()
+		{
+			using (var ctx = new MyTemporarySecureMimeContext () { CheckCertificateRevocation = true }) {
+				ImportTestCertificates (ctx);
+
+				VerifyMimeEncapsulatedSigningWithContext (ctx, ctx.MockHttpMessageHandler);
+			}
+		}
 	}
 
 	[TestFixture]