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

Commit 232a7b0

Browse files
joelverhagenjoelverhagen
committed
Use IPackageSignatureVerifier to verify the format of the signature file before reading the signature (#306)
Progress on NuGet/Engineering#785
1 parent 303dea8 commit 232a7b0

10 files changed

Lines changed: 454 additions & 36 deletions

‎src/Validation.PackageSigning.ExtractAndValidateSignature/Job.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,15 @@ private void ConfigureJobServices(IServiceCollection services, IConfigurationRoo
184184
services.AddTransient<IBrokeredMessageSerializer<SignatureValidationMessage>, SignatureValidationMessageSerializer>();
185185
services.AddTransient<IMessageHandler<SignatureValidationMessage>, SignatureValidationMessageHandler>();
186186
services.AddTransient<IPackageSigningStateService, PackageSigningStateService>();
187-
services.AddTransient<ISignatureValidator, SignatureValidator>();
188187
services.AddTransient<ISignaturePartsExtractor, SignaturePartsExtractor>();
189188

190-
services.AddTransient(p => PackageSignatureVerifierFactory.Create());
189+
services.AddTransient<ISignatureValidator, SignatureValidator>(p => new SignatureValidator(
190+
p.GetRequiredService<IPackageSigningStateService>(),
191+
PackageSignatureVerifierFactory.CreateMinimal(),
192+
PackageSignatureVerifierFactory.CreateFull(),
193+
p.GetRequiredService<ISignaturePartsExtractor>(),
194+
p.GetRequiredService<IEntityRepository<Certificate>>(),
195+
p.GetRequiredService<ILogger<SignatureValidator>>()));
191196

192197
services.AddSingleton(p =>
193198
{

‎src/Validation.PackageSigning.ExtractAndValidateSignature/MinimalSignatureVerificationProvider.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System.Linq;
5+
using System.Threading;
6+
using System.Threading.Tasks;
7+
using NuGet.Packaging.Signing;
8+
9+
namespace NuGet.Jobs.Validation.PackageSigning.ExtractAndValidateSignature
10+
{
11+
/// <summary>
12+
/// A signature verification provider which performs no verification at all. It allows amount of verification to be
13+
/// done by <see cref="IPackageSignatureVerifier"/> before performing more in-depth analysis.
14+
/// </summary>
15+
public class MinimalSignatureVerificationProvider : ISignatureVerificationProvider
16+
{
17+
public Task<PackageVerificationResult> GetTrustResultAsync(
18+
ISignedPackageReader package,
19+
Signature signature,
20+
SignedPackageVerifierSettings settings,
21+
CancellationToken token)
22+
{
23+
var result = new SignedPackageVerificationResult(
24+
SignatureVerificationStatus.Trusted,
25+
signature,
26+
Enumerable.Empty<SignatureLog>());
27+
28+
return Task.FromResult<PackageVerificationResult>(result);
29+
}
30+
}
31+
}

‎src/Validation.PackageSigning.ExtractAndValidateSignature/PackageSignatureVerifierFactory.cs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,33 @@ namespace NuGet.Jobs.Validation.PackageSigning.ExtractAndValidateSignature
1010
/// </summary>
1111
public static class PackageSignatureVerifierFactory
1212
{
13-
public static IPackageSignatureVerifier Create()
13+
/// <summary>
14+
/// Initializes a verifier that only verifies the format of the signature. No integrity or trust checks are
15+
/// performed.
16+
/// </summary>
17+
public static IPackageSignatureVerifier CreateMinimal()
18+
{
19+
var verificationProviders = new[]
20+
{
21+
new MinimalSignatureVerificationProvider(),
22+
};
23+
24+
var settings = new SignedPackageVerifierSettings(
25+
allowUnsigned: true,
26+
allowUntrusted: false, // Invalid format of the signature uses this flag to determine success.
27+
allowIgnoreTimestamp: true,
28+
failWithMultipleTimestamps: false,
29+
allowNoTimestamp: true);
30+
31+
return new PackageSignatureVerifier(
32+
verificationProviders,
33+
settings);
34+
}
35+
36+
/// <summary>
37+
/// Initializes a verifier that performs all integrity and trust checks required by the server.
38+
/// </summary>
39+
public static IPackageSignatureVerifier CreateFull()
1440
{
1541
var verificationProviders = new[]
1642
{

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

Lines changed: 70 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,23 @@ namespace NuGet.Jobs.Validation.PackageSigning.ExtractAndValidateSignature
1919
public class SignatureValidator : ISignatureValidator
2020
{
2121
private readonly IPackageSigningStateService _packageSigningStateService;
22-
private readonly IPackageSignatureVerifier _packageSignatureVerifier;
22+
private readonly IPackageSignatureVerifier _minimalPackageSignatureVerifier;
23+
private readonly IPackageSignatureVerifier _fullPackageSignatureVerifier;
2324
private readonly ISignaturePartsExtractor _signaturePartsExtractor;
2425
private readonly IEntityRepository<Certificate> _certificates;
2526
private readonly ILogger<SignatureValidator> _logger;
2627

2728
public SignatureValidator(
2829
IPackageSigningStateService packageSigningStateService,
29-
IPackageSignatureVerifier packageSignatureVerifier,
30+
IPackageSignatureVerifier minimalPackageSignatureVerifier,
31+
IPackageSignatureVerifier fullPackageSignatureVerifier,
3032
ISignaturePartsExtractor signaturePartsExtractor,
3133
IEntityRepository<Certificate> certificates,
3234
ILogger<SignatureValidator> logger)
3335
{
3436
_packageSigningStateService = packageSigningStateService ?? throw new ArgumentNullException(nameof(packageSigningStateService));
35-
_packageSignatureVerifier = packageSignatureVerifier ?? throw new ArgumentNullException(nameof(packageSignatureVerifier));
37+
_minimalPackageSignatureVerifier = minimalPackageSignatureVerifier ?? throw new ArgumentNullException(nameof(minimalPackageSignatureVerifier));
38+
_fullPackageSignatureVerifier = fullPackageSignatureVerifier ?? throw new ArgumentNullException(nameof(fullPackageSignatureVerifier));
3639
_signaturePartsExtractor = signaturePartsExtractor ?? throw new ArgumentNullException(nameof(signaturePartsExtractor));
3740
_certificates = certificates ?? throw new ArgumentNullException(nameof(certificates));
3841
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
@@ -77,7 +80,20 @@ private async Task<SignatureValidatorResult> HandleSignedPackageAsync(
7780
SignatureValidationMessage message,
7881
CancellationToken cancellationToken)
7982
{
80-
// Block packages that don't have exactly one signature.
83+
// First, detect format errors with a minimal verification. This doesn't even check package integrity. The
84+
// minimal verification is expected to swallow any sort of signature format exception.
85+
var invalidFormatResult = await GetVerifyResult(
86+
_minimalPackageSignatureVerifier,
87+
packageKey,
88+
signedPackageReader,
89+
message,
90+
cancellationToken);
91+
if (invalidFormatResult != null)
92+
{
93+
return invalidFormatResult;
94+
}
95+
96+
// We now know we can safely read the signature.
8197
var packageSignature = await signedPackageReader.GetSignatureAsync(cancellationToken);
8298

8399
// Block packages with any unknown signing certificates.
@@ -89,7 +105,7 @@ private async Task<SignatureValidatorResult> HandleSignedPackageAsync(
89105
.GetAll()
90106
.Where(c => packageThumbprint == c.Thumbprint)
91107
.Any();
92-
108+
93109
if (!isKnownCertificate)
94110
{
95111
_logger.LogInformation(
@@ -105,8 +121,55 @@ private async Task<SignatureValidatorResult> HandleSignedPackageAsync(
105121
ValidationIssue.PackageIsSigned);
106122
}
107123

124+
if (packageSignature.Type != SignatureType.Author)
125+
{
126+
_logger.LogInformation(
127+
"Signed package {PackageId} {PackageVersion} is blocked for validation {ValidationId} since it has a non-author signature: {SignatureType}",
128+
message.PackageId,
129+
message.PackageVersion,
130+
message.ValidationId,
131+
packageSignature.Type);
132+
133+
return await RejectAsync(
134+
packageKey,
135+
message,
136+
ValidationIssue.OnlyAuthorSignaturesSupported);
137+
}
138+
108139
// Call the "verify" API, which does the main logic of signature validation.
109-
var verifyResult = await _packageSignatureVerifier.VerifySignaturesAsync(
140+
var failureResult = await GetVerifyResult(
141+
_fullPackageSignatureVerifier,
142+
packageKey,
143+
signedPackageReader,
144+
message,
145+
cancellationToken);
146+
if (failureResult != null)
147+
{
148+
return failureResult;
149+
}
150+
151+
_logger.LogInformation(
152+
"Signed package {PackageId} {PackageVersion} for validation {ValidationId} is valid with certificate thumbprint: {PackageThumbprint}",
153+
message.PackageId,
154+
message.PackageVersion,
155+
message.ValidationId,
156+
packageThumbprint);
157+
158+
// Extract all of the signature artifacts and persist them.
159+
await _signaturePartsExtractor.ExtractAsync(signedPackageReader, cancellationToken);
160+
161+
// Mark this package as signed.
162+
return await AcceptAsync(packageKey, message, PackageSigningStatus.Valid);
163+
}
164+
165+
private async Task<SignatureValidatorResult> GetVerifyResult(
166+
IPackageSignatureVerifier verifier,
167+
int packageKey,
168+
ISignedPackageReader signedPackageReader,
169+
SignatureValidationMessage message,
170+
CancellationToken cancellationToken)
171+
{
172+
var verifyResult = await verifier.VerifySignaturesAsync(
110173
signedPackageReader,
111174
cancellationToken);
112175
if (!verifyResult.Valid)
@@ -140,19 +203,8 @@ private async Task<SignatureValidatorResult> HandleSignedPackageAsync(
140203
.Select(x => new ClientSigningVerificationFailure(x.Code.ToString(), x.Message))
141204
.ToArray());
142205
}
143-
144-
_logger.LogInformation(
145-
"Signed package {PackageId} {PackageVersion} for validation {ValidationId} is valid with certificate thumbprint: {PackageThumbprint}",
146-
message.PackageId,
147-
message.PackageVersion,
148-
message.ValidationId,
149-
packageThumbprint);
150-
151-
// Extract all of the signature artifacts and persist them.
152-
await _signaturePartsExtractor.ExtractAsync(signedPackageReader, cancellationToken);
153206

154-
// Mark this package as signed.
155-
return await AcceptAsync(packageKey, message, PackageSigningStatus.Valid);
207+
return null;
156208
}
157209

158210
private async Task<SignatureValidatorResult> RejectAsync(

‎src/Validation.PackageSigning.ExtractAndValidateSignature/Validation.PackageSigning.ExtractAndValidateSignature.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
<Compile Include="HashedCertificate.cs" />
4646
<Compile Include="ISignaturePartsExtractor.cs" />
4747
<Compile Include="ISignatureValidator.cs" />
48+
<Compile Include="MinimalSignatureVerificationProvider.cs" />
4849
<Compile Include="PackageSignatureVerifierFactory.cs" />
4950
<Compile Include="SignaturePartsExtractor.cs" />
5051
<Compile Include="SignatureValidator.cs" />

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

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,10 @@ public class ValidateAsync
2929
private readonly SignatureValidationMessage _message;
3030
private readonly CancellationToken _cancellationToken;
3131
private readonly Mock<IPackageSigningStateService> _packageSigningStateService;
32-
private VerifySignaturesResult _verifyResult;
33-
private readonly Mock<IPackageSignatureVerifier> _packageSignatureVerifier;
32+
private VerifySignaturesResult _mimialVerifyResult;
33+
private readonly Mock<IPackageSignatureVerifier> _mimimalPackageSignatureVerifier;
34+
private VerifySignaturesResult _fullVerifyResult;
35+
private readonly Mock<IPackageSignatureVerifier> _fullPackageSignatureVerifier;
3436
private readonly Mock<ISignaturePartsExtractor> _signaturePartsExtractor;
3537
private readonly Mock<IEntityRepository<Certificate>> _certificates;
3638
private readonly Mock<ILogger<SignatureValidator>> _logger;
@@ -53,11 +55,17 @@ public ValidateAsync()
5355

5456
_packageSigningStateService = new Mock<IPackageSigningStateService>();
5557

56-
_verifyResult = new VerifySignaturesResult(true);
57-
_packageSignatureVerifier = new Mock<IPackageSignatureVerifier>();
58-
_packageSignatureVerifier
58+
_mimialVerifyResult = new VerifySignaturesResult(true);
59+
_mimimalPackageSignatureVerifier = new Mock<IPackageSignatureVerifier>();
60+
_mimimalPackageSignatureVerifier
5961
.Setup(x => x.VerifySignaturesAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<CancellationToken>()))
60-
.ReturnsAsync(() => _verifyResult);
62+
.ReturnsAsync(() => _mimialVerifyResult);
63+
64+
_fullVerifyResult = new VerifySignaturesResult(true);
65+
_fullPackageSignatureVerifier = new Mock<IPackageSignatureVerifier>();
66+
_fullPackageSignatureVerifier
67+
.Setup(x => x.VerifySignaturesAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<CancellationToken>()))
68+
.ReturnsAsync(() => _fullVerifyResult);
6169

6270
_signaturePartsExtractor = new Mock<ISignaturePartsExtractor>();
6371
_certificates = new Mock<IEntityRepository<Certificate>>();
@@ -69,7 +77,8 @@ public ValidateAsync()
6977

7078
_target = new SignatureValidator(
7179
_packageSigningStateService.Object,
72-
_packageSignatureVerifier.Object,
80+
_mimimalPackageSignatureVerifier.Object,
81+
_fullPackageSignatureVerifier.Object,
7382
_signaturePartsExtractor.Object,
7483
_certificates.Object,
7584
_logger.Object);
@@ -155,14 +164,77 @@ await ConfigureKnownSignedPackage(
155164
}
156165

157166
[Fact]
158-
public async Task RejectsSignedPackagesWithKnownCertificatesButFailedVerifyResult()
167+
public async Task RejectsSignedPackagesWithFailedMinimalVerifyResult()
168+
{
169+
// Arrange
170+
await ConfigureKnownSignedPackage(
171+
TestResources.SignedPackageLeaf1Reader,
172+
TestResources.Leaf1Thumbprint);
173+
174+
_mimialVerifyResult = new VerifySignaturesResult(valid: false);
175+
176+
// Act
177+
var result = await _target.ValidateAsync(
178+
_packageKey,
179+
_packageMock.Object,
180+
_message,
181+
_cancellationToken);
182+
183+
// Assert
184+
Validate(result, ValidationStatus.Failed, PackageSigningStatus.Invalid);
185+
Assert.Empty(result.Issues);
186+
_fullPackageSignatureVerifier.Verify(
187+
x => x.VerifySignaturesAsync(It.IsAny<ISignedPackageReader>(), It.IsAny<CancellationToken>()),
188+
Times.Never);
189+
}
190+
191+
[Fact]
192+
public async Task RejectsPackagesWithMimimalVerificationErrors()
193+
{
194+
// Arrange
195+
await ConfigureKnownSignedPackage(
196+
TestResources.SignedPackageLeaf1Reader,
197+
TestResources.Leaf1Thumbprint);
198+
199+
_mimialVerifyResult = new VerifySignaturesResult(
200+
valid: false,
201+
results: new[]
202+
{
203+
new InvalidSignaturePackageVerificationResult(
204+
SignatureVerificationStatus.Invalid,
205+
new[]
206+
{
207+
SignatureLog.Issue(
208+
fatal: true,
209+
code: NuGetLogCode.NU3000,
210+
message: "The package signature is invalid."),
211+
})
212+
});
213+
214+
// Act
215+
var result = await _target.ValidateAsync(
216+
_packageKey,
217+
_packageMock.Object,
218+
_message,
219+
_cancellationToken);
220+
221+
// Assert
222+
Validate(result, ValidationStatus.Failed, PackageSigningStatus.Invalid);
223+
Assert.Single(result.Issues);
224+
var issue = Assert.IsType<ClientSigningVerificationFailure>(result.Issues[0]);
225+
Assert.Equal("NU3000", issue.ClientCode);
226+
Assert.Equal("The package signature is invalid.", issue.ClientMessage);
227+
}
228+
229+
[Fact]
230+
public async Task RejectsSignedPackagesWithKnownCertificatesButFailedFullVerifyResult()
159231
{
160232
// Arrange
161233
await ConfigureKnownSignedPackage(
162234
TestResources.SignedPackageLeaf1Reader,
163235
TestResources.Leaf1Thumbprint);
164236

165-
_verifyResult = new VerifySignaturesResult(valid: false);
237+
_fullVerifyResult = new VerifySignaturesResult(valid: false);
166238

167239
// Act
168240
var result = await _target.ValidateAsync(
@@ -177,14 +249,14 @@ await ConfigureKnownSignedPackage(
177249
}
178250

179251
[Fact]
180-
public async Task RejectsPackagesWithVerificationErrors()
252+
public async Task RejectsPackagesWithFullVerificationErrors()
181253
{
182254
// Arrange
183255
await ConfigureKnownSignedPackage(
184256
TestResources.SignedPackageLeaf1Reader,
185257
TestResources.Leaf1Thumbprint);
186258

187-
_verifyResult = new VerifySignaturesResult(
259+
_fullVerifyResult = new VerifySignaturesResult(
188260
valid: false,
189261
results: new[]
190262
{

0 commit comments

Comments
 (0)