Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable NuGet Audit in .NET repositories #15019

Open
10 of 29 tasks
ViktorHofer opened this issue Aug 21, 2024 · 11 comments
Open
10 of 29 tasks

Enable NuGet Audit in .NET repositories #15019

ViktorHofer opened this issue Aug 21, 2024 · 11 comments

Comments

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 21, 2024

NuGet Audit docs: https://devblogs.microsoft.com/nuget/nugetaudit-2-0-elevating-security-and-trust-in-package-management/ & https://learn.microsoft.com/en-us/nuget/concepts/auditing-packages

NuGet Audit flags vulnerable dependencies at restore and build time so that we don’t have to exclusively rely on post-build scanners like Component Governance. NuGet Audit is part of the .NET 8 and .NET 9 SDKs and now enabled by default.

NuGetAuditMode defaulted to direct when it was introduced in the .NET 8.0.100 SDK and VS 17.8. In .NET 9.0.100 SDK and VS 17.12 the default changed to all.

Our .NET repositories need to opt into that security feature as we don’t use nuget.org as our package database. Aside from resolving potential vulnerable packages in the build, all that's needed to turn it on is: #15018

Requirement

This depends on a NuGet feature that got added with the .NET 9 Preview 7 SDK. If your repository doesn't yet use that, that's fine and enabling NuGetAudit can be revisited at a later point.

Status

List below are repositories that are part of the VMR. The list can be extended.

cc @zivkan @ericstj @JonDouglas

@ViktorHofer ViktorHofer mentioned this issue Aug 21, 2024
1 task
@zivkan
Copy link
Member

zivkan commented Aug 21, 2024

NuGet.Client added auditSources, and therefore will get NuGetAudit warnings during restore, at the beginning of the month: https://github.com/NuGet/NuGet.Client/pull/5939/files#diff-b54ee602750f150b3d4986e0334cc3ec901d47510560e26e792e8d294ab6f37a

(I also added the 4 NU codes to WarningsNotAsErrors to prevent it from failing CI builds, or blocking people from working on their features)

ViktorHofer added a commit to dotnet/source-build-reference-packages that referenced this issue Aug 26, 2024
ViktorHofer added a commit to dotnet/templating that referenced this issue Aug 27, 2024
Contributes to dotnet/arcade#15019

Also remove the unnecessary `SystemFormatsAsn1Version` property.
ViktorHofer added a commit to dotnet/deployment-tools that referenced this issue Aug 27, 2024
Contributes to dotnet/arcade#15019

- Remove unnecessary package source feeds
- Move version properties into D.Packages.props
  as those aren't auto-updated.
- Enable NuGetAudit and make vulnerabilities
  to only fail the official build.
ViktorHofer added a commit to dotnet/symreader that referenced this issue Aug 27, 2024
ViktorHofer added a commit to dotnet/deployment-tools that referenced this issue Aug 27, 2024
Contributes to dotnet/arcade#15019

- Remove unnecessary package source feeds
- Move version properties into D.Packages.props
  as those aren't auto-updated.
- Enable NuGetAudit and make vulnerabilities
  to only fail the official build.
ViktorHofer added a commit to dotnet/templating that referenced this issue Aug 27, 2024
* Enable NuGetAudit

Contributes to dotnet/arcade#15019

Also remove the unnecessary `SystemFormatsAsn1Version` property.

* Update NuGet.config
ViktorHofer added a commit to dotnet/symreader that referenced this issue Aug 27, 2024
* Enable Central Package Management and NuGetAudit

Contributes to dotnet/arcade#15019

* Delete eng/SourceBuildPrebuiltBaseline.xml

* Don't hardcode package relative path
@ericstj
Copy link
Member

ericstj commented Sep 10, 2024

@mmitche @jaredpar @aortiz-msft - how do you think we should drive enforcement of this?

@jaredpar suggested forcing folks to emit a binlog then presumably adding some validation somewhere in the common templates or scripts to ensure they ran the build with audit enabled. Seems possible but feels like a lot of things to line up just to enforce this in the timeline we're trying to drive it.

I was thinking we could add a check in arcade for this. @aortiz-msft @zivkan suppose we wanted Arcade to enforce this, could we set a property that would make NuGet enforce that, or could we observe if NuGet ran with an audit source? If we did so, then we could make arcade check for that and emit an error if it's absent.

@mmitche
Copy link
Member

mmitche commented Sep 10, 2024

Binlog checking seems rather fragile and I don't love the idea of adding additional steps to builds just to do this checking. Is it possible to detect whether NuGet Audit is enabled via msbuild targets? We have plenty of injection points in arcade. What properties can we check?

@mmitche
Copy link
Member

mmitche commented Sep 10, 2024

I checked some binlogs from before and after Viktor's change to arcade to enable auditing. There's not much to go on. No real indication that actual auditing happened. The thing is that auditing is enabled in both, just only functional in the one with nuget.org as part of the audit sources. The only binlog entry differentiating the two is that searching for "nuget.org" shows a CACHE entry for nuget.org's vulnerability endpoint if auditing is working. But I wouldn't base detection on that.

Instead, it's probably better for arcade to include a target that reads the nuget.config file and verifies the audit sources are available. @zivkan @aortiz-msft Are the audit sources controllable with an itemgroup, or only through nuget.config?

@jaredpar
Copy link
Member

suggested forcing folks to emit a binlog then presumably adding some validation somewhere in the common templates or scripts to ensure they ran the build with audit enabled.

I think that if pipelines / actions published binlogs to a well known location there is a lot of standard analysis that we could do. I understand it's a bit of cost initially but need to pay that at some point.

Instead, it's probably better for arcade to include a target that reads the nuget.config file and verifies the audit sources are available.

That approach though limits our ability to say look at what warnings are suppressed, how globally are they suppressed, etc ...

@zivkan
Copy link
Member

zivkan commented Sep 10, 2024

suppose we wanted Arcade to enforce this, could we set a property that would make NuGet enforce that, or could we observe if NuGet ran with an audit source? If we did so, then we could make arcade check for that and emit an error if it's absent.

When <auditSources> is not empty, but a vulnerability database couldn't be downloaded, NuGet will raise a NU1905 warning. This can be elevated to an error. However, NuGetAudit will not run if the MSBuild property NuGetAudit is set to false. So, you could enforce by 1. checking that nuget.config has at least one audit source (no easy way, needs xml parsing, or use dotnet nuget config get all, and parse the output) and 2. make sure NuGetAudit isn't set to false (dotnet sln list and dotnet msbuild -getProperty:NuGetAudit, or search all project.asset.json files for $.project.restore.restoreAuditProperties.enableAudit)

Are the audit sources controllable with an itemgroup, or only through nuget.config?

Only through nuget.config. We have a request to make it available via msbuild, but it has zero upvotes so far.

@mmitche
Copy link
Member

mmitche commented Sep 10, 2024

Arcade can check whether NuGetAudit = false (it isn't). We would need to implement the additional check on the nuget.config. I don't think the binlog is the way to go here as there was no obvious way to say auditing happened. For instance, the list of audit sources is not even outputted into the binlog.

We could go in the direction of the binlog being more checkable, but IMO:

  • I don't think we should rely on the existence of the binlog for auditing. Some repos turn this off specifically (aspnetcore) for perf purposes. It seems pretty fragile to me.
  • If we're making changes to NuGet/MSBuild targets to make auditing behavior more detectable, we should just make those changes in such a way that we can detect desired behavior during the build, rather than after.

@jaredpar
Copy link
Member

I don't think we should rely on the existence of the binlog for auditing. Some repos turn this off specifically (aspnetcore) for perf purposes. It seems pretty fragile to me.

You can argue "they can turn it off" for any option here. The question is whether you can detect the situation or not. The abscence of a published file is pretty easy to detect 😄

jaredpar added a commit to jaredpar/roslyn that referenced this issue Sep 11, 2024
This enables NuGet audit using the audit sources from nuget.org. This is
the standard that is being pushed into all of our repositories. The end
result is that we will get early warning on CVE and won't need to wait
until CG scans on our scheduled builds reveal them.

I've set this up so that it warns not errors. That is deliberate because
the audit warnings are asynchronous. Setting them up as errors would
mean that builds, which were passing one minute, could start failing the
next t. A warning here feels like the right balance but we can
experiment and see how it goes.

Related dotnet/arcade#15019
@ericstj
Copy link
Member

ericstj commented Sep 12, 2024

Could we force a layered NuGet.config with the audit source in official builds? NuGet.config's are all about layering.

@mmitche
Copy link
Member

mmitche commented Sep 12, 2024

I don't think layering would work (not sure where we would put the layer, but we could modify Maestro to augment the nuget.config. It already can do so for other things. Not sure whether all repos would love that approach, though.

@AndriySvyryd
Copy link
Member

There should also be an option for arcade to set NuGetAudit to false when the branch is locked and a release is underway. Since it can start failing at any point and delay the builds.
Setting WarningsNotAsErrors is not enough, the build will still fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants