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

Commit 6c8d8e9

Browse files
authored
[Package Signing] Verify the author signature independently of the repository signature (#467)
In this change: * The process signature job now properly rejects untrusted author signing certificate when the package is repository countersigned * Updates NuGet client dependencies * Test improvements * Fixed several issues where responders were disposed more than intended * Properly wait for responder's responses to expire instead of sleeping by a second * Fixed several flaky tests (I think) Fixes https://github.com/NuGet/Engineering/issues/1417
1 parent f9f5309 commit 6c8d8e9

13 files changed

Lines changed: 647 additions & 372 deletions

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@
9090
<Version>1.1.2</Version>
9191
</PackageReference>
9292
<PackageReference Include="NuGet.Packaging">
93-
<Version>4.8.0-preview1.5195</Version>
93+
<Version>4.8.0-preview4.5289</Version>
9494
</PackageReference>
9595
<PackageReference Include="NuGet.Services.Configuration">
9696
<Version>2.25.0</Version>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
<dependency id="Microsoft.ApplicationInsights" version="2.2.0" />
1616
<dependency id="Microsoft.Extensions.DependencyInjection" version="1.1.1" />
1717
<dependency id="Microsoft.Extensions.Options.ConfigurationExtensions" version="1.1.2" />
18-
<dependency id="NuGet.Packaging" version="4.8.0-preview1.5195" />
18+
<dependency id="NuGet.Packaging" version="4.8.0-preview4.5289" />
1919
<dependency id="NuGet.Services.Configuration" version="2.25.0" />
2020
<dependency id="NuGet.Services.Logging" version="2.25.0" />
2121
<dependency id="NuGet.Services.Sql" version="2.26.0-master-33196" />

src/Validation.PackageSigning.ProcessSignature/ISignatureFormatValidator.cs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,23 @@ Task<VerifySignaturesResult> ValidateMinimalAsync(
1919
ISignedPackageReader package,
2020
CancellationToken token);
2121

22+
/// <summary>
23+
/// Run all validations on the package's author signature. This includes integrity and trust validations.
24+
/// </summary>
25+
/// <param name="package">The package whose author signature should be validated.</param>
26+
/// <param name="token"></param>
27+
/// <returns>The result of the author signature's verification.</returns>
28+
Task<VerifySignaturesResult> ValidateAuthorSignatureAsync(
29+
ISignedPackageReader package,
30+
CancellationToken token);
31+
2232
/// <summary>
2333
/// Run all validations on the package's repository signature. This includes integrity and trust validations.
2434
/// </summary>
2535
/// <param name="package">The package whose repository signature should be validated.</param>
2636
/// <param name="token"></param>
27-
/// <returns>Whether the package's signature is readable.</returns>
28-
Task<SignatureVerificationStatus> VerifyRepositorySignatureAsync(
37+
/// <returns>The result of the repository signature's verification.</returns>
38+
Task<VerifySignaturesResult> ValidateRepositorySignatureAsync(
2939
ISignedPackageReader package,
3040
CancellationToken token);
3141

@@ -35,10 +45,10 @@ Task<SignatureVerificationStatus> VerifyRepositorySignatureAsync(
3545
/// <param name="package">The package to validate.</param>
3646
/// <param name="hasRepositorySignature">If false, skips the certificate allow list verification of the repository signature.</param>
3747
/// <param name="token"></param>
38-
/// <returns>Whether the package's signature is valid.</returns>
39-
Task<VerifySignaturesResult> ValidateFullAsync(
48+
/// <returns>The result of the package's signature(s) verification.</returns>
49+
Task<VerifySignaturesResult> ValidateAllSignaturesAsync(
4050
ISignedPackageReader package,
41-
bool hasRepositorySignature,
51+
bool hasRepositorySignature, // TODO: Remove parameter once this is fixed: https://github.com/NuGet/Home/issues/7042
4252
CancellationToken token);
4353
}
4454
}

src/Validation.PackageSigning.ProcessSignature/SignatureFormatValidator.cs

Lines changed: 55 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -22,30 +22,32 @@ public class SignatureFormatValidator : ISignatureFormatValidator
2222
allowMultipleTimestamps: true,
2323
allowNoTimestamp: true,
2424
allowUnknownRevocation: true,
25+
reportUnknownRevocation: false,
2526
allowNoRepositoryCertificateList: true,
2627
allowNoClientCertificateList: true,
27-
alwaysVerifyCountersignature: false,
28+
verificationTarget: VerificationTarget.All,
29+
signaturePlacement: SignaturePlacement.PrimarySignature,
30+
repositoryCountersignatureVerificationBehavior: SignatureVerificationBehavior.Never,
2831
repoAllowListEntries: null,
2932
clientAllowListEntries: null);
3033

31-
private static readonly IEnumerable<ISignatureVerificationProvider> _minimalProviders = new[]
34+
private static readonly PackageSignatureVerifier _minimalVerifier = new PackageSignatureVerifier(new[]
3235
{
3336
new MinimalSignatureVerificationProvider(),
34-
};
37+
});
3538

36-
private static readonly IEnumerable<ISignatureVerificationProvider> _fullProviders = new ISignatureVerificationProvider[]
39+
private static readonly PackageSignatureVerifier _fullVerifier = new PackageSignatureVerifier(new ISignatureVerificationProvider[]
3740
{
3841
new IntegrityVerificationProvider(),
3942
new SignatureTrustAndValidityVerificationProvider(),
4043
new AllowListVerificationProvider(),
41-
};
44+
});
4245

4346
private readonly IOptionsSnapshot<ProcessSignatureConfiguration> _config;
4447
private readonly SignedPackageVerifierSettings _authorSignatureSettings;
48+
private readonly SignedPackageVerifierSettings _repositorySignatureSettings;
4549
private readonly SignedPackageVerifierSettings _authorOrRepositorySignatureSettings;
4650

47-
private readonly RepositorySignatureVerifier _repositorySignatureVerifier;
48-
4951
public SignatureFormatValidator(IOptionsSnapshot<ProcessSignatureConfiguration> config)
5052
{
5153
_config = config ?? throw new ArgumentNullException(nameof(config));
@@ -58,11 +60,14 @@ public SignatureFormatValidator(IOptionsSnapshot<ProcessSignatureConfiguration>
5860
allowMultipleTimestamps: false,
5961
allowNoTimestamp: false,
6062
allowUnknownRevocation: true,
61-
allowNoClientCertificateList: true,
62-
alwaysVerifyCountersignature: true,
63-
clientAllowListEntries: null,
63+
reportUnknownRevocation: true,
6464
allowNoRepositoryCertificateList: true,
65-
repoAllowListEntries: null);
65+
allowNoClientCertificateList: true,
66+
verificationTarget: VerificationTarget.Author,
67+
signaturePlacement: SignaturePlacement.PrimarySignature,
68+
repositoryCountersignatureVerificationBehavior: SignatureVerificationBehavior.Never,
69+
repoAllowListEntries: null,
70+
clientAllowListEntries: null);
6671

6772
var repoAllowListEntries = _config
6873
.Value
@@ -76,64 +81,81 @@ public SignatureFormatValidator(IOptionsSnapshot<ProcessSignatureConfiguration>
7681

7782
repoAllowListEntries = repoAllowListEntries ?? new List<CertificateHashAllowListEntry>();
7883

79-
_authorOrRepositorySignatureSettings = new SignedPackageVerifierSettings(
84+
_repositorySignatureSettings = new SignedPackageVerifierSettings(
8085
allowUnsigned: _authorSignatureSettings.AllowUnsigned,
8186
allowIllegal: _authorSignatureSettings.AllowIllegal,
8287
allowUntrusted: _authorSignatureSettings.AllowUntrusted,
8388
allowIgnoreTimestamp: _authorSignatureSettings.AllowIgnoreTimestamp,
8489
allowMultipleTimestamps: _authorSignatureSettings.AllowMultipleTimestamps,
8590
allowNoTimestamp: _authorSignatureSettings.AllowNoTimestamp,
8691
allowUnknownRevocation: _authorSignatureSettings.AllowUnknownRevocation,
87-
allowNoClientCertificateList: _authorSignatureSettings.AllowNoClientCertificateList,
88-
alwaysVerifyCountersignature: _authorSignatureSettings.AlwaysVerifyCountersignature,
89-
clientAllowListEntries: _authorSignatureSettings.ClientCertificateList,
92+
reportUnknownRevocation: _authorSignatureSettings.ReportUnknownRevocation,
9093
allowNoRepositoryCertificateList: false,
91-
repoAllowListEntries: repoAllowListEntries);
94+
allowNoClientCertificateList: _authorSignatureSettings.AllowNoClientCertificateList,
95+
verificationTarget: VerificationTarget.Repository,
96+
signaturePlacement: SignaturePlacement.Any,
97+
repositoryCountersignatureVerificationBehavior: SignatureVerificationBehavior.IfExists,
98+
repoAllowListEntries: repoAllowListEntries,
99+
clientAllowListEntries: _authorSignatureSettings.ClientCertificateList);
92100

93-
_repositorySignatureVerifier = new RepositorySignatureVerifier();
101+
_authorOrRepositorySignatureSettings = new SignedPackageVerifierSettings(
102+
allowUnsigned: _authorSignatureSettings.AllowUnsigned,
103+
allowIllegal: _authorSignatureSettings.AllowIllegal,
104+
allowUntrusted: _authorSignatureSettings.AllowUntrusted,
105+
allowIgnoreTimestamp: _authorSignatureSettings.AllowIgnoreTimestamp,
106+
allowMultipleTimestamps: _authorSignatureSettings.AllowMultipleTimestamps,
107+
allowNoTimestamp: _authorSignatureSettings.AllowNoTimestamp,
108+
allowUnknownRevocation: _authorSignatureSettings.AllowUnknownRevocation,
109+
reportUnknownRevocation: _authorSignatureSettings.ReportUnknownRevocation,
110+
allowNoRepositoryCertificateList: false,
111+
allowNoClientCertificateList: _authorSignatureSettings.AllowNoClientCertificateList,
112+
verificationTarget: VerificationTarget.All,
113+
signaturePlacement: SignaturePlacement.Any,
114+
repositoryCountersignatureVerificationBehavior: SignatureVerificationBehavior.IfExists,
115+
repoAllowListEntries: repoAllowListEntries,
116+
clientAllowListEntries: _authorSignatureSettings.ClientCertificateList);
94117
}
95118

96119
public async Task<VerifySignaturesResult> ValidateMinimalAsync(
97120
ISignedPackageReader package,
98121
CancellationToken token)
99122
{
100-
return await VerifyAsync(
123+
return await _minimalVerifier.VerifySignaturesAsync(
101124
package,
102-
_minimalProviders,
103125
_minimalSettings,
104126
token);
105127
}
106128

107-
public async Task<VerifySignaturesResult> ValidateFullAsync(
129+
public async Task<VerifySignaturesResult> ValidateAuthorSignatureAsync(
108130
ISignedPackageReader package,
109-
bool hasRepositorySignature,
110131
CancellationToken token)
111132
{
112-
var settings = hasRepositorySignature ? _authorOrRepositorySignatureSettings : _authorSignatureSettings;
113-
114-
return await VerifyAsync(
133+
return await _fullVerifier.VerifySignaturesAsync(
115134
package,
116-
_fullProviders,
117-
settings,
135+
_authorSignatureSettings,
118136
token);
119137
}
120138

121-
public async Task<SignatureVerificationStatus> VerifyRepositorySignatureAsync(
139+
public async Task<VerifySignaturesResult> ValidateRepositorySignatureAsync(
122140
ISignedPackageReader package,
123141
CancellationToken token)
124142
{
125-
return await _repositorySignatureVerifier.VerifyAsync(package, token);
143+
return await _fullVerifier.VerifySignaturesAsync(
144+
package,
145+
_repositorySignatureSettings,
146+
token);
126147
}
127148

128-
private static async Task<VerifySignaturesResult> VerifyAsync(
149+
public async Task<VerifySignaturesResult> ValidateAllSignaturesAsync(
129150
ISignedPackageReader package,
130-
IEnumerable<ISignatureVerificationProvider> verificationProviders,
131-
SignedPackageVerifierSettings settings,
151+
bool hasRepositorySignature,
132152
CancellationToken token)
133153
{
134-
var verifier = new PackageSignatureVerifier(verificationProviders);
154+
// TODO - Use only the "authorOrRepositorySignatureSettings" once this issue is fixed:
155+
// https://github.com/NuGet/Home/issues/7042
156+
var settings = hasRepositorySignature ? _authorOrRepositorySignatureSettings : _authorSignatureSettings;
135157

136-
return await verifier.VerifySignaturesAsync(
158+
return await _fullVerifier.VerifySignaturesAsync(
137159
package,
138160
settings,
139161
token);

0 commit comments

Comments
 (0)