[Repository Signing] Strip repository signatures that fail verification (#441)

Previously, the Process Signature job only stripped repository signatures if its service index URL or certificate were unexpected. Thus, invalid repository signatures wouldn't be stripped, and the package would fail validation. This change strips repository signatures that fail verification.

Addresses https://github.com/NuGet/Engineering/issues/1326
Addresses https://github.com/NuGet/Engineering/issues/1401

Author: loic-sharma

src/Validation.Common.Job/Validation.Common.Job.csproj

@@ -88,7 +88,7 @@
       <Version>1.1.2</Version>
     </PackageReference>
     <PackageReference Include="NuGet.Packaging">
-      <Version>4.8.0-preview1.5179</Version>
+      <Version>4.8.0-preview1.5195</Version>
     </PackageReference>
     <PackageReference Include="NuGet.Services.Configuration">
       <Version>2.23.0</Version>

src/Validation.Common.Job/Validation.Common.Job.nuspec

@@ -15,7 +15,7 @@
         <dependency id="Microsoft.ApplicationInsights" version="2.2.0" />
         <dependency id="Microsoft.Extensions.DependencyInjection" version="1.1.1" />
         <dependency id="Microsoft.Extensions.Options.ConfigurationExtensions" version="1.1.2" />
-        <dependency id="NuGet.Packaging" version="4.8.0-preview1.5179" />
+        <dependency id="NuGet.Packaging" version="4.8.0-preview1.5195" />
         <dependency id="NuGet.Services.Configuration" version="2.23.0" />
         <dependency id="NuGet.Services.Logging" version="2.23.0" />
         <dependency id="NuGet.Services.Storage" version="2.23.0" />

src/Validation.PackageSigning.ProcessSignature/ISignatureFormatValidator.cs

@@ -19,6 +19,16 @@ Task<VerifySignaturesResult> ValidateMinimalAsync(
             ISignedPackageReader package,
             CancellationToken token);
 
+        /// <summary>
+        /// Run all validations on the package's repository signature. This includes integrity and trust validations.
+        /// </summary>
+        /// <param name="package">The package whose repository signature should be validated.</param>
+        /// <param name="token"></param>
+        /// <returns>Whether the package's signature is readable.</returns>
+        Task<SignatureVerificationStatus> VerifyRepositorySignatureAsync(
+            ISignedPackageReader package,
+            CancellationToken token);
+
         /// <summary>
         /// Run all validations on the package's signature. This includes integrity and trust validations.
         /// </summary>

src/Validation.PackageSigning.ProcessSignature/SignatureFormatValidator.cs

@@ -44,6 +44,8 @@ public class SignatureFormatValidator : ISignatureFormatValidator
         private readonly SignedPackageVerifierSettings _authorSignatureSettings;
         private readonly SignedPackageVerifierSettings _authorOrRepositorySignatureSettings;
 
+        private readonly RepositorySignatureVerifier _repositorySignatureVerifier;
+
         public SignatureFormatValidator(IOptionsSnapshot<ProcessSignatureConfiguration> config)
         {
             _config = config ?? throw new ArgumentNullException(nameof(config));
@@ -87,6 +89,8 @@ public SignatureFormatValidator(IOptionsSnapshot<ProcessSignatureConfiguration>
                 clientAllowListEntries: _authorSignatureSettings.ClientCertificateList,
                 allowNoRepositoryCertificateList: false,
                 repoAllowListEntries: repoAllowListEntries);
+
+            _repositorySignatureVerifier = new RepositorySignatureVerifier();
         }
 
         public async Task<VerifySignaturesResult> ValidateMinimalAsync(
@@ -114,6 +118,13 @@ public async Task<VerifySignaturesResult> ValidateFullAsync(
                 token);
         }
 
+        public async Task<SignatureVerificationStatus> VerifyRepositorySignatureAsync(
+            ISignedPackageReader package,
+            CancellationToken token)
+        {
+            return await _repositorySignatureVerifier.VerifyAsync(package, token);
+        }
+
         private static async Task<VerifySignaturesResult> VerifyAsync(
             ISignedPackageReader package,
             IEnumerable<ISignatureVerificationProvider> verificationProviders,

src/Validation.PackageSigning.ProcessSignature/SignatureValidator.cs

@@ -2,7 +2,6 @@
 // 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.Diagnostics;
 using System.IO;
 using System.Linq;
@@ -239,7 +238,7 @@ private async Task<SignatureValidatorResult> PerformInitialValidationsAsync(Cont
         private async Task<SignatureValidatorResult> StripUnacceptableRepositorySignaturesAsync(Context context)
         {
             // Check if the repository signing certificates are acceptable.
-            if (HasAllValidRepositorySignatures(context))
+            if (await HasAllValidRepositorySignaturesAsync(context))
             {
                 _logger.LogInformation(
                     "No repository signatures needed removal from package {PackageId} {PackageVersion} for " +
@@ -348,11 +347,11 @@ private async Task<SignatureValidatorResult> StripUnacceptableRepositorySignatur
             return null;
         }
 
-        private bool HasAllValidRepositorySignatures(Context context)
+        private async Task<bool> HasAllValidRepositorySignaturesAsync(Context context)
         {
             if (context.Signature.Type == SignatureType.Repository)
             {
-                if (!IsValidRepositorySignature(context, (RepositoryPrimarySignature)context.Signature))
+                if (!await IsValidRepositorySignatureAsync(context, (RepositoryPrimarySignature)context.Signature))
                 {
                     _logger.LogInformation(
                         "Signed package {PackageId} {PackageVersion} for validation {ValidationId} has repository " +
@@ -370,7 +369,7 @@ private bool HasAllValidRepositorySignatures(Context context)
                 var repositoryCounterSignature = RepositoryCountersignature.GetRepositoryCountersignature(context.Signature);
 
                 if (repositoryCounterSignature != null
-                    && !IsValidRepositorySignature(context, repositoryCounterSignature))
+                    && !await IsValidRepositorySignatureAsync(context, repositoryCounterSignature))
                 {
                     _logger.LogInformation(
                         "Signed package {PackageId} {PackageVersion} for validation {ValidationId} has repository " +
@@ -400,7 +399,7 @@ private bool HasAllValidRepositorySignatures(Context context)
             return true;
         }
 
-        private bool IsValidRepositorySignature<T>(Context context, T signature)
+        private async Task<bool> IsValidRepositorySignatureAsync<T>(Context context, T signature)
             where T : Signature, IRepositorySignature
         {
             if (signature.V3ServiceIndexUrl?.AbsoluteUri != _configuration.Value.V3ServiceIndexUrl)
@@ -431,20 +430,41 @@ private bool IsValidRepositorySignature<T>(Context context, T signature)
                 return false;
             }
 
-            if (signature.Timestamps.Count != 1)
+            var status = await _formatValidator.VerifyRepositorySignatureAsync(context.PackageReader, context.CancellationToken);
+
+            if (status == SignatureVerificationStatus.Valid)
+            {
+                _logger.LogInformation(
+                    "Signed package {PackageId} {PackageVersion} for validation {ValidationId} has a valid repository signature",
+                    context.Message.PackageId,
+                    context.Message.PackageVersion,
+                    context.Message.ValidationId);
+
+                return true;
+            }
+            else if (status == SignatureVerificationStatus.Suspect && await _packageSigningStateService.HasValidPackageSigningStateAsync(context.PackageKey))
+            {
+                _logger.LogCritical(
+                    "Detected suspect repository signature on revalidation of package {PackageId} {PackageVersion} for " +
+                    "validation {ValidationId}",
+                    context.Message.PackageId,
+                    context.Message.PackageVersion,
+                    context.Message.ValidationId);
+
+                throw new InvalidOperationException($"Suspect repository signature for validation id '{context.Message.ValidationId}'");
+            }
+            else
             {
                 _logger.LogInformation(
-                    "Signed package {PackageId} {PackageVersion} for validation {ValidationId} has a repository " +
-                    "signature with an improper number of timestamps: {Count}.",
+                    "Package {PackageId} {PackageVersion} for validation {ValidationId} has an unacceptable repository " +
+                    "signature with status {Status} and will be stripped",
                     context.Message.PackageId,
                     context.Message.PackageVersion,
                     context.Message.ValidationId,
-                    signature.Timestamps.Count);
+                    status);
 
                 return false;
             }
-
-            return true;
         }
 
         private async Task<SignatureValidatorResult> PerformFinalValidationAsync(Context context)

src/Validation.PackageSigning.ProcessSignature/Storage/IPackageSigningStateService.cs

@@ -13,5 +13,7 @@ Task SetPackageSigningState(
             string packageId,
             string packageVersion,
             PackageSigningStatus status);
+
+        Task<bool> HasValidPackageSigningStateAsync(int packageKey);
     }
 }

src/Validation.PackageSigning.ProcessSignature/Storage/PackageSigningStateService.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Data.Entity;
+using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using NuGet.Services.Validation;
@@ -59,5 +60,12 @@ public async Task SetPackageSigningState(
                 });
             }
         }
+
+        public async Task<bool> HasValidPackageSigningStateAsync(int packageKey)
+        {
+            return await _validationContext.PackageSigningStates
+                .Where(s => s.PackageKey == packageKey)
+                .AnyAsync(s => s.SigningStatus == PackageSigningStatus.Valid);
+        }
     }
 }