[Package Signing] Integrate the ICertificateVerifier (#331)

Integrates the `ICertificateVerifier` to the Validate Certificate job. The `ICertificateVerifier`'s implementation depends on #309.

Author: loic-sharma

src/Validation.PackageSigning.ValidateCertificate/CertificateValidationMessageHandler.cs

@@ -2,6 +2,9 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Security.Cryptography.X509Certificates;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
@@ -20,18 +23,21 @@ public sealed class CertificateValidationMessageHandler : IMessageHandler<Certif
     {
         private readonly ICertificateStore _certificateStore;
         private readonly ICertificateValidationService _certificateValidationService;
+        private readonly ICertificateVerifier _certificateVerifier;
         private readonly ILogger<CertificateValidationMessageHandler> _logger;
 
         private readonly int _maximumValidationFailures;
 
         public CertificateValidationMessageHandler(
             ICertificateStore certificateStore,
             ICertificateValidationService certificateValidationService,
+            ICertificateVerifier certificateVerifier,
             ILogger<CertificateValidationMessageHandler> logger,
             int maximumValidationFailures = CertificateValidationService.DefaultMaximumValidationFailures)
         {
             _certificateStore = certificateStore ?? throw new ArgumentNullException(nameof(certificateStore));
             _certificateValidationService = certificateValidationService ?? throw new ArgumentNullException(nameof(certificateValidationService));
+            _certificateVerifier = certificateVerifier ?? throw new ArgumentNullException(nameof(certificateVerifier));
             _logger = logger ?? throw new ArgumentNullException(nameof(logger));
 
             _maximumValidationFailures = maximumValidationFailures;
@@ -95,11 +101,28 @@ public async Task<bool> HandleAsync(CertificateValidationMessage message)
                 }
             }
 
-            // Download and verify the certificate.
-            // TODO: Download all parent certificates and pass them to verification "VerifyAsync",
-            // will be done as part of: https://github.com/nuget/engineering/issues/787
-            var certificate = await _certificateStore.LoadAsync(validation.EndCertificate.Thumbprint, CancellationToken.None);
-            var result = await _certificateValidationService.VerifyAsync(certificate);
+            CertificateVerificationResult result;
+
+            using (var certificates = await LoadCertificatesAsync(validation))
+            {
+                switch (validation.EndCertificate.Use)
+                {
+                    case EndCertificateUse.CodeSigning:
+                        result = _certificateVerifier.VerifyCodeSigningCertificate(
+                                    certificates.EndCertificate,
+                                    certificates.AncestorCertificates);
+                        break;
+
+                    case EndCertificateUse.Timestamping:
+                        result = _certificateVerifier.VerifyTimestampingCertificate(
+                                    certificates.EndCertificate,
+                                    certificates.AncestorCertificates);
+                        break;
+
+                    default:
+                        throw new InvalidOperationException($"Unknown {nameof(EndCertificateUse)}: {validation.EndCertificate.Use}");
+                }
+            }
 
             // Save the result. This may alert if packages are invalidated.
             if (!await _certificateValidationService.TrySaveResultAsync(validation, result))
@@ -157,5 +180,45 @@ private bool HasValidationCompleted(EndCertificateValidation validation, Certifi
 
             throw new InvalidOperationException($"Unknown {nameof(EndCertificateStatus)} value: {result.Status}");
         }
+
+        private async Task<LoadCertificatesResult> LoadCertificatesAsync(EndCertificateValidation validation)
+        {
+            // Create a list of all the thumbprints that need to be downloaded. The first thumbprint is the end certificate,
+            // the rest are the end certificate's ancestors.
+            var thumbprints = new List<string>();
+
+            thumbprints.Add(validation.EndCertificate.Thumbprint);
+            thumbprints.AddRange(validation.EndCertificate.CertificateChainLinks.Select(l => l.ParentCertificate.Thumbprint));
+
+            var certificates = await Task.WhenAll(thumbprints.Select(t => _certificateStore.LoadAsync(t, CancellationToken.None)));
+
+            return new LoadCertificatesResult(
+                endCertificate: certificates.First(),
+                ancestorCertificates: certificates.Skip(1).ToArray());
+        }
+
+        private class LoadCertificatesResult : IDisposable
+        {
+            public LoadCertificatesResult(
+                X509Certificate2 endCertificate,
+                IReadOnlyList<X509Certificate2> ancestorCertificates)
+            {
+                EndCertificate = endCertificate ?? throw new ArgumentNullException(nameof(endCertificate));
+                AncestorCertificates = ancestorCertificates ?? throw new ArgumentNullException(nameof(ancestorCertificates));
+            }
+
+            public X509Certificate2 EndCertificate { get; }
+            public IReadOnlyList<X509Certificate2> AncestorCertificates { get; }
+
+            public void Dispose()
+            {
+                EndCertificate.Dispose();
+
+                foreach (var ancestor in AncestorCertificates)
+                {
+                    ancestor.Dispose();
+                }
+            }
+        }
     }
-}
+}

src/Validation.PackageSigning.ValidateCertificate/CertificateValidationService.cs

@@ -43,19 +43,15 @@ public CertificateValidationService(
 
         public Task<EndCertificateValidation> FindCertificateValidationAsync(CertificateValidationMessage message)
         {
+            // Fetch the validation, the end certificate that this validation is for, and all of the parent
+            // certificates that the end certificate depends on.
             return _context
                         .CertificateValidations
                         .Where(v => v.ValidationId == message.ValidationId && v.EndCertificateKey == message.CertificateKey)
-                        .Include(v => v.EndCertificate)
+                        .Include(v => v.EndCertificate.CertificateChainLinks.Select(l => l.ParentCertificate))
                         .FirstOrDefaultAsync();
         }
 
-        public Task<CertificateVerificationResult> VerifyAsync(X509Certificate2 certificate)
-        {
-            // TODO: This will be implemented in a separate change!
-            throw new NotImplementedException();
-        }
-
         public async Task<bool> TrySaveResultAsync(EndCertificateValidation validation, CertificateVerificationResult result)
         {
             if (validation.EndCertificate.Status == EndCertificateStatus.Revoked && result.Status != EndCertificateStatus.Revoked)
@@ -171,6 +167,7 @@ void InvalidateCertificate()
 
             return ProcessDependentSignaturesAsync(
                         validation.EndCertificate,
+                        result,
                         invalidationDecider,
                         onAllSignaturesHandled: InvalidateCertificate);
         }
@@ -193,6 +190,7 @@ void RevokeCertificate()
 
             return ProcessDependentSignaturesAsync(
                         validation.EndCertificate,
+                        result,
                         invalidationDecider,
                         onAllSignaturesHandled: RevokeCertificate);
         }
@@ -201,11 +199,13 @@ void RevokeCertificate()
         /// The helper that processes how a certificate's status change affects its dependent signatures.
         /// </summary>
         /// <param name="certificate">The certificate whose dependent signatures should be processed.</param>
+        /// <param name="certificateVerificationResult">The result of the certificate's verification.</param>
         /// <param name="signatureDecider">The delegate that decides how a dependent signature should be handled.</param>
         /// <param name="onAllSignaturesHandled">The action that will be called once all dependent signatures have been processed.</param>
         /// <returns></returns>
         private async Task ProcessDependentSignaturesAsync(
             EndCertificate certificate,
+            CertificateVerificationResult certificateVerificationResult,
             SignatureDecider signatureDecider,
             Action onAllSignaturesHandled)
         {
@@ -246,7 +246,7 @@ private async Task ProcessDependentSignaturesAsync(
                 {
                     var decision = signatureDecider(signature);
 
-                    HandleSignatureDecision(signature, decision);
+                    HandleSignatureDecision(signature, decision, certificate, certificateVerificationResult);
                 }
             }
             while (signatures.Count == MaxSignatureUpdatesPerTransaction);
@@ -294,7 +294,7 @@ private Task<List<PackageSignature>> FindSignaturesAsync(EndCertificate certific
             }
 
             return packageSignatures
-                        .Include(s => s.TrustedTimestamps)
+                        .Include(s => s.TrustedTimestamps.Select(t => t.EndCertificate))
                         .Include(s => s.PackageSigningState)
                         .OrderBy(s => s.Key)
                         .Skip(page * MaxSignatureUpdatesPerTransaction)
@@ -307,33 +307,72 @@ private Task<List<PackageSignature>> FindSignaturesAsync(EndCertificate certific
         /// Handle the decision on how to update the signature.
         /// </summary>
         /// <param name="signature">The signature that should be updated.</param>
-        /// <param name="decision"></param>
+        /// <param name="decision">How the signature should be updated.</param>
+        /// <param name="certificate">The certificate that signature depends on that changed the signature's state.</param>
+        /// <param name="certificateVerificationResult">The certificate verification that changed the signature's state.</param>
         private void HandleSignatureDecision(
             PackageSignature signature,
-            SignatureDecision decision)
+            SignatureDecision decision,
+            EndCertificate certificate,
+            CertificateVerificationResult certificateVerificationResult)
         {
-            // TODO: Log all the necessary information to investigate the signature decision.
             switch (decision)
             {
                 case SignatureDecision.Ignore:
+                    _logger.LogInformation(
+                        "Signature {SignatureKey} is not affected by certificate verification result: {CertificateVerificationResult}",
+                        signature.Key,
+                        certificateVerificationResult);
                     break;
 
                 case SignatureDecision.Warn:
-                    signature.Status = PackageSignatureStatus.Invalid;
-                    signature.PackageSigningState.SigningStatus = PackageSigningStatus.Invalid;
+                    _logger.LogWarning(
+                        "Invalidating signature {SignatureKey} due to certificate verification result: {CertificateVerificationResult}",
+                        signature.Key,
+                        certificateVerificationResult);
+
+                    InvalidateSignature(signature, certificate);
 
                     _telemetryService.TrackPackageSignatureMayBeInvalidatedEvent(signature);
+
                     break;
 
                 case SignatureDecision.Reject:
-                    signature.Status = PackageSignatureStatus.Invalid;
-                    signature.PackageSigningState.SigningStatus = PackageSigningStatus.Invalid;
+                    _logger.LogWarning(
+                        "Rejecting signature {SignatureKey} due to certificate verification result: {CertificateVerificationResult}",
+                        signature.Key,
+                        certificateVerificationResult);
+
+                    InvalidateSignature(signature, certificate);
 
                     _telemetryService.TrackPackageSignatureShouldBeInvalidatedEvent(signature);
                     break;
 
                 default:
-                    throw new InvalidOperationException($"Unknown signature decision: {decision}");
+                    throw new InvalidOperationException(
+                        $"Unknown signature decision '{decision}' for certificate verification result: {certificateVerificationResult}");
+            }
+        }
+
+        private void InvalidateSignature(PackageSignature signature, EndCertificate certificate)
+        {
+            signature.Status = PackageSignatureStatus.Invalid;
+            signature.PackageSigningState.SigningStatus = PackageSigningStatus.Invalid;
+
+            if (certificate.Use == EndCertificateUse.Timestamping)
+            {
+                var affectedTimestamps = signature.TrustedTimestamps
+                                                  .Where(t => t.EndCertificate.Thumbprint == certificate.Thumbprint);
+
+                foreach (var timestamp in affectedTimestamps)
+                {
+                    _logger.LogWarning(
+                        "Invalidating timestamp {TimestampKey} due to invalid certificate {CertificateKey}",
+                        signature.Key,
+                        certificate.Key);
+
+                    timestamp.Status = TrustedTimestampStatus.Invalid;
+                }
             }
         }
     }

src/Validation.PackageSigning.ValidateCertificate/ICertificateValidationService.cs

@@ -1,7 +1,6 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System.Security.Cryptography.X509Certificates;
 using System.Threading.Tasks;
 using NuGet.Jobs.Validation.PackageSigning.Messages;
 using NuGet.Services.Validation;
@@ -17,14 +16,6 @@ public interface ICertificateValidationService
         /// <returns>The entity representing the certificate validation's state, or null if one could not be found.</returns>
         Task<EndCertificateValidation> FindCertificateValidationAsync(CertificateValidationMessage message);
 
-        /// <summary>
-        /// Verify the certificate. Ensures the certificate is well-formed
-        /// and does online revocation checking.
-        /// </summary>
-        /// <param name="certificate">The certificate to validate.</param>
-        /// <returns>The result of the verification.</returns>
-        Task<CertificateVerificationResult> VerifyAsync(X509Certificate2 certificate);
-
         /// <summary>
         /// Update the requested <see cref="CertificateValidation"/> with the <see cref="CertificateVerificationResult"/>.
         /// This may kick off alerts if packages are invalidated!

src/Validation.PackageSigning.ValidateCertificate/ICertificateVerifier.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Security.Cryptography.X509Certificates;
 
 namespace Validation.PackageSigning.ValidateCertificate
@@ -26,14 +27,14 @@ public interface ICertificateVerifier
         /// <param name="certificate">The certificate to verify.</param>
         /// <param name="extraCertificates">A collection of certificates that may be used to build the certificate chain.</param>
         /// <returns>The result of the verification.</returns>
-        CertificateVerificationResult VerifyCodeSigningCertificate(X509Certificate2 certificate, X509Certificate2[] extraCertificates);
+        CertificateVerificationResult VerifyCodeSigningCertificate(X509Certificate2 certificate, IReadOnlyList<X509Certificate2> extraCertificates);
 
         /// <summary>
         /// Determine the status of a timestamping <see cref="X509Certificate2"/>.
         /// </summary>
         /// <param name="certificate">The certificate to verify.</param>
         /// <param name="extraCertificates">A collection of certificates that may be used to build the certificate chain.</param>
         /// <returns>The result of the verification.</returns>
-        CertificateVerificationResult VerifyTimestampingCertificate(X509Certificate2 certificate, X509Certificate2[] extraCertificates);
+        CertificateVerificationResult VerifyTimestampingCertificate(X509Certificate2 certificate, IReadOnlyList<X509Certificate2> extraCertificates);
     }
 }

src/Validation.PackageSigning.ValidateCertificate/OnlineCertificateVerifier.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
@@ -44,17 +45,17 @@ public CertificateVerificationResult VerifyCertificate(X509Certificate2 certific
             return VerifyCertificate(certificate, extraCertificates, applicationPolicy: null);
         }
 
-        public CertificateVerificationResult VerifyCodeSigningCertificate(X509Certificate2 certificate, X509Certificate2[] extraCertificates)
+        public CertificateVerificationResult VerifyCodeSigningCertificate(X509Certificate2 certificate, IReadOnlyList<X509Certificate2> extraCertificates)
         {
             return VerifyCertificate(certificate, extraCertificates, applicationPolicy: new Oid(CodeSigningEku));
         }
 
-        public CertificateVerificationResult VerifyTimestampingCertificate(X509Certificate2 certificate, X509Certificate2[] extraCertificates)
+        public CertificateVerificationResult VerifyTimestampingCertificate(X509Certificate2 certificate, IReadOnlyList<X509Certificate2> extraCertificates)
         {
             return VerifyCertificate(certificate, extraCertificates, applicationPolicy: new Oid(TimeStampingEku));
         }
 
-        private CertificateVerificationResult VerifyCertificate(X509Certificate2 certificate, X509Certificate2[] extraCertificates, Oid applicationPolicy)
+        private CertificateVerificationResult VerifyCertificate(X509Certificate2 certificate, IReadOnlyList<X509Certificate2> extraCertificates, Oid applicationPolicy)
         {
             X509Chain chain = null;
 
@@ -63,7 +64,7 @@ private CertificateVerificationResult VerifyCertificate(X509Certificate2 certifi
                 chain = new X509Chain();
 
                 // Allow the chain to use whatever additional extra certificates were provided.
-                chain.ChainPolicy.ExtraStore.AddRange(extraCertificates);
+                chain.ChainPolicy.ExtraStore.AddRange(extraCertificates.ToArray());
 
                 if (applicationPolicy != null)
                 {