feat(vuln): include pkg identifier on detected vulnerabilities

[juan131]

Description

This PR attempt to ensure we include a package identifier on every detected vulnerability using a generated pURL when there's no package reference.

It deprecates the Results[].Vulnerabilities[].PkgRef (string) field on scan reports in favor of Results[].Vulnerabilities[].PkgIdentifier which is now an object following the struct below:

// PkgIdentifier represents a software identifiers in one of more of the supported formats.
type PkgIdentifier struct {
	// Software identifier in PURL format
	PURL string `json:",omitempty"`
	// Software identifier in CPE format
	CPE string `json:",omitempty"`
}

... and it enhances the detector so pkg identifiers are generated when there's no package reference.

Before :

$ trivy -q i --scanners vuln library/redis:7.2.1-bookworm -f json | jq .Results[].Vulnerabilities[].PkgRef
null
null
null
(...)

After:

$ trivy -q i --scanners vuln library/redis:7.2.1-bookworm -f json | jq .Results[].Vulnerabilities[].PkgIdentifier
{
  "PURL": "pkg:dpkg/[email protected]"
}
{
  "PURL": "pkg:dpkg/bsdutils@1%3A2.38.1-5%2Bb1"
}
{
  "PURL": "pkg:dpkg/[email protected]"
}
(...)

Related issues

• Related to this discussion.

Checklist

• I've read the guidelines for contributing to this repository.
• I've followed the conventions in the PR title.
• I've added tests that prove my fix is effective or that my feature works.
• I've updated the documentation with the relevant information (if needed).
• I've added usage information (if the PR introduces new options)
• I've included a "before" and "after" example to the description (if the PR is a user interface change).

[knqyf263]

Thanks for your great contribution! I think we should add Identifier in Package as well as DetectedVulnerability since it is an attribute of the package. What do you think? It means PURLs should be generated in the analysis part.

trivy/pkg/fanal/types/artifact.go

Lines 64 to 102 in f2a12f5

	type Package struct {
	ID string `json:",omitempty"`
	Name string `json:",omitempty"`
	Version string `json:",omitempty"`
	Release string `json:",omitempty"`
	Epoch int `json:",omitempty"`
	Arch string `json:",omitempty"`
	Dev bool `json:",omitempty"`
	SrcName string `json:",omitempty"`
	SrcVersion string `json:",omitempty"`
	SrcRelease string `json:",omitempty"`
	SrcEpoch int `json:",omitempty"`
	Licenses []string `json:",omitempty"`
	Maintainer string `json:",omitempty"`
	Modularitylabel string `json:",omitempty"` // only for Red Hat based distributions
	BuildInfo *BuildInfo `json:",omitempty"` // only for Red Hat
	Ref string `json:",omitempty"` // identifier which can be used to reference the component elsewhere
	Indirect bool `json:",omitempty"` // this package is direct dependency of the project or not
	// Dependencies of this package
	// Note:　it may have interdependencies, which may lead to infinite loops.
	DependsOn []string `json:",omitempty"`
	Layer Layer `json:",omitempty"`
	// Each package metadata have the file path, while the package from lock files does not have.
	FilePath string `json:",omitempty"`
	// This is required when using SPDX formats. Otherwise, it will be empty.
	Digest digest.Digest `json:",omitempty"`
	// lines from the lock file where the dependency is written
	Locations []Location `json:",omitempty"`
	// Files installed by the package
	InstalledFiles []string `json:",omitempty"`
	}

PURL can be generated here for language-specific packages.

trivy/pkg/fanal/analyzer/language/analyze.go

Lines 119 to 130 in 44656f2

	pkgs = append(pkgs, types.Package{
	ID: lib.ID,
	Name: lib.Name,
	Version: lib.Version,
	Dev: lib.Dev,
	FilePath: libPath,
	Indirect: lib.Indirect,
	Licenses: licenses,
	DependsOn: deps[lib.ID],
	Locations: locs,
	Digest: d,
	})

OS packages may need to generate PURLs in each analyzer.

trivy/pkg/fanal/analyzer/pkg/apk/apk.go

Lines 65 to 67 in 5b2b4ea

	if !pkg.Empty() {
	pkgs = append(pkgs, pkg)
	}

@DmitriyLewen Please let me know if you have any comments.

[DmitriyLewen]

Agree with you @knqyf263
purl/cpe is one of the package parameters.

If the package contains PkgIdentifier, we can simply add it to DetectedVulnerability:
e.g. for alpine packages:

trivy/pkg/detector/ospkg/alpine/alpine.go

Lines 126 to 136 in a1b4744

	vulns = append(vulns, types.DetectedVulnerability{
	VulnerabilityID: adv.VulnerabilityID,
	PkgID: pkg.ID,
	PkgName: pkg.Name,
	InstalledVersion: utils.FormatVersion(pkg),
	FixedVersion: adv.FixedVersion,
	Layer: pkg.Layer,
	PkgRef: pkg.Ref,
	Custom: adv.Custom,
	DataSource: adv.DataSource,
	})

[juan131]

Thanks for your comments @DmitriyLewen @knqyf263 ! I'll update the PR moving PkgIdentifier as part of the Package identity and generate pURL(s) at analysis.

[juan131]

I need advice on how to update pkg/fanal/test/integration/testdata/goldens data automatically to fix integration tests @knqyf263

[DmitriyLewen]

mage test:updateGolden command should update golden files for integration tests.

[juan131]

I'm still finding issues with integration tests and recreating golden images (mage test:updateGolden threw some unauthorized errors). Could you help me with this @DmitriyLewen ?

[DmitriyLewen]

Hello @juan131
I have fixed containerd tests (you may be using macOS or a non-AMD64 processor. In this case, we are skipping containerd tests -

trivy/pkg/fanal/test/integration/containerd_test.go

Line 1 in b6fafa0

//go:build integration && linux

,

trivy/pkg/fanal/test/integration/containerd_test.go

Line 118 in b6fafa0

if runtime.GOARCH != "amd64" {

)

About ClientServer tests:
you need to update rpc package -

trivy/rpc/common/service.proto

Line 104 in b6fafa0

message Vulnerability {

Looks like your changes don't work in client-server mode.

[juan131]

Thanks for implementing the missing changes @DmitriyLewen ! I think with this the PR is ready, am I right?

[DmitriyLewen]

This is a big PR and i realized that I’m already confusing myself 😄

I think the PR is ready. But perhaps this code requires refactoring.

I want to double check this with a clear head.
I'll do it tomorrow or Monday.

[DmitriyLewen]

Hello @juan131
PR looks good.

We merged #5764
Can you merge main branch into this PR and update goldenfiles, please?

[juan131]

Thanks @DmitriyLewen !! PR was updated

[DmitriyLewen]

@juan131 there is conflict.
I'm also confused - GitHub marked go.mod and go.sum files as modified - https://github.com/aquasecurity/trivy/pull/5439/files#diff-33ef32bf6c23acb95f5902d7097b7a1d5128ca061167ec0716715b0b9eeaa5f6R16

[juan131]

Conflict solved @DmitriyLewen

[knqyf263]

I'm slowly starting to work this week and will review this PR shortly.

[knqyf263]

Thanks! I changed the PURL field from string to the PURL struct, but the rest looks good.

[juan131]

Awesome! Thanks so much @knqyf263 @DmitriyLewen