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

feat: Expose Ubuntu fix status for downstream consumption #407

Merged
merged 26 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d7d0ec2
initial change
skahn007gl May 22, 2024
d37a7d0
Status ingestion added.
skahn007gl May 29, 2024
b6e2928
tests & code cleanup
skahn007gl May 30, 2024
04f2549
chore(deps): bump golang.org/x/tools from 0.20.0 to 0.21.0 (#403)
dependabot[bot] Jun 2, 2024
d27f9cd
Merge branch 'aquasecurity:main' into ubuntu-fix-status
skahn007gl Jun 3, 2024
b17897b
reverting change to ubuntu test data
skahn007gl Jun 4, 2024
39d07a0
lint
skahn007gl Jun 4, 2024
cc52460
Revert name change fo gopls
skahn007gl Jun 10, 2024
ea1e8b8
Compress needed, pending status into FixDeferred.
skahn007gl Jun 10, 2024
7b934dd
Update .gitignore
skahn007gl Jun 20, 2024
afc2b63
removed unneeded ubuntu status
skahn007gl Jun 20, 2024
be70bd0
removing space
skahn007gl Jun 20, 2024
d0e32c0
missed 1 status from targetStatuses
skahn007gl Jun 20, 2024
11edc67
Update pkg/vulnsrc/ubuntu/ubuntu.go
skahn007gl Jun 21, 2024
a0de802
resolved type conflict. Added missing target status
skahn007gl Jun 21, 2024
f6a5b03
fix failing happy path test
skahn007gl Jun 21, 2024
d3a544d
removed needs-triage as it is not needed
skahn007gl Jun 21, 2024
ee6fbae
linting
skahn007gl Jun 21, 2024
15339ea
added test for non fixed status
skahn007gl Jun 25, 2024
9bdfe07
packagename update in tes data
skahn007gl Jun 26, 2024
4aafbcc
Merge branch 'aquasecurity:main' into ubuntu-fix-status
skahn007gl Jul 23, 2024
a488d7f
reapply test fix
skahn007gl Jul 23, 2024
e4f74bd
Update pkg/vulnsrc/ubuntu/ubuntu.go
skahn007gl Aug 20, 2024
20b22f3
var name correction
skahn007gl Aug 21, 2024
a3e59a1
remove ubuntu ignored status
skahn007gl Sep 5, 2024
6bc4582
chore: update a comment
knqyf263 Sep 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion pkg/types/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ var (
//
// In addition to them, Red Hat has "will_not_fix" and "fix_deferred".
// cf. https://access.redhat.com/blogs/product-security/posts/2066793
//
// In addition to them, Ubuntu has "DNE", "ignored", "needed", "pending"
// https://askubuntu.com/a/1509706
Statuses = []string{
"unknown",
"not_affected",
Expand All @@ -29,7 +32,7 @@ const (
StatusAffected
StatusFixed
StatusUnderInvestigation
StatusWillNotFix // Red Hat specific
StatusWillNotFix
StatusFixDeferred
StatusEndOfLife
)
Expand Down
5 changes: 2 additions & 3 deletions pkg/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,8 @@ type Advisory struct {

Arches []string `json:",omitempty"`

// It is filled only when FixedVersion is empty since it is obvious the state is "Fixed" when FixedVersion is not empty.
// e.g. Will not fix and Affected
Status Status `json:"-"`
// This is used to provide the status when a status is known & support by the the vulnsrc
Status Status `json:",omitempty"`

// Trivy DB has "vulnerability" bucket and severities are usually stored in the bucket per a vulnerability ID.
// In some cases, the advisory may have multiple severities depending on the packages.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@
"bionic": {
"Status": "released",
"Note": "1.2.3"
},
"focal": {
"Status": "needs-triage",
"Note": ""
}
}
},
"UpstreamLinks": {}
},
"trusty": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key must be a package name. Isn't trusty a codename?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes it should be. I read xen as xenial, Will change the test data and get it pushed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test data updated and pushed.

"bionic": {
"Status": "deferred"
}
},
"UpstreamLinks": {}
}
}
22 changes: 20 additions & 2 deletions pkg/vulnsrc/ubuntu/ubuntu.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const (
)

var (
targetStatuses = []string{"needed", "deferred", "released"}
targetStatuses = []string{"ignored", "needed", "pending", "deferred", "released"}
skahn007gl marked this conversation as resolved.
Show resolved Hide resolved
UbuntuReleasesMapping = map[string]string{
"precise": "12.04",
"quantal": "12.10",
Expand Down Expand Up @@ -170,8 +170,12 @@ func defaultPut(dbc db.Operation, tx *bolt.Tx, advisory interface{}) error {
}

adv := types.Advisory{}
if status.Status == "released" {
normalised_status := StatusFromUbuntuStatus(status.Status)
skahn007gl marked this conversation as resolved.
Show resolved Hide resolved
if normalised_status == types.StatusFixed {
adv.FixedVersion = status.Note
} else {
// Store the status only if it's unfixed
adv.Status = normalised_status
}
if err := dbc.PutAdvisoryDetail(tx, cve.Candidate, pkgName, []string{platformName}, adv); err != nil {
return xerrors.Errorf("failed to save Ubuntu advisory: %w", err)
Expand Down Expand Up @@ -213,3 +217,17 @@ func SeverityFromPriority(priority string) types.Severity {
return types.SeverityUnknown
}
}

// StatusFromUbuntuStatus normalises Ubuntu status into common Trivy Types
func StatusFromUbuntuStatus(status string) types.Status {
switch status {
case "ignored":
return types.StatusWillNotFix
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took another read of the document.
https://askubuntu.com/questions/1509705/cve-questions-about-status/1509706#1509706

"ignored" support has ended for one reason or another/ end-of-life.

It looks like Ubuntu doesn't determine if the package is affected. So, if I understand correctly, we should not show this type of vulnerability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We consider this an intentional decision that is vendor will not fix, this status is important in our downstream workflow and was included for this reason.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Will not fix" means "this package is affected by this flaw on this platform, but there is currently no intention to fix it." It's a Red Hat definition, but we follow it.
https://access.redhat.com/blogs/product-security/posts/2066793

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knqyf263 Are you happy to proceed with this?
What do you need from me to get this merged?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skahn007gl - it seems that treating ignored as wontfix is the issue here, as ignored/EOL findings are typically not ingested to reduce Trivy DB size. I think we can just skip ingesting ignored, we still have the "notaffected" status to determine if the outcome of analysis is that Ubuntu decided not to fix it. For EOL stuff, we can work that out another way on our end, and treat findings for EOL releases as wontfix if they don't have another status.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trivy is designed to minimize false positives as much as possible. EOL means unknown and is possibly a false positive, which is why we currently do not detect it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While i see the reasoning behind this and do not disagree, I am happy to follow guidance and remove the status for ubuntu.

@knqyf263 let me know if you feel strongly on removing ignored for Ubuntu and i will make the changes.

I would like to highlight the ignored is ingested for Debian in pkg/vulnsrc/debian.go:682

func newStatus(s string) types.Status {
	switch strings.ToLower(s) {
	// "end-of-life" is considered as vulnerable
	// e.g. https://security-tracker.debian.org/tracker/CVE-2022-1488
	case "no-dsa", "unfixed":
		return types.StatusAffected
	case "ignored":
		return types.StatusWillNotFix
	case "postponed":
		return types.StatusFixDeferred
	case "end-of-life":
		return types.StatusEndOfLife
	}
	return types.StatusUnknown
} 

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to highlight the ignored is ingested for Debian in pkg/vulnsrc/debian.go:682

We believe "ignored" in Debian differs from "ignored" in Ubuntu. They use the same term by chance. Please let us know if you find a document in which Debian also uses "ignored" for EOL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @knqyf263, @jhebden-gl
I have removed ignored status as we can make an assumption on wontfix by presence of data (Affected), Deferred status, No fix version.

case "needed", "pending", "deferred":
return types.StatusFixDeferred
case "released":
return types.StatusFixed
default:
return types.StatusUnknown
}
}
54 changes: 54 additions & 0 deletions pkg/vulnsrc/ubuntu/ubuntu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package ubuntu_test
import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/aquasecurity/trivy-db/pkg/types"
"github.com/aquasecurity/trivy-db/pkg/vulnsrc/ubuntu"
"github.com/aquasecurity/trivy-db/pkg/vulnsrc/vulnerability"
Expand Down Expand Up @@ -35,6 +37,12 @@ func TestVulnSrc_Update(t *testing.T) {
FixedVersion: "1.2.3",
},
},
{
Key: []string{"advisory-detail", "CVE-2020-1234", "ubuntu 18.04", "trusty"},
Value: types.Advisory{
Status: types.StatusFixDeferred,
},
},
{
Key: []string{"vulnerability-detail", "CVE-2020-1234", "ubuntu"},
Value: types.VulnerabilityDetail{
Expand All @@ -60,3 +68,49 @@ func TestVulnSrc_Update(t *testing.T) {
})
}
}

func TestUbuntuStatusFromStatus(t *testing.T) {
tests := []struct {
name string
status string
expected types.Status
}{
{
name: "ignored",
status: "ignored",
expected: types.StatusWillNotFix,
},
{
name: "deferred",
status: "deferred",
expected: types.StatusFixDeferred,
},
{
name: "needed",
status: "needed",
expected: types.StatusFixDeferred,
},
{
name: "pending",
status: "pending",
expected: types.StatusFixDeferred,
},
{
name: "released",
status: "released",
expected: types.StatusFixed,
},
{
name: "unknown",
status: "unknown",
expected: types.StatusUnknown,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
actual := ubuntu.StatusFromUbuntuStatus(test.status)
assert.Equal(t, test.expected, actual)
})
}
}