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

NuGet Support #681

Closed
sean-redmond opened this issue Oct 12, 2020 · 14 comments
Closed

NuGet Support #681

sean-redmond opened this issue Oct 12, 2020 · 14 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@sean-redmond
Copy link

I am using trivy and really like the tool, I would love to see support for checking NuGet packages for .net core containers.

@sean-redmond sean-redmond added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 12, 2020
@knqyf263
Copy link
Collaborator

It sounds nice.

  1. Implement a new parser of packages.lock.json in go-dep-parser
  2. Add a new analyzer like other package managers in fanal
  3. Implement a new detector in trivy at last

@knqyf263 knqyf263 added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Oct 14, 2020
@Johannestegner
Copy link
Contributor

Johannestegner commented Oct 14, 2020

I'll gladly take a look at this. Will set up a PR draft in go-dep-parser as soon as I have something started on that part if noone else have done it before ;)


Edit:
Started here: aquasecurity/go-dep-parser#14

@Johannestegner
Copy link
Contributor

@knqyf263 , I have a question:

NuGet does not create a lock file by default, it's more of an opt-in thing. The default dependency file is basically the csproj file, with the PackageReference xml node, while if activating the lock functionality, a packages.lock.json file is created.

Should the parser only look for the packages.lock.json file, or should it use the PackageReference nodes as a fallback?

@knqyf263
Copy link
Collaborator

Hi @Johannestegner, thank you for the quick action! I'm not familiar with NuGet yet. Does PackageReference include the exact version or just a package name?

@Johannestegner
Copy link
Contributor

Hi there!

Well, the PackageReference contains a package name, some extra data (both as props and sub-nodes) and a version property which can be a "floating version" (quite annoying yes!). I think that the packages.lock.json file is good to have as a primary target (like for example npm's package-lock.json) while the .csproj's PackageReference entries might be a good idea to parse too (if none-lock files should be parsed, it basically works as the package.json file for npm).

Worth noting is that the .csproj file is a XML file, hehe.

@knqyf263
Copy link
Collaborator

I see. Let's start with package-lock.json as a first step. Then, we can support a .csproj file. But my concern is that we might need to communicate with the repository to determine the exact version. It is like reimplementing of NuGet partially. I'm not sure how hard it is.

@Johannestegner
Copy link
Contributor

It is like reimplementing of NuGet partially. I'm not sure how hard it is.

Aye, was basically what I was thinking too. And honestly, I would think that if someone scans their code for vulnerabilities, it's quite likely that they lock their versions anyway :)

@knqyf263
Copy link
Collaborator

Or, does NuGet have a metadata file for an installed package like *.gemspec in RubyGems? If so, we can look for all files with such a suffix and parse those files.

@Johannestegner
Copy link
Contributor

When it is compiled, is have a Project.deps.json which contains all dependencies, but that would not work on a Repository or a none-compiled application, only when compiled and all debug data is left.
So quite uncertain file that is.

@sean-redmond, which file would you expect that the scanner scanned in a netcore project?

@sean-redmond
Copy link
Author

Hey @Johannestegner - Thanks for taking a look at this feature request.

The .csproj file is XML but can be useful if trivy is used maybe with a flag such as fs to scan an uncompiled repo, a sample of what it has is looks like this:

<PackageReference Include="Serilog" Version="2.9.0" />

Its worth pointing out however the Version can be set to a value of * meaning latest, or something like 2.9.* is also valid. This file can also contain package references that are not just NuGet e.g a local Package Reference that maybe just need to ignore.

In a compiled project (maybe when fs flag is not used?) I would look at Project.deps.json that contains json with versions such as:

"log4net": "2.0.8"

From what I can tell the Project.deps.json can also contain references other than just NuGet, so you maybe able to drop any with a Type: or project

@davidfowl Would anyone from Microsoft be able to offer any further input on the above?

@Johannestegner
Copy link
Contributor

Hi @sean-redmond ! Thanks for the response, and happy to have the chance to work on trivy, I use it a lot and have wanted to contribute for some time now, hehee.

Looking at the other analyzers in the project, most (if not all) of them uses the lock files for scanning. This is probably (I'm guessing here) partially because the lock files have specific versions (not floating versions but exact ones) in them making it a whole lot easier to use in a project like this.
The lockfile in a nuget project does not have a lockfile by default (which I kinda wonder why it doesnt!) but is an opt-in option which is kinda hard to figure out if one doesn't know about it already.

The Project.deps.json file would also be an option, as it does contain specific versions and is quite easy to parse, while, that would only work for a compiled project.

My general thought when it comes to this is that it would be a good idea to search for packages.lock.json as a primary target, but also look for the *.deps.json file aswell. It's not a huge difference to parse either way.

What do you think about this @knqyf263?

@Johannestegner
Copy link
Contributor

There is another discussion going on in one of the PR's, that I just wanted to add a reference to here so (to make it easier to navigate between the different problems with the implementation! hehe): #686 (comment)

In short: Legacy packages can have a versioning which looks like: <major>.<minor>.<build>.<revision> while newer follow SEMVER-2.0.

@knqyf263
Copy link
Collaborator

My general thought when it comes to this is that it would be a good idea to search for packages.lock.json as a primary target, but also look for the *.deps.json file aswell. It's not a huge difference to parse either way.

I'm sorry for the late reply. I agree with you. packages.lock.json looks fine as a primary target.

@knqyf263
Copy link
Collaborator

knqyf263 commented Feb 7, 2021

#686 supported NuGet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants