Make Certificate Validator gracefully handle repository signed packages (#431)

The certificate validator expected a single author signature to be present if the package was marked as signed. This is no longer true as the package could be repository signed with no author signature.

Author: loic-sharma

src/NuGet.Services.Validation.Orchestrator/PackageSigning/ValidateCertificate/PackageCertificatesValidator.cs

@@ -86,7 +86,19 @@ private async Task<ValidatorStatus> GetStatusAsync(IValidationRequest request)
             // may have a status of "Unknown" if the package is at ingestion and its signature has passed
             // all validations, "Invalid" if one or more of the signature's certificates has failed validations,
             // or "InGracePeriod" or "Valid" if this is a revalidation request.
-            var signature = await FindSignatureAsync(request);
+            var signature = await FindAuthorSignatureAsync(request);
+
+            if (signature == null)
+            {
+                _logger.LogError(
+                    "Could not find author signature for {PackageKey} {PackageId} {PackageVersion} {ValidationId}",
+                    request.PackageKey,
+                    request.PackageId,
+                    request.PackageVersion,
+                    request.ValidationId);
+
+                throw new InvalidOperationException($"Package with key {request.PackageKey} does not have an author signature");
+            }
 
             if (signature.Status == PackageSignatureStatus.Invalid)
             {
@@ -160,12 +172,22 @@ private async Task<ValidatorStatus> StartInternalAsync(IValidationRequest reques
                 return await _validatorStateService.TryAddValidatorStatusAsync(request, status, ValidationStatus.Failed);
             }
 
-            var isRevalidationRequest = await _validatorStateService.IsRevalidationRequestAsync(request);
+            // Skip packages that are only repository signed.
+            var signature = await FindAuthorSignatureAsync(request);
 
-            // Find the signatures used to sign the package and see if any certificates known to be revoked
-            // invalidate any of these signatures. Note that a revoked certificate is assumed to remain
-            // revoked forever.
-            var signature = await FindSignatureAsync(request);
+            if (signature == null)
+            {
+                _logger.LogInformation(
+                    "Package {PackageId} {PackageVersion} does not have an author signature, no additional validations necessary",
+                    request.PackageId,
+                    request.PackageVersion);
+
+                return await _validatorStateService.TryAddValidatorStatusAsync(request, status, ValidationStatus.Succeeded);
+            }
+
+            // If any of the author signature's certificates are known to be revoked, invalidate any the signatures.
+            // A revoked certificate is assumed to remain revoked forever.
+            var isRevalidationRequest = await _validatorStateService.IsRevalidationRequestAsync(request);
 
             if (ShouldInvalidateSignature(signature, isRevalidationRequest))
             {
@@ -226,18 +248,18 @@ private Task<PackageSigningState> FindPackageSigningStateAsync(IValidationReques
         }
 
         /// <summary>
-        /// Find all of the signatures and their certificates for the given validation request's package.
+        /// Find the package's author signature, if one exists.
         /// </summary>
         /// <param name="request">The validation request containing the package whose signatures should be fetched.</param>
-        /// <returns>The package's signatures with their certificates.</returns>
-        private Task<PackageSignature> FindSignatureAsync(IValidationRequest request)
+        /// <returns>The package's author signature with its certificates, or null.</returns>
+        private Task<PackageSignature> FindAuthorSignatureAsync(IValidationRequest request)
         {
             return _validationContext
                         .PackageSignatures
                         .Include(s => s.EndCertificate)
                         .Include(s => s.TrustedTimestamps.Select(t => t.EndCertificate))
                         .Where(s => s.Type == PackageSignatureType.Author)
-                        .SingleAsync(s => s.PackageKey == request.PackageKey);
+                        .SingleOrDefaultAsync(s => s.PackageKey == request.PackageKey);
         }
 
         /// <summary>

tests/NuGet.Services.Validation.Orchestrator.Tests/PackageSigning/ValidateCertificate/PackageCertificatesValidatorFacts.cs

@@ -536,6 +536,70 @@ public async Task ThrowsIfValidSignaturesHasTimestampWithRevokedCertificate()
 
                 Assert.Equal($"Package signature {signature.Key} is valid but has a timestamp whose end certificate is revoked\r\nParameter name: signature", ex.Message);
             }
+
+            [Fact]
+            public async Task ThrowsIfPackageIsOnlyRepositorySigned()
+            {
+                var validatorStatus = new ValidatorStatus
+                {
+                    ValidationId = ValidationId,
+                    ValidatorName = nameof(PackageCertificatesValidator),
+                    PackageKey = PackageKey,
+                    State = ValidationStatus.Incomplete,
+                    ValidatorIssues = new List<ValidatorIssue>(),
+                };
+
+                var packageSigningState = new PackageSigningState
+                {
+                    PackageKey = PackageKey,
+                    PackageId = PackageId,
+                    PackageNormalizedVersion = PackageNormalizedVersion,
+                    SigningStatus = PackageSigningStatus.Valid
+                };
+
+                var signature = new PackageSignature
+                {
+                    PackageKey = PackageKey,
+                    Status = PackageSignatureStatus.Unknown,
+                    Type = PackageSignatureType.Repository,
+                };
+
+                var timestamp = new TrustedTimestamp
+                {
+                    Value = DateTime.UtcNow.AddDays(-1)
+                };
+
+                var signingCertificate = new EndCertificate
+                {
+                    Status = EndCertificateStatus.Good,
+                    StatusUpdateTime = DateTime.UtcNow.AddSeconds(-1),
+                };
+
+                var timestampCertificate = new EndCertificate
+                {
+                    Status = EndCertificateStatus.Revoked,
+                    StatusUpdateTime = DateTime.UtcNow.AddSeconds(-1),
+                    RevocationTime = DateTime.UtcNow.AddSeconds(-1),
+                };
+
+                packageSigningState.PackageSignatures = new[] { signature };
+                signature.PackageSigningState = packageSigningState;
+                signature.EndCertificate = signingCertificate;
+                signature.TrustedTimestamps = new[] { timestamp };
+                timestamp.PackageSignature = signature;
+                timestamp.EndCertificate = timestampCertificate;
+
+                _validationContext.Mock(
+                    validatorStatuses: new[] { validatorStatus },
+                    packageSigningStates: new[] { packageSigningState },
+                    packageSignatures: new[] { signature },
+                    endCertificates: new[] { signature.EndCertificate, timestampCertificate });
+
+                // Act & Assert
+                var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => _target.GetResultAsync(_validationRequest.Object));
+
+                Assert.Equal($"Package with key {validatorStatus.PackageKey} does not have an author signature", ex.Message);
+            }
         }
 
         public class TheStartValidationAsyncMethod : FactsBase
@@ -614,6 +678,52 @@ public async Task UnsignedPackagesAlwaysSucceed()
                 Assert.Equal(ValidationStatus.Succeeded, validatorStatus.State);
             }
 
+            [Fact]
+            public async Task OnlyRepositorySignedPackagesAlwaysSucceed()
+            {
+                var validatorStatus = new ValidatorStatus
+                {
+                    ValidationId = ValidationId,
+                    PackageKey = PackageKey,
+                    ValidatorName = nameof(PackageCertificatesValidator),
+                    State = ValidationStatus.NotStarted,
+                    ValidatorIssues = new List<ValidatorIssue>(),
+                };
+
+                var packageSigningState = new PackageSigningState
+                {
+                    PackageKey = PackageKey,
+                    PackageId = PackageId,
+                    PackageNormalizedVersion = PackageNormalizedVersion,
+                    SigningStatus = PackageSigningStatus.Valid
+                };
+
+                var packageSignature = new PackageSignature
+                {
+                    PackageKey = PackageKey,
+                    Status = PackageSignatureStatus.Valid,
+                    Type = PackageSignatureType.Repository,
+                };
+
+                // Arrange
+                _validationContext.Mock(
+                    validatorStatuses: new[] { validatorStatus },
+                    packageSigningStates: new[] { packageSigningState },
+                    packageSignatures: new[] { packageSignature });
+
+                // Act & Assert
+                var actual = await _target.StartAsync(_validationRequest.Object);
+
+                _certificateVerifier.Verify(v => v.EnqueueVerificationAsync(It.IsAny<IValidationRequest>(), It.IsAny<EndCertificate>()), Times.Never);
+                _validationContext.Verify(c => c.SaveChangesAsync(), Times.Once);
+                _telemetryService.Verify(
+                    x => x.TrackDurationToStartPackageCertificatesValidator(It.IsAny<TimeSpan>()),
+                    Times.Never);
+
+                Assert.Equal(ValidationStatus.Succeeded, actual.Status);
+                Assert.Equal(ValidationStatus.Succeeded, validatorStatus.State);
+            }
+
             [Fact]
             public async Task ReturnsSucceededIfAllCertificatesAlreadyValidated()
             {