Skip to content
This repository was archived by the owner on Jul 30, 2024. It is now read-only.

Commit ed45b4e

Browse files
loic-sharmaloic-sharma
authored
[Process Signature] Strip valid repository signatures (#697)
Add a config to strip valid repository signatures. This will be used to re-reposign Microsoft packages. Part of https://github.com/NuGet/Engineering/issues/1964
1 parent 5c882bb commit ed45b4e

9 files changed

Lines changed: 263 additions & 8 deletions

File tree

‎src/Validation.PackageSigning.ProcessSignature/ProcessSignatureConfiguration.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ public class ProcessSignatureConfiguration
2121
/// </summary>
2222
public string V3ServiceIndexUrl { get; set; }
2323

24+
/// <summary>
25+
/// If true, revalidating a package will strip its repository signature and then apply a new repository signature,
26+
/// even if current repository signature is valid. This mode should be disabled unless absolutely necessary!
27+
/// </summary>
28+
public bool StripValidRepositorySignatures { get; set; }
29+
2430
/// <summary>
2531
/// Whether repository signatures should be persisted to the database. Disable this if repository signing
2632
/// is in test mode and repository signed packages are not published.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
"cf6ce6768ef858a3a667be1af8aa524d386c7f59a34542713f5dfb0d79acf3dd"
2424
],
2525
"V3ServiceIndexUrl": "https://apidev.nugettest.org/v3/index.json",
26+
"StripValidRepositorySignatures": #{Jobs.validation.packagesigning.processsignature.StripValidRepositorySignatures},
2627
"CommitRepositorySignatures": true
2728
},
2829

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
"cf6ce6768ef858a3a667be1af8aa524d386c7f59a34542713f5dfb0d79acf3dd"
2424
],
2525
"V3ServiceIndexUrl": "https://apiint.nugettest.org/v3/index.json",
26+
"StripValidRepositorySignatures": #{Jobs.validation.packagesigning.processsignature.StripValidRepositorySignatures},
2627
"CommitRepositorySignatures": true
2728
},
2829

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
"0e5f38f57dc1bcc806d8494f4f90fbcedd988b46760709cbeec6f4219aa6157d"
2424
],
2525
"V3ServiceIndexUrl": "https://api.nuget.org/v3/index.json",
26+
"StripValidRepositorySignatures": #{Jobs.validation.packagesigning.processsignature.StripValidRepositorySignatures},
2627
"CommitRepositorySignatures": true
2728
},
2829

‎src/Validation.PackageSigning.ProcessSignature/SignatureValidator.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,25 @@ private async Task<bool> IsValidRepositorySignatureAsync<T>(Context context, T s
495495
context.Message.PackageVersion,
496496
context.Message.ValidationId);
497497

498+
// If configured, strip valid repository signatures from packages to force a new repository signature to be applied.
499+
if (_configuration.Value.StripValidRepositorySignatures)
500+
{
501+
// Packages' signatures are validated twice: once before the package is repository signed, and once after. We do not
502+
// want to strip the newly applied repository signature, so we will only strip in the "before" case. We can detect the
503+
// "after" case as it requires the presence of a repository signature.
504+
if (!context.Message.RequireRepositorySignature)
505+
{
506+
_logger.LogWarning(
507+
$"The {nameof(ProcessSignatureConfiguration.StripValidRepositorySignatures)} configuration is enabled, " +
508+
"stripping the valid repository signature from package {PackageId} {PackageVersion} for validation {ValidationId}.",
509+
context.Message.PackageId,
510+
context.Message.PackageVersion,
511+
context.Message.ValidationId);
512+
513+
return false;
514+
}
515+
}
516+
498517
return true;
499518
}
500519

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,23 @@ public async Task SetPackageSigningState(
4040
}
4141

4242
// Update the signing state if it already exists, otherwise, create a new record.
43-
var signatureState = await _validationContext.PackageSigningStates.FirstOrDefaultAsync(s => s.PackageKey == packageKey);
43+
var signatureState = await _validationContext
44+
.PackageSigningStates
45+
.Include(s => s.PackageSignatures)
46+
.FirstOrDefaultAsync(s => s.PackageKey == packageKey);
4447

4548
if (signatureState != null)
4649
{
4750
signatureState.SigningStatus = status;
51+
52+
// Remove all stored signatures if the package is transitioning to an unsigned state.
53+
if (status == PackageSigningStatus.Unsigned)
54+
{
55+
foreach (var signature in signatureState.PackageSignatures.ToList())
56+
{
57+
_validationContext.PackageSignatures.Remove(signature);
58+
}
59+
}
4860
}
4961
else
5062
{

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace Validation.PackageSigning.Core.Tests.Support
1010
{
1111
public static class DbSetMockFactory
1212
{
13-
public static IDbSet<T> Create<T>(params T[] sourceList) where T : class
13+
public static Mock<IDbSet<T>> CreateMock<T>(params T[] sourceList) where T : class
1414
{
1515
var list = new List<T>(sourceList);
1616

@@ -22,7 +22,12 @@ public static IDbSet<T> Create<T>(params T[] sourceList) where T : class
2222
dbSet.Setup(m => m.Add(It.IsAny<T>())).Callback<T>(e => list.Add(e));
2323
dbSet.Setup(m => m.Remove(It.IsAny<T>())).Callback<T>(e => list.Remove(e));
2424

25-
return dbSet.Object;
25+
return dbSet;
26+
}
27+
28+
public static IDbSet<T> Create<T>(params T[] sourceList) where T : class
29+
{
30+
return CreateMock(sourceList).Object;
2631
}
2732
}
2833
}

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

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System.Linq;
5-
using System.Threading.Tasks;
64
using Microsoft.Extensions.Logging;
75
using Moq;
86
using NuGet.Jobs.Validation.PackageSigning.Storage;
97
using NuGet.Services.Validation;
8+
using System.Collections.Generic;
9+
using System.Linq;
10+
using System.Threading.Tasks;
1011
using Validation.PackageSigning.Core.Tests.Support;
1112
using Xunit;
1213
using Xunit.Abstractions;
@@ -15,11 +16,11 @@ namespace Validation.PackageSigning.ProcessSignature.Tests
1516
{
1617
public class PackageSigningStateServiceFacts
1718
{
18-
public class TheTrySetPackageSigningStateMethod
19+
public class TheSetPackageSigningStateMethod
1920
{
2021
private readonly ILoggerFactory _loggerFactory;
2122

22-
public TheTrySetPackageSigningStateMethod(ITestOutputHelper testOutput)
23+
public TheSetPackageSigningStateMethod(ITestOutputHelper testOutput)
2324
{
2425
_loggerFactory = new LoggerFactory();
2526
_loggerFactory.AddXunit(testOutput);
@@ -64,6 +65,57 @@ await packageSigningStateService.SetPackageSigningState(
6465
"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.");
6566
}
6667

68+
[Fact]
69+
public async Task DropsAllPackageSignaturesWhenPackageStateTransitionsToUnsigned()
70+
{
71+
// Arrange
72+
const int packageKey = 1;
73+
const string packageId = "packageId";
74+
const string packageVersion = "1.0.0";
75+
const PackageSigningStatus newStatus = PackageSigningStatus.Unsigned;
76+
77+
var signature1 = new PackageSignature();
78+
var signature2 = new PackageSignature();
79+
80+
var packageSigningState = new PackageSigningState
81+
{
82+
PackageId = packageId,
83+
PackageKey = packageKey,
84+
SigningStatus = PackageSigningStatus.Valid,
85+
PackageNormalizedVersion = packageVersion,
86+
87+
PackageSignatures = new List<PackageSignature> { signature1, signature2 }
88+
};
89+
90+
var logger = _loggerFactory.CreateLogger<PackageSigningStateService>();
91+
var packageSigningStatesDbSetMock = DbSetMockFactory.CreateMock(packageSigningState);
92+
var packageSignaturesDbSetMock = DbSetMockFactory.CreateMock(signature1, signature2);
93+
var validationContextMock = new Mock<IValidationEntitiesContext>(MockBehavior.Strict);
94+
validationContextMock.Setup(m => m.PackageSigningStates).Returns(packageSigningStatesDbSetMock.Object);
95+
validationContextMock.Setup(m => m.PackageSignatures).Returns(packageSignaturesDbSetMock.Object);
96+
97+
// Act
98+
var packageSigningStateService = new PackageSigningStateService(validationContextMock.Object, logger);
99+
100+
// Assert
101+
await packageSigningStateService.SetPackageSigningState(
102+
packageKey,
103+
packageId,
104+
packageVersion,
105+
status: newStatus);
106+
107+
// Assert
108+
Assert.Equal(newStatus, packageSigningState.SigningStatus);
109+
110+
packageSignaturesDbSetMock.Verify(m => m.Remove(signature1), Times.Once);
111+
packageSignaturesDbSetMock.Verify(m => m.Remove(signature2), Times.Once);
112+
113+
validationContextMock.Verify(
114+
m => m.SaveChangesAsync(),
115+
Times.Never,
116+
"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.");
117+
}
118+
67119
[Fact]
68120
public async Task AddsNewStateWhenSignatureStateIsNull()
69121
{

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

Lines changed: 159 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ public class ValidateAsync
3939
private readonly Mock<ISignatureFormatValidator> _formatValidator;
4040
private VerifySignaturesResult _minimalVerifyResult;
4141
private VerifySignaturesResult _fullVerifyResult;
42+
private VerifySignaturesResult _authorSignatureVerifyResult;
43+
private VerifySignaturesResult _repositorySignatureVerifyResult;
4244
private readonly Mock<ISignaturePartsExtractor> _signaturePartsExtractor;
4345
private readonly Mock<ICorePackageService> _corePackageService;
4446
private readonly ILogger<SignatureValidator> _logger;
@@ -73,6 +75,16 @@ public ValidateAsync(ITestOutputHelper output)
7375
.Setup(x => x.ValidateAllSignaturesAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<bool>(), It.IsAny<CancellationToken>()))
7476
.ReturnsAsync(() => _fullVerifyResult);
7577

78+
_authorSignatureVerifyResult = new VerifySignaturesResult(isValid: true, isSigned: true);
79+
_formatValidator
80+
.Setup(x => x.ValidateAuthorSignatureAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<CancellationToken>()))
81+
.ReturnsAsync(() => _authorSignatureVerifyResult);
82+
83+
_repositorySignatureVerifyResult = new VerifySignaturesResult(isValid: true, isSigned: true);
84+
_formatValidator
85+
.Setup(x => x.ValidateRepositorySignatureAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<CancellationToken>()))
86+
.ReturnsAsync(() => _repositorySignatureVerifyResult);
87+
7688
_signaturePartsExtractor = new Mock<ISignaturePartsExtractor>();
7789
_corePackageService = new Mock<ICorePackageService>();
7890
var loggerFactory = new LoggerFactory().AddXunit(output);
@@ -88,7 +100,8 @@ public ValidateAsync(ITestOutputHelper output)
88100
_configuration = new ProcessSignatureConfiguration
89101
{
90102
AllowedRepositorySigningCertificates = new List<string> { "fake-thumbprint" },
91-
V3ServiceIndexUrl = "http://example/v3/index.json",
103+
V3ServiceIndexUrl = TestResources.V3ServiceIndexUrl,
104+
StripValidRepositorySignatures = false,
92105
};
93106
_optionsSnapshot.Setup(x => x.Value).Returns(() => _configuration);
94107

@@ -490,6 +503,33 @@ public async Task AcceptsSignedPackagesWithUnknownCertificatesOnRevalidation()
490503
Assert.Empty(result.Issues);
491504
}
492505

506+
[Fact]
507+
public async Task AcceptsRepositorySignedPackage()
508+
{
509+
// Arrange
510+
_configuration.AllowedRepositorySigningCertificates = new List<string> { TestResources.Leaf1Thumbprint };
511+
_packageStream = TestResources.GetResourceStream(TestResources.RepoSignedPackageLeaf1);
512+
513+
TestUtility.RequireUnsignedPackage(_corePackageService, TestResources.RepoSignedPackageLeafId, TestResources.RepoSignedPackageLeaf1Version);
514+
515+
_message = new SignatureValidationMessage(
516+
TestResources.RepoSignedPackageLeafId,
517+
TestResources.RepoSignedPackageLeaf1Version,
518+
new Uri($"https://unit.test/{TestResources.RepoSignedPackageLeaf1.ToLowerInvariant()}"),
519+
Guid.NewGuid());
520+
521+
// Act
522+
var result = await _target.ValidateAsync(
523+
_packageKey,
524+
_packageStream,
525+
_message,
526+
_cancellationToken);
527+
528+
// Assert
529+
Validate(result, ValidationStatus.Succeeded, PackageSigningStatus.Valid);
530+
Assert.Empty(result.Issues);
531+
}
532+
493533
[Fact]
494534
public async Task WhenRepositorySigningIsRequired_FailsValidationOfSignedPackagesWithNoRepositorySignature()
495535
{
@@ -663,6 +703,124 @@ public async Task StripsAndAcceptsPackagesWithRepositorySignatures(
663703
}
664704
}
665705

706+
[Theory]
707+
[InlineData(
708+
TestResources.RepoSignedPackageLeaf1,
709+
TestResources.RepoSignedPackageLeafId,
710+
TestResources.RepoSignedPackageLeaf1Version,
711+
PackageSigningStatus.Unsigned)]
712+
[InlineData(
713+
TestResources.AuthorAndRepoSignedPackageLeaf1,
714+
TestResources.AuthorAndRepoSignedPackageLeafId,
715+
TestResources.AuthorAndRepoSignedPackageLeaf1Version,
716+
PackageSigningStatus.Valid)]
717+
public async Task WhenStripsValidRepositorySignature_StripsAndAcceptsRepositorySignatureWhenRepositorySignatureIsNotRequired(
718+
string resourceName,
719+
string packageId,
720+
string packageVersion,
721+
PackageSigningStatus expectedSigningStatus)
722+
{
723+
// Arrange
724+
_configuration.StripValidRepositorySignatures = true;
725+
_configuration.AllowedRepositorySigningCertificates = new List<string> { TestResources.Leaf1Thumbprint, TestResources.Leaf2Thumbprint };
726+
727+
_packageStream = TestResources.GetResourceStream(resourceName);
728+
729+
if (resourceName == TestResources.RepoSignedPackageLeaf1)
730+
{
731+
TestUtility.RequireUnsignedPackage(_corePackageService, TestResources.RepoSignedPackageLeafId, TestResources.RepoSignedPackageLeaf1Version);
732+
}
733+
734+
if (resourceName == TestResources.AuthorAndRepoSignedPackageLeaf1)
735+
{
736+
TestUtility.RequireSignedPackage(_corePackageService, TestResources.AuthorAndRepoSignedPackageLeafId, TestResources.AuthorAndRepoSignedPackageLeaf1Version, TestResources.Leaf1Thumbprint);
737+
}
738+
739+
_message = new SignatureValidationMessage(
740+
packageId,
741+
packageVersion,
742+
new Uri($"https://unit.test/{resourceName.ToLowerInvariant()}"),
743+
Guid.NewGuid(),
744+
requireRepositorySignature: false);
745+
746+
Stream uploadedStream = null;
747+
_packageFileService
748+
.Setup(x => x.SaveAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<Guid>(), It.IsAny<Stream>()))
749+
.Returns(Task.CompletedTask)
750+
.Callback<string, string, Guid, Stream>((_, __, ___, s) => uploadedStream = s);
751+
752+
// Act
753+
var result = await _target.ValidateAsync(
754+
_packageKey,
755+
_packageStream,
756+
_message,
757+
_cancellationToken);
758+
759+
// Assert
760+
Validate(result, ValidationStatus.Succeeded, expectedSigningStatus, _nupkgUri);
761+
Assert.Empty(result.Issues);
762+
_packageFileService.Verify(
763+
x => x.SaveAsync(_message.PackageId, _message.PackageVersion, _message.ValidationId, It.IsAny<Stream>()),
764+
Times.Once);
765+
_packageFileService.Verify(
766+
x => x.GetReadAndDeleteUriAsync(_message.PackageId, _message.PackageVersion, _message.ValidationId),
767+
Times.Once);
768+
Assert.IsType<FileStream>(uploadedStream);
769+
Assert.Throws<ObjectDisposedException>(() => uploadedStream.Length);
770+
}
771+
772+
[Theory]
773+
[InlineData(
774+
TestResources.RepoSignedPackageLeaf1,
775+
TestResources.RepoSignedPackageLeafId,
776+
TestResources.RepoSignedPackageLeaf1Version,
777+
PackageSigningStatus.Valid)]
778+
[InlineData(
779+
TestResources.AuthorAndRepoSignedPackageLeaf1,
780+
TestResources.AuthorAndRepoSignedPackageLeafId,
781+
TestResources.AuthorAndRepoSignedPackageLeaf1Version,
782+
PackageSigningStatus.Valid)]
783+
public async Task WhenStripsValidRepositorySignature_AcceptsRepositorySignatureWhenRepositorySignatureIsRequired(
784+
string resourceName,
785+
string packageId,
786+
string packageVersion,
787+
PackageSigningStatus expectedSigningStatus)
788+
{
789+
// Arrange
790+
_configuration.StripValidRepositorySignatures = true;
791+
_configuration.AllowedRepositorySigningCertificates = new List<string> { TestResources.Leaf1Thumbprint, TestResources.Leaf2Thumbprint };
792+
793+
_packageStream = TestResources.GetResourceStream(resourceName);
794+
795+
if (resourceName == TestResources.RepoSignedPackageLeaf1)
796+
{
797+
TestUtility.RequireUnsignedPackage(_corePackageService, TestResources.RepoSignedPackageLeafId, TestResources.RepoSignedPackageLeaf1Version);
798+
}
799+
800+
if (resourceName == TestResources.AuthorAndRepoSignedPackageLeaf1)
801+
{
802+
TestUtility.RequireSignedPackage(_corePackageService, TestResources.AuthorAndRepoSignedPackageLeafId, TestResources.AuthorAndRepoSignedPackageLeaf1Version, TestResources.Leaf1Thumbprint);
803+
}
804+
805+
_message = new SignatureValidationMessage(
806+
packageId,
807+
packageVersion,
808+
new Uri($"https://unit.test/{resourceName.ToLowerInvariant()}"),
809+
Guid.NewGuid(),
810+
requireRepositorySignature: true);
811+
812+
// Act
813+
var result = await _target.ValidateAsync(
814+
_packageKey,
815+
_packageStream,
816+
_message,
817+
_cancellationToken);
818+
819+
// Assert
820+
Validate(result, ValidationStatus.Succeeded, expectedSigningStatus);
821+
Assert.Empty(result.Issues);
822+
}
823+
666824
[Fact]
667825
public async Task StripsAndRejectsPackagesWithRepositorySignatureWhenPackageMustBeAuthorSigned()
668826
{

0 commit comments

Comments
 (0)