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

Commit db681f9

Browse files
joelverhagenjoelverhagen
committed
Reject signatures that are not version 1 and reject author and repository counter signatures (#308)
Progress on NuGet/Engineering#785
1 parent d746a83 commit db681f9

12 files changed

Lines changed: 456 additions & 139 deletions

File tree

‎src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -131,25 +131,25 @@
131131
<Version>1.1.2</Version>
132132
</PackageReference>
133133
<PackageReference Include="NuGet.Services.Configuration">
134-
<Version>2.9.0</Version>
134+
<Version>2.10.0</Version>
135135
</PackageReference>
136136
<PackageReference Include="NuGet.Services.Contracts">
137-
<Version>2.9.0</Version>
137+
<Version>2.10.0</Version>
138138
</PackageReference>
139139
<PackageReference Include="NuGet.Services.KeyVault">
140-
<Version>2.9.0</Version>
140+
<Version>2.10.0</Version>
141141
</PackageReference>
142142
<PackageReference Include="NuGet.Services.Logging">
143-
<Version>2.9.0</Version>
143+
<Version>2.10.0</Version>
144144
</PackageReference>
145145
<PackageReference Include="NuGet.Services.ServiceBus">
146-
<Version>2.9.0</Version>
146+
<Version>2.10.0</Version>
147147
</PackageReference>
148148
<PackageReference Include="NuGet.Services.Validation">
149-
<Version>2.9.0</Version>
149+
<Version>2.10.0</Version>
150150
</PackageReference>
151151
<PackageReference Include="NuGet.Services.Validation.Issues">
152-
<Version>2.9.0</Version>
152+
<Version>2.10.0</Version>
153153
</PackageReference>
154154
<PackageReference Include="NuGet.Versioning">
155155
<Version>4.3.0</Version>

‎src/Validation.Callback.Vcs/Web.config‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@
7070
</dependentAssembly>
7171
<dependentAssembly>
7272
<assemblyIdentity name="NuGet.Services.KeyVault" publicKeyToken="31bf3856ad364e35" culture="neutral" />
73-
<bindingRedirect oldVersion="0.0.0.0-2.9.0.0" newVersion="2.9.0.0" />
73+
<bindingRedirect oldVersion="0.0.0.0-2.10.0.0" newVersion="2.10.0.0" />
7474
</dependentAssembly>
7575
<dependentAssembly>
7676
<assemblyIdentity name="Microsoft.Extensions.Configuration.Abstractions" publicKeyToken="adb9793829ddae60" culture="neutral" />

‎src/Validation.Common/Validation.Common.csproj‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,11 @@
105105
<Reference Include="NuGet.ApplicationInsights.Owin, Version=1.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
106106
<HintPath>..\..\packages\NuGet.ApplicationInsights.Owin.4.1.0\lib\net452\NuGet.ApplicationInsights.Owin.dll</HintPath>
107107
</Reference>
108-
<Reference Include="NuGet.Services.KeyVault, Version=2.9.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
109-
<HintPath>..\..\packages\NuGet.Services.KeyVault.2.9.0\lib\net45\NuGet.Services.KeyVault.dll</HintPath>
108+
<Reference Include="NuGet.Services.KeyVault, Version=2.10.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
109+
<HintPath>..\..\packages\NuGet.Services.KeyVault.2.10.0\lib\net45\NuGet.Services.KeyVault.dll</HintPath>
110110
</Reference>
111-
<Reference Include="NuGet.Services.Logging, Version=2.9.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
112-
<HintPath>..\..\packages\NuGet.Services.Logging.2.9.0\lib\net452\NuGet.Services.Logging.dll</HintPath>
111+
<Reference Include="NuGet.Services.Logging, Version=2.10.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
112+
<HintPath>..\..\packages\NuGet.Services.Logging.2.10.0\lib\net452\NuGet.Services.Logging.dll</HintPath>
113113
</Reference>
114114
<Reference Include="NuGet.Services.VirusScanning.Core, Version=1.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
115115
<HintPath>..\..\packages\NuGet.Services.VirusScanning.Vcs.3.2.0\lib\net452\NuGet.Services.VirusScanning.Core.dll</HintPath>

‎src/Validation.Common/app.config‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
55
<dependentAssembly>
66
<assemblyIdentity name="NuGet.Services.Logging" publicKeyToken="31BF3856AD364E35" culture="neutral" />
7-
<bindingRedirect oldVersion="0.0.0.0-2.9.0.0" newVersion="2.9.0.0" />
7+
<bindingRedirect oldVersion="0.0.0.0-2.10.0.0" newVersion="2.10.0.0" />
88
</dependentAssembly>
99
<dependentAssembly>
1010
<assemblyIdentity name="Newtonsoft.Json" publicKeyToken="30ad4fe6b2a6aeed" culture="neutral" />
@@ -40,7 +40,7 @@
4040
</dependentAssembly>
4141
<dependentAssembly>
4242
<assemblyIdentity name="NuGet.Services.KeyVault" publicKeyToken="31bf3856ad364e35" culture="neutral" />
43-
<bindingRedirect oldVersion="0.0.0.0-2.9.0.0" newVersion="2.9.0.0" />
43+
<bindingRedirect oldVersion="0.0.0.0-2.10.0.0" newVersion="2.10.0.0" />
4444
</dependentAssembly>
4545
<dependentAssembly>
4646
<assemblyIdentity name="Microsoft.Extensions.Configuration.Abstractions" publicKeyToken="adb9793829ddae60" culture="neutral" />

‎src/Validation.Common/packages.config‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
<package id="Microsoft.WindowsAzure.ConfigurationManager" version="3.2.1" targetFramework="net452" />
2323
<package id="Newtonsoft.Json" version="9.0.1" targetFramework="net452" />
2424
<package id="NuGet.ApplicationInsights.Owin" version="4.1.0" targetFramework="net452" />
25-
<package id="NuGet.Services.KeyVault" version="2.9.0" targetFramework="net452" />
26-
<package id="NuGet.Services.Logging" version="2.9.0" targetFramework="net452" />
25+
<package id="NuGet.Services.KeyVault" version="2.10.0" targetFramework="net452" />
26+
<package id="NuGet.Services.Logging" version="2.10.0" targetFramework="net452" />
2727
<package id="NuGet.Services.VirusScanning.Vcs" version="3.2.0" targetFramework="net452" />
2828
<package id="Owin" version="1.0" targetFramework="net452" />
2929
<package id="Serilog" version="2.0.0" targetFramework="net452" />

‎src/Validation.PackageSigning.Core/Validation.PackageSigning.Core.csproj‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@
3939
</ItemGroup>
4040
<ItemGroup>
4141
<PackageReference Include="NuGet.Services.ServiceBus">
42-
<Version>2.9.0</Version>
42+
<Version>2.10.0</Version>
4343
</PackageReference>
4444
<PackageReference Include="NuGet.Services.Storage">
45-
<Version>2.9.0</Version>
45+
<Version>2.10.0</Version>
4646
</PackageReference>
4747
<PackageReference Include="NuGet.Services.Validation">
48-
<Version>2.9.0</Version>
48+
<Version>2.10.0</Version>
4949
</PackageReference>
5050
<PackageReference Include="WindowsAzure.Storage">
5151
<Version>7.1.2</Version>

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

Lines changed: 118 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33

44
using System;
55
using System.Linq;
6+
using System.Security.Cryptography;
7+
using System.Security.Cryptography.Pkcs;
68
using System.Security.Cryptography.X509Certificates;
79
using System.Threading;
810
using System.Threading.Tasks;
911
using Microsoft.Extensions.Logging;
12+
using NuGet.Common;
1013
using NuGet.Jobs.Validation.PackageSigning.Messages;
1114
using NuGet.Jobs.Validation.PackageSigning.Storage;
1215
using NuGet.Packaging.Signing;
@@ -80,81 +83,123 @@ private async Task<SignatureValidatorResult> HandleSignedPackageAsync(
8083
SignatureValidationMessage message,
8184
CancellationToken cancellationToken)
8285
{
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)
86+
try
9287
{
93-
return invalidFormatResult;
94-
}
88+
// First, detect format errors with a minimal verification. This doesn't even check package integrity. The
89+
// minimal verification is expected to swallow any sort of signature format exception.
90+
var invalidFormatResult = await GetVerifyResult(
91+
_minimalPackageSignatureVerifier,
92+
packageKey,
93+
signedPackageReader,
94+
message,
95+
cancellationToken);
96+
if (invalidFormatResult != null)
97+
{
98+
return invalidFormatResult;
99+
}
95100

96-
// We now know we can safely read the signature.
97-
var packageSignature = await signedPackageReader.GetSignatureAsync(cancellationToken);
101+
// We now know we can safely read the signature.
102+
var packageSignature = await signedPackageReader.GetSignatureAsync(cancellationToken);
98103

99-
// Block packages with any unknown signing certificates.
100-
var packageThumbprint = packageSignature
101-
.SignerInfo
102-
.Certificate
103-
.ComputeSHA256Thumbprint();
104-
var isKnownCertificate = _certificates
105-
.GetAll()
106-
.Where(c => packageThumbprint == c.Thumbprint)
107-
.Any();
104+
// Block packages with any unknown signing certificates.
105+
var packageThumbprint = packageSignature
106+
.SignerInfo
107+
.Certificate
108+
.ComputeSHA256Thumbprint();
109+
var isKnownCertificate = _certificates
110+
.GetAll()
111+
.Any(c => packageThumbprint == c.Thumbprint);
112+
if (!isKnownCertificate)
113+
{
114+
_logger.LogInformation(
115+
"Signed package {PackageId} {PackageVersion} is blocked for validation {ValidationId} since it has an unknown certificate thumbprint: {UnknownThumbprint}",
116+
message.PackageId,
117+
message.PackageVersion,
118+
message.ValidationId,
119+
packageThumbprint);
120+
121+
return await RejectAsync(
122+
packageKey,
123+
message,
124+
ValidationIssue.PackageIsSigned);
125+
}
126+
127+
// Only reject counter signatures that have the author or repository commitment type. Other types of
128+
// counter signatures are not produced by the client but technically just fine.
129+
var authorOrRepositoryCounterSignatureCount = packageSignature
130+
.SignerInfo
131+
.CounterSignerInfos
132+
.Cast<SignerInfo>()
133+
.Select(x => x.SignedAttributes.FirstOrDefault(Oids.CommitmentTypeIndication))
134+
.Where(x => x != null)
135+
.Select(x => AttributeUtility.GetCommitmentTypeIndication(x))
136+
.Count(x => x != SignatureType.Unknown);
137+
if (authorOrRepositoryCounterSignatureCount > 0)
138+
{
139+
_logger.LogInformation(
140+
"Signed package {PackageId} {PackageVersion} is blocked for validation {ValidationId} since it has {AuthorOrRepositorySignatureCount} invalid countersignatures.",
141+
message.PackageId,
142+
message.PackageVersion,
143+
message.ValidationId,
144+
authorOrRepositoryCounterSignatureCount);
145+
146+
return await RejectAsync(
147+
packageKey,
148+
message,
149+
ValidationIssue.AuthorAndRepositoryCounterSignaturesNotSupported);
150+
}
151+
152+
if (packageSignature.Type != SignatureType.Author)
153+
{
154+
_logger.LogInformation(
155+
"Signed package {PackageId} {PackageVersion} is blocked for validation {ValidationId} since it is not author signed: {SignatureType}",
156+
message.PackageId,
157+
message.PackageVersion,
158+
message.ValidationId,
159+
packageSignature.Type);
160+
161+
return await RejectAsync(
162+
packageKey,
163+
message,
164+
ValidationIssue.OnlyAuthorSignaturesSupported);
165+
}
166+
167+
// Call the "verify" API, which does the main logic of signature validation.
168+
var failureResult = await GetVerifyResult(
169+
_fullPackageSignatureVerifier,
170+
packageKey,
171+
signedPackageReader,
172+
message,
173+
cancellationToken);
174+
if (failureResult != null)
175+
{
176+
return failureResult;
177+
}
108178

109-
if (!isKnownCertificate)
110-
{
111179
_logger.LogInformation(
112-
"Signed package {PackageId} {PackageVersion} is blocked for validation {ValidationId} since it has an unknown certificate thumbprint: {UnknownThumbprint}",
180+
"Signed package {PackageId} {PackageVersion} for validation {ValidationId} is valid with certificate thumbprint: {PackageThumbprint}",
113181
message.PackageId,
114182
message.PackageVersion,
115183
message.ValidationId,
116184
packageThumbprint);
117-
118-
return await RejectAsync(
119-
packageKey,
120-
message,
121-
ValidationIssue.PackageIsSigned);
122185
}
123-
124-
if (packageSignature.Type != SignatureType.Author)
186+
catch (SignatureException ex)
125187
{
188+
EventId eventId = 0;
126189
_logger.LogInformation(
127-
"Signed package {PackageId} {PackageVersion} is blocked for validation {ValidationId} since it has a non-author signature: {SignatureType}",
190+
eventId,
191+
ex,
192+
"Signed package {PackageId} {PackageVersion} is blocked for validation {ValidationId} due to an exception during signature validation.",
128193
message.PackageId,
129194
message.PackageVersion,
130-
message.ValidationId,
131-
packageSignature.Type);
195+
message.ValidationId);
132196

133197
return await RejectAsync(
134198
packageKey,
135199
message,
136-
ValidationIssue.OnlyAuthorSignaturesSupported);
200+
new ClientSigningVerificationFailure(ex.Code.ToString(), ex.Message));
137201
}
138202

139-
// Call the "verify" API, which does the main logic of signature validation.
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-
158203
// Extract all of the signature artifacts and persist them.
159204
await _signaturePartsExtractor.ExtractAsync(signedPackageReader, cancellationToken);
160205

@@ -196,12 +241,27 @@ private async Task<SignatureValidatorResult> GetVerifyResult(
196241
errorsForLogs,
197242
warningsForLogs);
198243

244+
// Treat the "signature format version" error specially. This is because the message provided by the
245+
// client does not make sense in the server context.
246+
IValidationIssue[] errorValidationIssues;
247+
if (errorIssues.Any(x => x.Code == NuGetLogCode.NU3007))
248+
{
249+
errorValidationIssues = new[]
250+
{
251+
ValidationIssue.OnlySignatureFormatVersion1Supported,
252+
};
253+
}
254+
else
255+
{
256+
errorValidationIssues = errorIssues
257+
.Select(x => new ClientSigningVerificationFailure(x.Code.ToString(), x.Message))
258+
.ToArray();
259+
}
260+
199261
return await RejectAsync(
200262
packageKey,
201263
message,
202-
errorIssues
203-
.Select(x => new ClientSigningVerificationFailure(x.Code.ToString(), x.Message))
204-
.ToArray());
264+
errorValidationIssues);
205265
}
206266

207267
return null;

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,22 +109,22 @@
109109
<Version>4.6.0-rtm-4822</Version>
110110
</PackageReference>
111111
<PackageReference Include="NuGet.Services.Configuration">
112-
<Version>2.9.0</Version>
112+
<Version>2.10.0</Version>
113113
</PackageReference>
114114
<PackageReference Include="NuGet.Services.Contracts">
115-
<Version>2.9.0</Version>
115+
<Version>2.10.0</Version>
116116
</PackageReference>
117117
<PackageReference Include="NuGet.Services.KeyVault">
118-
<Version>2.9.0</Version>
118+
<Version>2.10.0</Version>
119119
</PackageReference>
120120
<PackageReference Include="NuGet.Services.Logging">
121-
<Version>2.9.0</Version>
121+
<Version>2.10.0</Version>
122122
</PackageReference>
123123
<PackageReference Include="NuGet.Services.ServiceBus">
124-
<Version>2.9.0</Version>
124+
<Version>2.10.0</Version>
125125
</PackageReference>
126126
<PackageReference Include="NuGet.Services.Validation">
127-
<Version>2.9.0</Version>
127+
<Version>2.10.0</Version>
128128
</PackageReference>
129129
<PackageReference Include="NuGetGallery.Core">
130130
<Version>4.4.4-dev-19677</Version>

‎src/Validation.PackageSigning.ValidateCertificate/Validation.PackageSigning.ValidateCertificate.csproj‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,16 @@
103103
<Version>1.1.2</Version>
104104
</PackageReference>
105105
<PackageReference Include="NuGet.Services.Configuration">
106-
<Version>2.9.0</Version>
106+
<Version>2.10.0</Version>
107107
</PackageReference>
108108
<PackageReference Include="NuGet.Services.Contracts">
109-
<Version>2.9.0</Version>
109+
<Version>2.10.0</Version>
110110
</PackageReference>
111111
<PackageReference Include="NuGet.Services.KeyVault">
112-
<Version>2.9.0</Version>
112+
<Version>2.10.0</Version>
113113
</PackageReference>
114114
<PackageReference Include="NuGet.Services.Validation">
115-
<Version>2.9.0</Version>
115+
<Version>2.10.0</Version>
116116
</PackageReference>
117117
<PackageReference Include="System.Net.Http">
118118
<Version>4.3.3</Version>

‎src/Validation.Runner/App.config‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
</dependentAssembly>
4848
<dependentAssembly>
4949
<assemblyIdentity name="NuGet.Services.KeyVault" publicKeyToken="31bf3856ad364e35" culture="neutral" />
50-
<bindingRedirect oldVersion="0.0.0.0-2.9.0.0" newVersion="2.9.0.0" />
50+
<bindingRedirect oldVersion="0.0.0.0-2.10.0.0" newVersion="2.10.0.0" />
5151
</dependentAssembly>
5252
<dependentAssembly>
5353
<assemblyIdentity name="Microsoft.Extensions.Configuration.Abstractions" publicKeyToken="adb9793829ddae60" culture="neutral" />

0 commit comments

Comments
 (0)