Skip to content

Commit

Permalink
Include Intermediate Certificate Resolution for S/MIME and Ensure Tru…
Browse files Browse the repository at this point in the history
…st Validation during BuildCertificateChain() (#1124)

* 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.

* ValidateCertificateChain

Move the IO operations to ValidateCertificateChain and ValidateCertificateChainAsync.  Removed IO from BuildCertificateChain.

* Fixup CRL tests
  • Loading branch information
JoeShook authored Jan 1, 2025
1 parent 49bb9d4 commit ea87b4d
Show file tree
Hide file tree
Showing 11 changed files with 712 additions and 49 deletions.
202 changes: 193 additions & 9 deletions MimeKit/Cryptography/BouncyCastleSecureMimeContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
using IssuerAndSerialNumber = Org.BouncyCastle.Asn1.Cms.IssuerAndSerialNumber;

using MimeKit.IO;
using System.Linq;
using Org.BouncyCastle.Tls;

namespace MimeKit.Cryptography {
/// <summary>
Expand Down Expand Up @@ -169,6 +171,9 @@ protected virtual HttpClient HttpClient {
/// generally issued by a certificate authority (CA).</para>
/// <para>This method is used to build a certificate chain while verifying
/// signed content.</para>
/// <para>It is critical to always load the designated trust anchors
/// and not the anchor in the end certificate when building a certificate chain
/// to validated trust.</para>
/// </remarks>
/// <returns>The trusted anchors.</returns>
protected abstract ISet<TrustAnchor> GetTrustedAnchors ();
Expand Down Expand Up @@ -325,6 +330,9 @@ CmsSignedDataStreamGenerator CreateSignedDataGenerator (CmsSigner signer)

Stream Sign (CmsSigner signer, Stream content, bool encapsulate, CancellationToken cancellationToken)
{
if (CheckCertificateRevocation)
ValidateCertificateChain (signer.CertificateChain, DateTime.UtcNow, cancellationToken);

var signedData = CreateSignedDataGenerator (signer);
var memory = new MemoryBlockStream ();

Expand All @@ -339,6 +347,9 @@ Stream Sign (CmsSigner signer, Stream content, bool encapsulate, CancellationTok

async Task<Stream> SignAsync (CmsSigner signer, Stream content, bool encapsulate, CancellationToken cancellationToken)
{
if (CheckCertificateRevocation)
await ValidateCertificateChainAsync (signer.CertificateChain, DateTime.UtcNow, cancellationToken);

var signedData = CreateSignedDataGenerator (signer);
var memory = new MemoryBlockStream ();

Expand Down Expand Up @@ -694,20 +705,31 @@ 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 issuerStore = GetTrustedAnchors ();
var anchorStore = new X509CertificateStore ();

foreach (var anchor in issuerStore) {
anchorStore.Add (anchor.TrustedCert);
}

var parameters = new PkixBuilderParameters (issuerStore, selector) {
ValidityModel = PkixParameters.PkixValidityModel,
IsRevocationEnabled = false,
Date = DateTime.UtcNow
};
parameters.AddStoreCert (intermediates);
parameters.AddStoreCert (GetIntermediateCertificates ());
parameters.AddStoreCert (userCertificateStore);

var intermediateStore = GetIntermediateCertificates ();

foreach (var intermediate in intermediateStore.EnumerateMatches (new X509CertStoreSelector ()))
anchorStore.Add (intermediate);

parameters.AddStoreCert (anchorStore);

var builder = new PkixCertPathBuilder ();
var result = builder.Build (parameters);
Expand All @@ -720,6 +742,158 @@ protected IList<X509Certificate> BuildCertificateChain (X509Certificate certific
return chain;
}

/// <summary>
/// Validate an S/MIME certificate chain.
/// </summary>
/// <remarks>
/// <para>Validates an S/MIME certificate chain.</para>
/// <para>Downloads the CRLs for each certificate in the chain and then validates that the chain
/// is both valid and that none of the certificates in the chain have been revoked or compromised
/// in any way.</para>
/// </remarks>
/// <returns><c>true</c> if the certificate chain is valid; otherwise, <c>false</c>.</returns>
/// <param name="chain">The S/MIME certificate chain.</param>
/// <param name="dateTime">The date and time to use for validation.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <exception cref="ArgumentNullException">
/// <paramref name="chain"/> is <see langword="null"/>.
/// </exception>
/// <exception cref="ArgumentException">
/// <paramref name="chain"/> is empty or contains a <see langword="null"/> certificate.
/// </exception>
public bool ValidateCertificateChain (X509CertificateChain chain, DateTime dateTime, CancellationToken cancellationToken = default)
{
if (chain == null)
throw new ArgumentNullException (nameof (chain));

if (chain.Count == 0)
throw new ArgumentException ("The certificate chain must contain at least one certificate.", nameof (chain));

if (chain.Any (certificate => certificate == null))
throw new ArgumentException ("The certificate chain contains at least one null certificate.", nameof (chain));

var selector = new X509CertStoreSelector ();

var userCertificateStore = new X509CertificateStore ();
userCertificateStore.AddRange (chain);

var issuerStore = GetTrustedAnchors ();
var anchorStore = new X509CertificateStore ();

foreach (var anchor in issuerStore) {
anchorStore.Add (anchor.TrustedCert);
}

var parameters = new PkixBuilderParameters (issuerStore, selector) {
ValidityModel = PkixParameters.PkixValidityModel,
IsRevocationEnabled = false,
Date = DateTime.UtcNow
};
parameters.AddStoreCert (userCertificateStore);

if (CheckCertificateRevocation) {
foreach (var certificate in chain)
DownloadCrls (certificate, cancellationToken);
}

var intermediateStore = GetIntermediateCertificates ();

foreach (var intermediate in intermediateStore.EnumerateMatches (new X509CertStoreSelector ())) {
anchorStore.Add (intermediate);
if (CheckCertificateRevocation)
DownloadCrls (intermediate, cancellationToken);
}

parameters.AddStoreCert (anchorStore);

if (CheckCertificateRevocation)
parameters.AddStoreCrl (GetCertificateRevocationLists ());

try {
var builder = new PkixCertPathBuilder ();
builder.Build (parameters);
return true;
} catch {
return false;
}
}

/// <summary>
/// Validate an S/MIME certificate chain.
/// </summary>
/// <remarks>
/// <para>Asynchronously validates an S/MIME certificate chain.</para>
/// <para>Downloads the CRLs for each certificate in the chain and then validates that the chain
/// is both valid and that none of the certificates in the chain have been revoked or compromised
/// in any way.</para>
/// </remarks>
/// <returns><c>true</c> if the certificate chain is valid; otherwise, <c>false</c>.</returns>
/// <param name="chain">The S/MIME certificate chain.</param>
/// <param name="dateTime">The date and time to use for validation.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <exception cref="ArgumentNullException">
/// <paramref name="chain"/> is <see langword="null"/>.
/// </exception>
/// <exception cref="ArgumentException">
/// <paramref name="chain"/> is empty or contains a <see langword="null"/> certificate.
/// </exception>
public async Task<bool> ValidateCertificateChainAsync (X509CertificateChain chain, DateTime dateTime, CancellationToken cancellationToken = default)
{
if (chain == null)
throw new ArgumentNullException (nameof (chain));

if (chain.Count == 0)
throw new ArgumentException ("The certificate chain must contain at least one certificate.", nameof (chain));

if (chain.Any (certificate => certificate == null))
throw new ArgumentException ("The certificate chain contains at least one null certificate.", nameof (chain));

var selector = new X509CertStoreSelector ();

var userCertificateStore = new X509CertificateStore ();
userCertificateStore.AddRange (chain);

var issuerStore = GetTrustedAnchors ();
var anchorStore = new X509CertificateStore ();

foreach (var anchor in issuerStore) {
anchorStore.Add (anchor.TrustedCert);
}

var parameters = new PkixBuilderParameters (issuerStore, selector) {
ValidityModel = PkixParameters.PkixValidityModel,
IsRevocationEnabled = false,
Date = DateTime.UtcNow
};
parameters.AddStoreCert (userCertificateStore);

if (CheckCertificateRevocation) {
foreach (var certificate in chain)
await DownloadCrlsAsync (certificate, cancellationToken).ConfigureAwait (false);
}

var intermediateStore = GetIntermediateCertificates ();

foreach (var intermediate in intermediateStore.EnumerateMatches (new X509CertStoreSelector ())) {
anchorStore.Add (intermediate);
if (CheckCertificateRevocation)
await DownloadCrlsAsync (intermediate, cancellationToken).ConfigureAwait (false);
}

parameters.AddStoreCert (anchorStore);

if (CheckCertificateRevocation)
parameters.AddStoreCrl (GetCertificateRevocationLists ());

try {
var builder = new PkixCertPathBuilder ();
builder.Build (parameters);
return true;
} catch {
return false;
}
}

PkixCertPath BuildCertPath (ISet<TrustAnchor> anchors, IStore<X509Certificate> certificates, IStore<X509Crl> crls, X509Certificate certificate, DateTime signingTime)
{
var selector = new X509CertStoreSelector {
Expand Down Expand Up @@ -995,7 +1169,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;
Expand Down Expand Up @@ -1125,10 +1299,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 {
Expand Down Expand Up @@ -1179,10 +1358,15 @@ async Task<DigitalSignatureCollection> GetDigitalSignaturesAsync (CmsSignedDataP
}

var anchors = GetTrustedAnchors ();
var intermediates = GetIntermediateCertificates ();

if (CheckCertificateRevocation) {
foreach (var anchor in anchors)
await DownloadCrlsAsync (anchor.TrustedCert, cancellationToken).ConfigureAwait (false);

foreach (X509Certificate intermediate in intermediates.EnumerateMatches (new X509CertStoreSelector ())) {
await DownloadCrlsAsync (intermediate, cancellationToken);
}
}

try {
Expand Down
21 changes: 10 additions & 11 deletions MimeKit/Cryptography/DefaultSecureMimeContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
13 changes: 13 additions & 0 deletions UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@
using MimeKit.Cryptography;

using BCX509Certificate = Org.BouncyCastle.X509.X509Certificate;
using Org.BouncyCastle.Cms;
using Org.BouncyCastle.Crypto;
using Org.BouncyCastle.OpenSsl;

using Org.BouncyCastle.Crypto;

Check warning on line 42 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Release)

The using directive for 'Org.BouncyCastle.Crypto' appeared previously in this namespace

Check warning on line 42 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Debug)

The using directive for 'Org.BouncyCastle.Crypto' appeared previously in this namespace

Check warning on line 42 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Release)

The using directive for 'Org.BouncyCastle.Crypto' appeared previously in this namespace

Check warning on line 42 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

The using directive for 'Org.BouncyCastle.Crypto' appeared previously in this namespace

Check warning on line 42 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Debug)

The using directive for 'Org.BouncyCastle.Crypto' appeared previously in this namespace
using Org.BouncyCastle.OpenSsl;

Check warning on line 43 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Release)

The using directive for 'Org.BouncyCastle.OpenSsl' appeared previously in this namespace

Check warning on line 43 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Debug)

The using directive for 'Org.BouncyCastle.OpenSsl' appeared previously in this namespace

Check warning on line 43 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Release)

The using directive for 'Org.BouncyCastle.OpenSsl' appeared previously in this namespace

Check warning on line 43 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

The using directive for 'Org.BouncyCastle.OpenSsl' appeared previously in this namespace

Check warning on line 43 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Debug)

The using directive for 'Org.BouncyCastle.OpenSsl' appeared previously in this namespace
using Org.BouncyCastle.Security;

Check warning on line 44 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Release)

The using directive for 'Org.BouncyCastle.Security' appeared previously in this namespace

Check warning on line 44 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Debug)

The using directive for 'Org.BouncyCastle.Security' appeared previously in this namespace

Check warning on line 44 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Release)

The using directive for 'Org.BouncyCastle.Security' appeared previously in this namespace

Check warning on line 44 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

The using directive for 'Org.BouncyCastle.Security' appeared previously in this namespace

Check warning on line 44 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Debug)

The using directive for 'Org.BouncyCastle.Security' appeared previously in this namespace
using Org.BouncyCastle.X509;

Check warning on line 45 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Release)

The using directive for 'Org.BouncyCastle.X509' appeared previously in this namespace

Check warning on line 45 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Debug)

The using directive for 'Org.BouncyCastle.X509' appeared previously in this namespace

Check warning on line 45 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Release)

The using directive for 'Org.BouncyCastle.X509' appeared previously in this namespace

Check warning on line 45 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

The using directive for 'Org.BouncyCastle.X509' appeared previously in this namespace

Check warning on line 45 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Debug)

The using directive for 'Org.BouncyCastle.X509' appeared previously in this namespace
using Org.BouncyCastle.Pkcs;
using Org.BouncyCastle.Cms;

Check warning on line 47 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Release)

The using directive for 'Org.BouncyCastle.Cms' appeared previously in this namespace

Check warning on line 47 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (ubuntu-latest, Debug)

The using directive for 'Org.BouncyCastle.Cms' appeared previously in this namespace

Check warning on line 47 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Release)

The using directive for 'Org.BouncyCastle.Cms' appeared previously in this namespace

Check warning on line 47 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / Analyze (csharp)

The using directive for 'Org.BouncyCastle.Cms' appeared previously in this namespace

Check warning on line 47 in UnitTests/Cryptography/ApplicationPkcs7MimeTests.cs

View workflow job for this annotation

GitHub Actions / ci (windows-latest, Debug)

The using directive for 'Org.BouncyCastle.Cms' appeared previously in this namespace
using Org.BouncyCastle.Crypto.Encodings;
using Org.BouncyCastle.Crypto.Engines;
using Org.BouncyCastle.Crypto.Parameters;

namespace UnitTests.Cryptography {
[TestFixture]
Expand Down
Loading

0 comments on commit ea87b4d

Please sign in to comment.