Merge pull request #700 from NuGet/dev

[ReleasePrep][2018.12.12]RI of dev into master

src/Validation.PackageSigning.ProcessSignature/ProcessSignatureConfiguration.cs

@@ -21,6 +21,12 @@ public class ProcessSignatureConfiguration
         /// </summary>
         public string V3ServiceIndexUrl { get; set; }
 
+        /// <summary>
+        /// If true, revalidating a package will strip its repository signature and then apply a new repository signature,
+        /// even if current repository signature is valid. This mode should be disabled unless absolutely necessary!
+        /// </summary>
+        public bool StripValidRepositorySignatures { get; set; }
+
         /// <summary>
         /// Whether repository signatures should be persisted to the database. Disable this if repository signing
         /// is in test mode and repository signed packages are not published.

src/Validation.PackageSigning.ProcessSignature/Settings/dev.json

@@ -23,6 +23,7 @@
       "cf6ce6768ef858a3a667be1af8aa524d386c7f59a34542713f5dfb0d79acf3dd"
     ],
     "V3ServiceIndexUrl": "https://apidev.nugettest.org/v3/index.json",
+    "StripValidRepositorySignatures": #{Jobs.validation.packagesigning.processsignature.StripValidRepositorySignatures},
     "CommitRepositorySignatures": true
   },
 

src/Validation.PackageSigning.ProcessSignature/Settings/int.json

@@ -23,6 +23,7 @@
       "cf6ce6768ef858a3a667be1af8aa524d386c7f59a34542713f5dfb0d79acf3dd"
     ],
     "V3ServiceIndexUrl": "https://apiint.nugettest.org/v3/index.json",
+    "StripValidRepositorySignatures": #{Jobs.validation.packagesigning.processsignature.StripValidRepositorySignatures},
     "CommitRepositorySignatures": true
   },
 

src/Validation.PackageSigning.ProcessSignature/Settings/prod.json

@@ -23,6 +23,7 @@
       "0e5f38f57dc1bcc806d8494f4f90fbcedd988b46760709cbeec6f4219aa6157d"
     ],
     "V3ServiceIndexUrl": "https://api.nuget.org/v3/index.json",
+    "StripValidRepositorySignatures": #{Jobs.validation.packagesigning.processsignature.StripValidRepositorySignatures},
     "CommitRepositorySignatures": true
   },
 

src/Validation.PackageSigning.ProcessSignature/SignatureValidator.cs

@@ -495,6 +495,25 @@ private async Task<bool> IsValidRepositorySignatureAsync<T>(Context context, T s
                 context.Message.PackageVersion,
                 context.Message.ValidationId);
 
+            // If configured, strip valid repository signatures from packages to force a new repository signature to be applied.
+            if (_configuration.Value.StripValidRepositorySignatures)
+            {
+                // Packages' signatures are validated twice: once before the package is repository signed, and once after. We do not
+                // want to strip the newly applied repository signature, so we will only strip in the "before" case. We can detect the
+                // "after" case as it requires the presence of a repository signature.
+                if (!context.Message.RequireRepositorySignature)
+                {
+                    _logger.LogWarning(
+                        $"The {nameof(ProcessSignatureConfiguration.StripValidRepositorySignatures)} configuration is enabled, " +
+                        "stripping the valid repository signature from package {PackageId} {PackageVersion} for validation {ValidationId}.",
+                        context.Message.PackageId,
+                        context.Message.PackageVersion,
+                        context.Message.ValidationId);
+
+                    return false;
+                }
+            }
+
             return true;
         }
 

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

@@ -40,11 +40,23 @@ public async Task SetPackageSigningState(
             }
 
             // Update the signing state if it already exists, otherwise, create a new record.
-            var signatureState = await _validationContext.PackageSigningStates.FirstOrDefaultAsync(s => s.PackageKey == packageKey);
+            var signatureState = await _validationContext
+                .PackageSigningStates
+                .Include(s => s.PackageSignatures)
+                .FirstOrDefaultAsync(s => s.PackageKey == packageKey);
 
             if (signatureState != null)
             {
                 signatureState.SigningStatus = status;
+
+                // Remove all stored signatures if the package is transitioning to an unsigned state.
+                if (status == PackageSigningStatus.Unsigned)
+                {
+                    foreach (var signature in signatureState.PackageSignatures.ToList())
+                    {
+                        _validationContext.PackageSignatures.Remove(signature);
+                    }
+                }
             }
             else
             {

tests/Validation.PackageSigning.Core.Tests/Support/DbSetMockFactory.cs

@@ -10,7 +10,7 @@ namespace Validation.PackageSigning.Core.Tests.Support
 {
     public static class DbSetMockFactory
     {
-        public static IDbSet<T> Create<T>(params T[] sourceList) where T : class
+        public static Mock<IDbSet<T>> CreateMock<T>(params T[] sourceList) where T : class
         {
             var list = new List<T>(sourceList);
 
@@ -22,7 +22,12 @@ public static IDbSet<T> Create<T>(params T[] sourceList) where T : class
             dbSet.Setup(m => m.Add(It.IsAny<T>())).Callback<T>(e => list.Add(e));
             dbSet.Setup(m => m.Remove(It.IsAny<T>())).Callback<T>(e => list.Remove(e));
 
-            return dbSet.Object;
+            return dbSet;
+        }
+
+        public static IDbSet<T> Create<T>(params T[] sourceList) where T : class
+        {
+            return CreateMock(sourceList).Object;
         }
     }
 }

tests/Validation.PackageSigning.ProcessSignature.Tests/PackageSigningStateServiceFacts.cs

@@ -1,12 +1,13 @@
 // 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.Linq;
-using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using Moq;
 using NuGet.Jobs.Validation.PackageSigning.Storage;
 using NuGet.Services.Validation;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
 using Validation.PackageSigning.Core.Tests.Support;
 using Xunit;
 using Xunit.Abstractions;
@@ -15,11 +16,11 @@ namespace Validation.PackageSigning.ProcessSignature.Tests
 {
     public class PackageSigningStateServiceFacts
     {
-        public class TheTrySetPackageSigningStateMethod
+        public class TheSetPackageSigningStateMethod
         {
             private readonly ILoggerFactory _loggerFactory;
 
-            public TheTrySetPackageSigningStateMethod(ITestOutputHelper testOutput)
+            public TheSetPackageSigningStateMethod(ITestOutputHelper testOutput)
             {
                 _loggerFactory = new LoggerFactory();
                 _loggerFactory.AddXunit(testOutput);
@@ -64,6 +65,57 @@ await packageSigningStateService.SetPackageSigningState(
                     "Saving the context here is incorrect as updating the validator's status also saves the context. Doing so would cause both queries not to be executed in the same transaction.");
             }
 
+            [Fact]
+            public async Task DropsAllPackageSignaturesWhenPackageStateTransitionsToUnsigned()
+            {
+                // Arrange
+                const int packageKey = 1;
+                const string packageId = "packageId";
+                const string packageVersion = "1.0.0";
+                const PackageSigningStatus newStatus = PackageSigningStatus.Unsigned;
+
+                var signature1 = new PackageSignature();
+                var signature2 = new PackageSignature();
+
+                var packageSigningState = new PackageSigningState
+                {
+                    PackageId = packageId,
+                    PackageKey = packageKey,
+                    SigningStatus = PackageSigningStatus.Valid,
+                    PackageNormalizedVersion = packageVersion,
+
+                    PackageSignatures = new List<PackageSignature> { signature1, signature2 }
+                };
+
+                var logger = _loggerFactory.CreateLogger<PackageSigningStateService>();
+                var packageSigningStatesDbSetMock = DbSetMockFactory.CreateMock(packageSigningState);
+                var packageSignaturesDbSetMock = DbSetMockFactory.CreateMock(signature1, signature2);
+                var validationContextMock = new Mock<IValidationEntitiesContext>(MockBehavior.Strict);
+                validationContextMock.Setup(m => m.PackageSigningStates).Returns(packageSigningStatesDbSetMock.Object);
+                validationContextMock.Setup(m => m.PackageSignatures).Returns(packageSignaturesDbSetMock.Object);
+
+                // Act
+                var packageSigningStateService = new PackageSigningStateService(validationContextMock.Object, logger);
+
+                // Assert
+                await packageSigningStateService.SetPackageSigningState(
+                    packageKey,
+                    packageId,
+                    packageVersion,
+                    status: newStatus);
+
+                // Assert
+                Assert.Equal(newStatus, packageSigningState.SigningStatus);
+
+                packageSignaturesDbSetMock.Verify(m => m.Remove(signature1), Times.Once);
+                packageSignaturesDbSetMock.Verify(m => m.Remove(signature2), Times.Once);
+
+                validationContextMock.Verify(
+                    m => m.SaveChangesAsync(),
+                    Times.Never,
+                    "Saving the context here is incorrect as updating the validator's status also saves the context. Doing so would cause both queries not to be executed in the same transaction.");
+            }
+
             [Fact]
             public async Task AddsNewStateWhenSignatureStateIsNull()
             {

tests/Validation.PackageSigning.ProcessSignature.Tests/SignatureValidatorFacts.cs

@@ -39,6 +39,8 @@ public class ValidateAsync
             private readonly Mock<ISignatureFormatValidator> _formatValidator;
             private VerifySignaturesResult _minimalVerifyResult;
             private VerifySignaturesResult _fullVerifyResult;
+            private VerifySignaturesResult _authorSignatureVerifyResult;
+            private VerifySignaturesResult _repositorySignatureVerifyResult;
             private readonly Mock<ISignaturePartsExtractor> _signaturePartsExtractor;
             private readonly Mock<ICorePackageService> _corePackageService;
             private readonly ILogger<SignatureValidator> _logger;
@@ -73,6 +75,16 @@ public ValidateAsync(ITestOutputHelper output)
                     .Setup(x => x.ValidateAllSignaturesAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
                     .ReturnsAsync(() => _fullVerifyResult);
 
+                _authorSignatureVerifyResult = new VerifySignaturesResult(isValid: true, isSigned: true);
+                _formatValidator
+                    .Setup(x => x.ValidateAuthorSignatureAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<CancellationToken>()))
+                    .ReturnsAsync(() => _authorSignatureVerifyResult);
+
+                _repositorySignatureVerifyResult = new VerifySignaturesResult(isValid: true, isSigned: true);
+                _formatValidator
+                    .Setup(x => x.ValidateRepositorySignatureAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<CancellationToken>()))
+                    .ReturnsAsync(() => _repositorySignatureVerifyResult);
+
                 _signaturePartsExtractor = new Mock<ISignaturePartsExtractor>();
                 _corePackageService = new Mock<ICorePackageService>();
                 var loggerFactory = new LoggerFactory().AddXunit(output);
@@ -88,7 +100,8 @@ public ValidateAsync(ITestOutputHelper output)
                 _configuration = new ProcessSignatureConfiguration
                 {
                     AllowedRepositorySigningCertificates = new List<string> { "fake-thumbprint" },
-                    V3ServiceIndexUrl = "http://example/v3/index.json",
+                    V3ServiceIndexUrl = TestResources.V3ServiceIndexUrl,
+                    StripValidRepositorySignatures = false,
                 };
                 _optionsSnapshot.Setup(x => x.Value).Returns(() => _configuration);
 
@@ -490,6 +503,33 @@ public async Task AcceptsSignedPackagesWithUnknownCertificatesOnRevalidation()
                 Assert.Empty(result.Issues);
             }
 
+            [Fact]
+            public async Task AcceptsRepositorySignedPackage()
+            {
+                // Arrange
+                _configuration.AllowedRepositorySigningCertificates = new List<string> { TestResources.Leaf1Thumbprint };
+                _packageStream = TestResources.GetResourceStream(TestResources.RepoSignedPackageLeaf1);
+
+                TestUtility.RequireUnsignedPackage(_corePackageService, TestResources.RepoSignedPackageLeafId, TestResources.RepoSignedPackageLeaf1Version);
+
+                _message = new SignatureValidationMessage(
+                    TestResources.RepoSignedPackageLeafId,
+                    TestResources.RepoSignedPackageLeaf1Version,
+                    new Uri($"https://unit.test/{TestResources.RepoSignedPackageLeaf1.ToLowerInvariant()}"),
+                    Guid.NewGuid());
+
+                // Act
+                var result = await _target.ValidateAsync(
+                    _packageKey,
+                    _packageStream,
+                    _message,
+                    _cancellationToken);
+
+                // Assert
+                Validate(result, ValidationStatus.Succeeded, PackageSigningStatus.Valid);
+                Assert.Empty(result.Issues);
+            }
+
             [Fact]
             public async Task WhenRepositorySigningIsRequired_FailsValidationOfSignedPackagesWithNoRepositorySignature()
             {
@@ -663,6 +703,124 @@ public async Task StripsAndAcceptsPackagesWithRepositorySignatures(
                 }
             }
 
+            [Theory]
+            [InlineData(
+                TestResources.RepoSignedPackageLeaf1,
+                TestResources.RepoSignedPackageLeafId,
+                TestResources.RepoSignedPackageLeaf1Version,
+                PackageSigningStatus.Unsigned)]
+            [InlineData(
+                TestResources.AuthorAndRepoSignedPackageLeaf1,
+                TestResources.AuthorAndRepoSignedPackageLeafId,
+                TestResources.AuthorAndRepoSignedPackageLeaf1Version,
+                PackageSigningStatus.Valid)]
+            public async Task WhenStripsValidRepositorySignature_StripsAndAcceptsRepositorySignatureWhenRepositorySignatureIsNotRequired(
+                string resourceName,
+                string packageId,
+                string packageVersion,
+                PackageSigningStatus expectedSigningStatus)
+            {
+                // Arrange
+                _configuration.StripValidRepositorySignatures = true;
+                _configuration.AllowedRepositorySigningCertificates = new List<string> { TestResources.Leaf1Thumbprint, TestResources.Leaf2Thumbprint };
+
+                _packageStream = TestResources.GetResourceStream(resourceName);
+
+                if (resourceName == TestResources.RepoSignedPackageLeaf1)
+                {
+                    TestUtility.RequireUnsignedPackage(_corePackageService, TestResources.RepoSignedPackageLeafId, TestResources.RepoSignedPackageLeaf1Version);
+                }
+
+                if (resourceName == TestResources.AuthorAndRepoSignedPackageLeaf1)
+                {
+                    TestUtility.RequireSignedPackage(_corePackageService, TestResources.AuthorAndRepoSignedPackageLeafId, TestResources.AuthorAndRepoSignedPackageLeaf1Version, TestResources.Leaf1Thumbprint);
+                }
+
+                _message = new SignatureValidationMessage(
+                    packageId,
+                    packageVersion,
+                    new Uri($"https://unit.test/{resourceName.ToLowerInvariant()}"),
+                    Guid.NewGuid(),
+                    requireRepositorySignature: false);
+
+                Stream uploadedStream = null;
+                _packageFileService
+                    .Setup(x => x.SaveAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<Guid>(), It.IsAny<Stream>()))
+                    .Returns(Task.CompletedTask)
+                    .Callback<string, string, Guid, Stream>((_, __, ___, s) => uploadedStream = s);
+
+                // Act
+                var result = await _target.ValidateAsync(
+                    _packageKey,
+                    _packageStream,
+                    _message,
+                    _cancellationToken);
+
+                // Assert
+                Validate(result, ValidationStatus.Succeeded, expectedSigningStatus, _nupkgUri);
+                Assert.Empty(result.Issues);
+                _packageFileService.Verify(
+                    x => x.SaveAsync(_message.PackageId, _message.PackageVersion, _message.ValidationId, It.IsAny<Stream>()),
+                    Times.Once);
+                _packageFileService.Verify(
+                    x => x.GetReadAndDeleteUriAsync(_message.PackageId, _message.PackageVersion, _message.ValidationId),
+                    Times.Once);
+                Assert.IsType<FileStream>(uploadedStream);
+                Assert.Throws<ObjectDisposedException>(() => uploadedStream.Length);
+            }
+
+            [Theory]
+            [InlineData(
+                TestResources.RepoSignedPackageLeaf1,
+                TestResources.RepoSignedPackageLeafId,
+                TestResources.RepoSignedPackageLeaf1Version,
+                PackageSigningStatus.Valid)]
+            [InlineData(
+                TestResources.AuthorAndRepoSignedPackageLeaf1,
+                TestResources.AuthorAndRepoSignedPackageLeafId,
+                TestResources.AuthorAndRepoSignedPackageLeaf1Version,
+                PackageSigningStatus.Valid)]
+            public async Task WhenStripsValidRepositorySignature_AcceptsRepositorySignatureWhenRepositorySignatureIsRequired(
+                string resourceName,
+                string packageId,
+                string packageVersion,
+                PackageSigningStatus expectedSigningStatus)
+            {
+                // Arrange
+                _configuration.StripValidRepositorySignatures = true;
+                _configuration.AllowedRepositorySigningCertificates = new List<string> { TestResources.Leaf1Thumbprint, TestResources.Leaf2Thumbprint };
+
+                _packageStream = TestResources.GetResourceStream(resourceName);
+
+                if (resourceName == TestResources.RepoSignedPackageLeaf1)
+                {
+                    TestUtility.RequireUnsignedPackage(_corePackageService, TestResources.RepoSignedPackageLeafId, TestResources.RepoSignedPackageLeaf1Version);
+                }
+
+                if (resourceName == TestResources.AuthorAndRepoSignedPackageLeaf1)
+                {
+                    TestUtility.RequireSignedPackage(_corePackageService, TestResources.AuthorAndRepoSignedPackageLeafId, TestResources.AuthorAndRepoSignedPackageLeaf1Version, TestResources.Leaf1Thumbprint);
+                }
+
+                _message = new SignatureValidationMessage(
+                    packageId,
+                    packageVersion,
+                    new Uri($"https://unit.test/{resourceName.ToLowerInvariant()}"),
+                    Guid.NewGuid(),
+                    requireRepositorySignature: true);
+
+                // Act
+                var result = await _target.ValidateAsync(
+                    _packageKey,
+                    _packageStream,
+                    _message,
+                    _cancellationToken);
+
+                // Assert
+                Validate(result, ValidationStatus.Succeeded, expectedSigningStatus);
+                Assert.Empty(result.Issues);
+            }
+
             [Fact]
             public async Task StripsAndRejectsPackagesWithRepositorySignatureWhenPackageMustBeAuthorSigned()
             {