Skip to content

Commit 62bd2c3

Browse files
drewgilliesdrewgillies
authored
Extend vulnerabilities verification to metadata (#8412)
Extend vulnerabilities verification to metadata
1 parent 5871c77 commit 62bd2c3

5 files changed

Lines changed: 237 additions & 19 deletions

File tree

‎src/VerifyGitHubVulnerabilities/Configuration/VerifyGitHubVulnerabilitiesConfiguration.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,20 @@ public class VerifyGitHubVulnerabilitiesConfiguration
1616
/// The personal access token to use to authenticate with GitHub.
1717
/// </summary>
1818
public string GitHubPersonalAccessToken { get; set; }
19+
20+
/// <summary>
21+
/// The v3 index URI string for fetching registration metadata
22+
/// </summary>
23+
public string NuGetV3Index { get; set; }
24+
25+
/// <summary>
26+
/// Whether to verify GitHubVulnerabilities in the gallery database
27+
/// </summary>
28+
public bool VerifyDatabase { get; set; } = true;
29+
30+
/// <summary>
31+
/// Whether to verify GitHubVulnerabilities in the registration blobs
32+
/// </summary>
33+
public bool VerifyRegistrationMetadata { get; set; } = true;
1934
}
2035
}

‎src/VerifyGitHubVulnerabilities/Job.cs‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@ public override async Task Run()
3434
Console.WriteLine($" FOUND {advisories.Count} advisories.");
3535

3636
Console.WriteLine("Fetching vulnerabilities from DB...");
37-
var verifier = new PackageVulnerabilitiesVerifier(_serviceProvider.GetRequiredService<IEntitiesContext>());
38-
var ingestor = new AdvisoryIngestor(verifier, new GitHubVersionRangeParser());
37+
var ingestor = _serviceProvider.GetRequiredService<IAdvisoryIngestor>();
3938
await ingestor.IngestAsync(advisories);
4039

40+
var verifier = _serviceProvider.GetRequiredService<IPackageVulnerabilitiesVerifier>();
4141
Console.WriteLine(verifier.HasErrors ?
4242
"DB does not match GitHub API - see stderr output for details" :
43-
"DB matches GitHub API!");
43+
"DB/metadata matches GitHub API!");
4444
}
4545

4646
protected override void ConfigureJobServices(IServiceCollection services, IConfigurationRoot configurationRoot)
@@ -68,6 +68,12 @@ protected void ConfigureIngestionServices(ContainerBuilder containerBuilder)
6868
containerBuilder
6969
.RegisterType<AdvisoryIngestor>()
7070
.As<IAdvisoryIngestor>();
71+
72+
containerBuilder
73+
.RegisterType<PackageVulnerabilitiesVerifier>()
74+
.As<IPackageVulnerabilitiesManagementService>()
75+
.As<IPackageVulnerabilitiesVerifier>()
76+
.SingleInstance();
7177
}
7278

7379
protected void ConfigureGalleryServices(ContainerBuilder containerBuilder)

‎src/VerifyGitHubVulnerabilities/Verify/IPackageVulnerabilitiesVerifier.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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 NuGet.Services.Entities;
5+
using NuGetGallery;
6+
7+
namespace VerifyGitHubVulnerabilities.Verify
8+
{
9+
/// <summary>
10+
/// We need a <see cref=IPackageVulnerabilitiesManagementService" /> for inserting the verifier into the ingestion job,
11+
/// so we extend for our extra reporting data.
12+
/// </summary>
13+
public interface IPackageVulnerabilitiesVerifier : IPackageVulnerabilitiesManagementService
14+
{
15+
/// <summary>
16+
/// An error flag for verification
17+
/// </summary>
18+
bool HasErrors { get; }
19+
}
20+
}

‎src/VerifyGitHubVulnerabilities/Verify/PackageVulnerabilitiesVerifier.cs‎

Lines changed: 192 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,42 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Collections.Generic;
56
using System.Data.Entity;
67
using System.Linq;
8+
using System.Threading;
79
using System.Threading.Tasks;
10+
using NuGet.Configuration;
11+
using NuGet.Protocol;
12+
using NuGet.Protocol.Core.Types;
813
using NuGet.Services.Entities;
914
using NuGet.Versioning;
1015
using NuGetGallery;
16+
using VerifyGitHubVulnerabilities.Configuration;
1117

1218
namespace VerifyGitHubVulnerabilities.Verify
1319
{
14-
public class PackageVulnerabilitiesVerifier : IPackageVulnerabilitiesManagementService
20+
public class PackageVulnerabilitiesVerifier : IPackageVulnerabilitiesVerifier
1521
{
22+
private readonly VerifyGitHubVulnerabilitiesConfiguration _configuration;
1623
private readonly IEntitiesContext _entitiesContext;
1724

18-
public PackageVulnerabilitiesVerifier(
25+
private Lazy<Task<PackageMetadataResource>> _packageMetadataResource;
26+
private Dictionary<string, IEnumerable<IPackageSearchMetadata>> _packageMetadata;
27+
28+
private static readonly SemaphoreSlim semaphoreSlim = new SemaphoreSlim(1);
29+
30+
public PackageVulnerabilitiesVerifier(VerifyGitHubVulnerabilitiesConfiguration configuration,
1931
IEntitiesContext entitiesContext)
2032
{
21-
_entitiesContext = entitiesContext ?? throw new ArgumentNullException(nameof(entitiesContext));
33+
_configuration = configuration;
34+
if (_configuration.VerifyDatabase)
35+
{
36+
_entitiesContext = entitiesContext;
37+
}
38+
39+
_packageMetadata = new Dictionary<string, IEnumerable<IPackageSearchMetadata>>();
40+
_packageMetadataResource = new Lazy<Task<PackageMetadataResource>>(InitializeMetadataResourceAsync);
2241
}
2342

2443
public bool HasErrors { get; private set; }
@@ -36,7 +55,26 @@ public Task UpdateVulnerabilityAsync(PackageVulnerability vulnerability, bool wi
3655
return Task.CompletedTask;
3756
}
3857

39-
Console.WriteLine($"Verifying vulnerability {vulnerability.GitHubDatabaseKey}.");
58+
if (_configuration.VerifyDatabase)
59+
{
60+
VerifyVulnerabilityInDatabase(vulnerability, withdrawn);
61+
}
62+
63+
// Note: testing a withdrawn advisory isn't practical in registration metadata. We can only download
64+
// metadata for a package, and would need to download all package/version blobs to determine an advisory
65+
// is no longer present. Covering withdrawn advisory processing in the database will be adequate.
66+
if (_configuration.VerifyRegistrationMetadata && !withdrawn)
67+
{
68+
return VerifyVulnerabilityInMetadataAsync(vulnerability);
69+
}
70+
71+
return Task.CompletedTask;
72+
}
73+
74+
private void VerifyVulnerabilityInDatabase(PackageVulnerability vulnerability, bool withdrawn)
75+
{
76+
Console.WriteLine($"[Database] Verifying vulnerability {vulnerability.GitHubDatabaseKey}.");
77+
4078
var existingVulnerability = _entitiesContext.Vulnerabilities
4179
.Include(v => v.AffectedRanges)
4280
.SingleOrDefault(v => v.GitHubDatabaseKey == vulnerability.GitHubDatabaseKey);
@@ -46,47 +84,47 @@ public Task UpdateVulnerabilityAsync(PackageVulnerability vulnerability, bool wi
4684
if (existingVulnerability != null)
4785
{
4886
Console.Error.WriteLine(withdrawn ?
49-
$@"Vulnerability advisory {vulnerability.GitHubDatabaseKey} was withdrawn and should not be in DB!" :
50-
$@"Vulnerability advisory {vulnerability.GitHubDatabaseKey} affects no packages and should not be in DB!");
87+
$@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey} was withdrawn and should not be in DB!" :
88+
$@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey} affects no packages and should not be in DB!");
5189
HasErrors = true;
5290
}
5391

54-
return Task.CompletedTask;
92+
return;
5593
}
5694

5795
if (existingVulnerability == null)
5896
{
59-
Console.Error.WriteLine($"Cannot find vulnerability {vulnerability.GitHubDatabaseKey} in DB!");
97+
Console.Error.WriteLine($"[Database] Cannot find vulnerability {vulnerability.GitHubDatabaseKey} in DB!");
6098
HasErrors = true;
61-
return Task.CompletedTask;
99+
return;
62100
}
63101

64102
if (existingVulnerability.Severity != vulnerability.Severity)
65103
{
66104
Console.Error.WriteLine(
67-
$@"Vulnerability advisory {vulnerability.GitHubDatabaseKey
105+
$@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey
68106
}, severity does not match! GitHub: {vulnerability.Severity}, DB: {existingVulnerability.Severity}");
69107
HasErrors = true;
70108
}
71109

72110
if (existingVulnerability.AdvisoryUrl != vulnerability.AdvisoryUrl)
73111
{
74112
Console.Error.WriteLine(
75-
$@"Vulnerability advisory {vulnerability.GitHubDatabaseKey
113+
$@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey
76114
}, advisory URL does not match! GitHub: {vulnerability.AdvisoryUrl}, DB: { existingVulnerability.AdvisoryUrl}");
77115
HasErrors = true;
78116
}
79117

80118
foreach (var range in vulnerability.AffectedRanges)
81119
{
82-
Console.WriteLine($"Verifying range affecting {range.PackageId} {range.PackageVersionRange}.");
120+
Console.WriteLine($"[Database] Verifying range affecting {range.PackageId} {range.PackageVersionRange}.");
83121
var existingRange = existingVulnerability.AffectedRanges
84122
.SingleOrDefault(r => r.PackageId == range.PackageId && r.PackageVersionRange == range.PackageVersionRange);
85123

86124
if (existingRange == null)
87125
{
88126
Console.Error.WriteLine(
89-
$@"Vulnerability advisory {vulnerability.GitHubDatabaseKey
127+
$@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey
90128
}, cannot find range {range.PackageId} {range.PackageVersionRange} in DB!");
91129
HasErrors = true;
92130
continue;
@@ -95,7 +133,7 @@ public Task UpdateVulnerabilityAsync(PackageVulnerability vulnerability, bool wi
95133
if (existingRange.FirstPatchedPackageVersion != range.FirstPatchedPackageVersion)
96134
{
97135
Console.Error.WriteLine(
98-
$@"Vulnerability advisory {vulnerability.GitHubDatabaseKey
136+
$@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey
99137
}, range {range.PackageId} {range.PackageVersionRange}, first patched version does not match! GitHub: {
100138
range.FirstPatchedPackageVersion}, DB: {range.FirstPatchedPackageVersion}");
101139
HasErrors = true;
@@ -113,15 +151,153 @@ public Task UpdateVulnerabilityAsync(PackageVulnerability vulnerability, bool wi
113151
if (versionRange.Satisfies(version) != package.VulnerablePackageRanges.Contains(existingRange))
114152
{
115153
Console.Error.WriteLine(
116-
$@"Vulnerability advisory {vulnerability.GitHubDatabaseKey
154+
$@"[Database] Vulnerability advisory {vulnerability.GitHubDatabaseKey
117155
}, range {range.PackageId} {range.PackageVersionRange}, package {package.NormalizedVersion
118156
} is not properly marked vulnerable to vulnerability!");
119157
HasErrors = true;
120158
}
121159
}
122160
}
161+
}
123162

124-
return Task.CompletedTask;
163+
private Task VerifyVulnerabilityInMetadataAsync(PackageVulnerability gitHubAdvisory)
164+
{
165+
Console.WriteLine($"[Metadata] Verifying vulnerability {gitHubAdvisory.GitHubDatabaseKey}.");
166+
167+
if (gitHubAdvisory.AffectedRanges == null || !gitHubAdvisory.AffectedRanges.Any())
168+
{
169+
return Task.CompletedTask;
170+
}
171+
172+
// Group ranges by id -- this makes testing metadata collections cleaner
173+
var rangesById = new Dictionary<string, IList<string>>();
174+
foreach (var range in gitHubAdvisory.AffectedRanges)
175+
{
176+
var id = range.PackageId.Trim(' '); // some incoming data needs cleaning
177+
if (rangesById.TryGetValue(id, out var packageVersionRangeForId))
178+
{
179+
packageVersionRangeForId.Add(range.PackageVersionRange);
180+
}
181+
else
182+
{
183+
rangesById[id] = new List<string> {range.PackageVersionRange};
184+
}
185+
}
186+
187+
var verificationJobsForAdvisory = new List<Task>();
188+
foreach (var rangeById in rangesById)
189+
{
190+
verificationJobsForAdvisory.Add(
191+
VerifyVulnerabilityForRangeAsync(
192+
rangeById.Key,
193+
ranges: rangeById.Value,
194+
gitHubAdvisory.AdvisoryUrl,
195+
gitHubAdvisory.GitHubDatabaseKey,
196+
gitHubAdvisory.Severity)
197+
);
198+
}
199+
200+
return Task.WhenAll(verificationJobsForAdvisory);
201+
}
202+
203+
private async Task VerifyVulnerabilityForRangeAsync(
204+
string packageId,
205+
IList<string> ranges,
206+
string advisoryUrl,
207+
int advisoryDatabaseKey,
208+
PackageVulnerabilitySeverity advisorySeverity)
209+
{
210+
// Fetch metadata from registration blobs for verification--a collection of all versions of the package Id
211+
var metadata = await GetPackageMetadataAsync(packageId);
212+
foreach (var versionMetadata in metadata)
213+
{
214+
var matchingVulnerabilities = Enumerable.Empty<PackageVulnerabilityMetadata>();
215+
if (versionMetadata.Vulnerabilities != null)
216+
{
217+
matchingVulnerabilities = versionMetadata.Vulnerabilities.Where(v => v.AdvisoryUrl.ToString() == advisoryUrl);
218+
}
219+
220+
var hasTheVulnerability = matchingVulnerabilities.Any();
221+
222+
// Check whether a version range pertaining to this id in the github advisory is satisfied by this metadata version
223+
var versionisInGitHubRange = false;
224+
foreach (var range in ranges)
225+
{
226+
var gitHubVersionRange = VersionRange.Parse(range);
227+
if (gitHubVersionRange.Satisfies(versionMetadata.Identity.Version, new VersionComparer()))
228+
{
229+
versionisInGitHubRange = true;
230+
break;
231+
}
232+
}
233+
234+
if (versionisInGitHubRange)
235+
{
236+
if (!hasTheVulnerability)
237+
{
238+
Console.Error.WriteLine(
239+
$@"[Metadata] Vulnerability advisory {advisoryDatabaseKey
240+
}, version {versionMetadata.Identity.Version} of package {packageId} is not marked vulnerable and is in a vulnerable range!");
241+
HasErrors = true;
242+
}
243+
244+
// Test whether we have any severity mismatches
245+
var firstSeverityMismatch = matchingVulnerabilities
246+
.FirstOrDefault(v => v.Severity != (int)advisorySeverity);
247+
if (firstSeverityMismatch != null)
248+
{
249+
Console.Error.WriteLine(
250+
$@"[Metadata] Vulnerability advisory {advisoryDatabaseKey
251+
}, severities has at least one mismatch! GitHub: {advisorySeverity}, Metadata: {firstSeverityMismatch.Severity}");
252+
HasErrors = true;
253+
}
254+
}
255+
else
256+
{
257+
if (hasTheVulnerability)
258+
{
259+
Console.Error.WriteLine(
260+
$@"[Metadata] Vulnerability advisory {advisoryDatabaseKey
261+
}, version {versionMetadata} of package {packageId} is marked vulnerable and is not in a vulnerable range!");
262+
HasErrors = true;
263+
}
264+
}
265+
}
266+
}
267+
268+
private async Task<IEnumerable<IPackageSearchMetadata>> GetPackageMetadataAsync(string packageId)
269+
{
270+
// We need this to be thread-safe as it's called by multiple tasks concurrently
271+
await semaphoreSlim.WaitAsync();
272+
273+
try
274+
{
275+
if (!_packageMetadata.TryGetValue(packageId, out IEnumerable<IPackageSearchMetadata> metadata))
276+
{
277+
metadata = (await (await _packageMetadataResource.Value).GetMetadataAsync(
278+
packageId,
279+
includePrerelease: true,
280+
includeUnlisted: false,
281+
sourceCacheContext: new SourceCacheContext(),
282+
log: NuGet.Common.NullLogger.Instance,
283+
token: CancellationToken.None)).ToList();
284+
_packageMetadata[packageId] = metadata;
285+
}
286+
287+
return metadata;
288+
}
289+
finally
290+
{
291+
semaphoreSlim.Release();
292+
}
293+
}
294+
295+
private async Task<PackageMetadataResource> InitializeMetadataResourceAsync()
296+
{
297+
var providers = Repository.Provider.GetCoreV3();
298+
var packageSource = new PackageSource(_configuration.NuGetV3Index, "NuGet Source", isEnabled: true);
299+
var sourceRepository = Repository.CreateSource(providers, packageSource, FeedType.Undefined);
300+
return await sourceRepository.GetResourceAsync<PackageMetadataResource>(CancellationToken.None);
125301
}
126302
}
127303
}

‎src/VerifyGitHubVulnerabilities/VerifyGitHubVulnerabilities.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
<Compile Include="Job.cs" />
5050
<Compile Include="Program.cs" />
5151
<Compile Include="Properties\AssemblyInfo.cs" />
52+
<Compile Include="Verify\IPackageVulnerabilitiesVerifier.cs" />
5253
<Compile Include="Verify\PackageVulnerabilitiesVerifier.cs" />
5354
</ItemGroup>
5455
<ItemGroup>

0 commit comments

Comments
 (0)