feat: Expose Ubuntu fix status for downstream consumption #407

skahn007gl · 2024-06-04T01:21:44Z

Summary

resolves #408

Trivy Ubuntu advisories provide a FixedVersion when there is a released fix for a package, Affected can be inferred when the advisory is present without a FixedVersion, it does not expose any other status that Canonical use to indicate the status of a fix. This is insufficient to infer a status of ignored, pending or needed as these status show the package is affected and in the process of getting to a fixed version or not if the status is ignored.

This change exposes the Status provided in launchpad advisories without changed existing behaviour to populate the notes field in an advisory with Status:$status

Changes

gitignore:21 :: name change of cache, assets to _cache, _assets to reflect directory name change to support gopls lint ignore declaration of _ prefix on directories.
MakeFile :: Adjusting naming convention of cache & _assets to avoid gopls linter causing max open file errors on MacOS.
pkg/types/status.go :: New status added to support all potential status on launchpad advisories.
pkg/vulnsrc/ubuntu/ubuntu.go :: Expanded target status to capture all provided status's. Adjusted logic to use these status. Added ubuntu status to trivy status normalisation function.
pkg/vulnsrc/ubuntu/ubuntu_test.go :: Added test for status normalisation.

…ity#403) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

CLAassistant · 2024-06-04T01:21:49Z

All committers have signed the CLA.

jhebden-gl

Whilst I'm not a maintainer here, I am aware of the scope of this work and did work on the RHEL changes for fix status, so thought I'd add some review comments to try and help to save some time for the reviewers.

.gitignore

pkg/types/status.go

pkg/vulnsrc/ubuntu/ubuntu.go

pkg/vulnsrc/ubuntu/ubuntu_test.go

skahn007gl · 2024-06-10T03:08:34Z

Whilst I'm not a maintainer here, I am aware of the scope of this work and did work on the RHEL changes for fix status, so thought I'd add some review comments to try and help to save some time for the reviewers.

@jhebden-gl Thanks or the feedback. I have pushed a new change set with your suggested changes

jhebden-gl · 2024-06-19T01:08:24Z

Hi @knqyf263 👋🏻 - I was wondering if you might be able to assist with a review on this PR? We are hoping to use this data downstream. Thank you!

.gitignore

knqyf263 · 2024-06-20T05:15:59Z

pkg/vulnsrc/ubuntu/ubuntu.go

+// StatusFromUbuntuStatus normalises Ubuntu status into common Trivy Types
+func StatusFromUbuntuStatus(status string) types.Status {
+	switch status {
+	case "not-affected", "DNE", "not-vulnerable", "ignored":


I don't think not-affected, DNE and not-vulnerable are "Will not fix". They're "Not affected", and we don't want to save them in the database as it increases the size.

I agree.
I will push a change set for this today.

ignored makes sense to me as it's considered "Will not fix".

@knqyf263 I have pushed all the tidy up changes. Please re-review when you have a chance.
Thanks

Co-authored-by: Teppei Fukuda <knqyf263@gmail.com>

pkg/vulnsrc/ubuntu/ubuntu.go

Co-authored-by: Teppei Fukuda <knqyf263@gmail.com>

pkg/vulnsrc/ubuntu/ubuntu.go

pkg/vulnsrc/ubuntu/ubuntu_test.go

DmitriyLewen

fix linter error, please
https://github.com/aquasecurity/trivy-db/actions/runs/9608945100/job/26502649422?pr=407#step:4:26

skahn007gl · 2024-06-21T12:10:24Z

fix linter error, please https://github.com/aquasecurity/trivy-db/actions/runs/9608945100/job/26502649422?pr=407#step:4:26

Done :)

knqyf263 · 2024-06-24T07:25:29Z

@skahn007gl Thanks for updating. At last, could you add a test with a non-fixed status? Adding a new advisory or updating the existing one.

knqyf263 · 2024-06-24T08:00:11Z

@skahn007gl Also, could you open a PR with this change in Trivy? It's better to test this change with Trivy before merging this PR.

Add Status here and update unit tests
https://github.com/aquasecurity/trivy/blob/4369a19af771f81df141530bacdc8680e7120ac7/pkg/detector/ospkg/ubuntu/ubuntu.go#L100-L110
Update integration tests
https://github.com/aquasecurity/trivy/blob/bbaf5952bc8059c537401bd4364e019eecd16c9a/integration/standalone_tar_test.go#L217-L233
Update documentation in two places
https://github.com/aquasecurity/trivy/blob/648ead9553eb2cbeac90e3ef7330a70c352255ac/docs/docs/configuration/filtering.md#by-status
https://github.com/aquasecurity/trivy/blob/648ead9553eb2cbeac90e3ef7330a70c352255ac/docs/docs/coverage/os/ubuntu.md#status

Tips: Use replace until this PR gets merged.

knqyf263 · 2024-06-26T03:21:37Z

pkg/vulnsrc/ubuntu/testdata/vuln-list/ubuntu/CVE-2020-1234.json

-        "Note": ""
+      }
+    },
+    "trusty": {


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

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

test data updated and pushed.

skahn007gl · 2024-06-26T08:24:35Z

@skahn007gl Also, could you open a PR with this change in Trivy? It's better to test this change with Trivy before merging this PR.

Add Status here and update unit tests
https://github.com/aquasecurity/trivy/blob/4369a19af771f81df141530bacdc8680e7120ac7/pkg/detector/ospkg/ubuntu/ubuntu.go#L100-L110

Update integration tests
https://github.com/aquasecurity/trivy/blob/bbaf5952bc8059c537401bd4364e019eecd16c9a/integration/standalone_tar_test.go#L217-L233

Update documentation in two places
https://github.com/aquasecurity/trivy/blob/648ead9553eb2cbeac90e3ef7330a70c352255ac/docs/docs/configuration/filtering.md#by-status
https://github.com/aquasecurity/trivy/blob/648ead9553eb2cbeac90e3ef7330a70c352255ac/docs/docs/coverage/os/ubuntu.md#status

Tips: Use replace until this PR gets merged.

Hey @knqyf263 I am unsure what needs to change in the existing integration tests.
Is my understanding of the tests correct that it tests 2 scenarios?

Include all findings
Include findings with fixes only

This includes the status field in the .golden test data.

I have made the other changes and raised a PR to Trivy :)
Trivy PR7020

knqyf263 · 2024-06-26T08:43:45Z

There is a DB fixture here.
https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/ubuntu.yaml

We need to add a status here and ensure the status appears in the result.

This bucket must also be updated if you want to add a new vulnerability.
https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/vulnerability.yaml

When adding a new bucket for Ubuntu, that needs to be added here.
https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/data-source.yaml#L137-L141

skahn007gl · 2024-06-27T08:21:44Z

There is a DB fixture here. https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/ubuntu.yaml

We need to add a status here and ensure the status appears in the result.

This bucket must also be updated if you want to add a new vulnerability. https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/vulnerability.yaml

When adding a new bucket for Ubuntu, that needs to be added here. https://github.com/aquasecurity/trivy/blob/8d618e48a2f1b60c2e4c49cdd9deb8eb45c972b0/integration/testdata/fixtures/db/data-source.yaml#L137-L141

@knqyf263 How are the .golden files generated/created?

I have updated the DB fixture with an existing CVE for ubuntu 18.04 that would have a non fixed status, but i'm unable to trigger a failure on the existing ubuntu18.04 integration tests.

- bucket: ubuntu 18.04
  pairs:
    - bucket: libspring-java
      pairs:
        - key: CVE-2022-22965
          value:
            Status: deferred

knqyf263 · 2024-06-27T08:25:30Z

deferred is not our status.

trivy-db/pkg/types/status.go

Lines 17 to 26 in 9bdfe07

    
           Statuses = []string{ 
        
           	"unknown", 
        
           	"not_affected", 
        
           	"affected", 
        
           	"fixed", 
        
           	"under_investigation", 
        
           	"will_not_fix", 
        
           	"fix_deferred", 
        
           	"end_of_life", 
        
           }

skahn007gl · 2024-06-27T08:36:55Z

deferred is not our status.

trivy-db/pkg/types/status.go

Lines 17 to 26 in 9bdfe07

Statuses = []string{

"unknown",

"not_affected",

"affected",

"fixed",

"under_investigation",

"will_not_fix",

"fix_deferred",

"end_of_life",

}

Corrected to fix_deferred, but still no failure in the test run.

I am assuming I need to update ubuntu-1804.json.golden to update the expected results for the tests linked https://github.com/aquasecurity/trivy/blob/bbaf5952bc8059c537401bd4364e019eecd16c9a/integration/standalone_tar_test.go#L217-L233

If this is the case, How are the .golden files generated/created?

Thanks for the help and guidance on this as I work through it.

knqyf263 · 2024-06-27T08:48:41Z

I am assuming I need to update ubuntu-1804.json.golden to update the expected results for the tests linked https://github.com/aquasecurity/trivy/blob/bbaf5952bc8059c537401bd4364e019eecd16c9a/integration/standalone_tar_test.go#L217-L233

No, the test should fail due to a mismatch with the golden file. You need to update the golden file once you confirm the test fails. Did you update all the buckets I listed above? It may be easier for us to debug if you push the change to your PR in Trivy.

skahn007gl · 2024-06-28T03:21:19Z

I am assuming I need to update ubuntu-1804.json.golden to update the expected results for the tests linked https://github.com/aquasecurity/trivy/blob/bbaf5952bc8059c537401bd4364e019eecd16c9a/integration/standalone_tar_test.go#L217-L233

No, the test should fail due to a mismatch with the golden file. You need to update the golden file once you confirm the test fails. Did you update all the buckets I listed above? It may be easier for us to debug if you push the change to your PR in Trivy.

Thanks for confirming, I agree it will be easier to debug if i share my changes. I will update the PR shortly.

skahn007gl · 2024-06-28T04:28:56Z

@knqyf263 I have pushed the updated DB fixture and OS bucket.
I used an existing vulnerability.

Note the changes do not include the use of replace in go.mod, but i do use it locally.
e.g: replace github.com/aquasecurity/trivy-db => /xxx/yyy/zzz/trivy-db

knqyf263 · 2024-06-28T04:35:11Z

You added Ubuntu 21.10, but the scanned image is not Ubuntu 21.10. The data is not used.

skahn007gl · 2024-06-28T05:22:08Z

You added Ubuntu 21.10, but the scanned image is not Ubuntu 21.10. The data is not used.

Adjusted to ubuntu 16.04 as there is an image under integration/testdata/fixtures/images
Added test for ubuntu 16.04 into integration/standalone_tar_test.go
note: Test does not use the correct json.golden as there is no existing one for ubuntu 16.04

Successfully generated a failed test result.

I am running the tests via mage test:integration(mage is new to me) or go test -timeout 15m -v -tags=integration ./integration/...
But i do not see any test artefacts generated, only pass/fail stat's.

Is there a way to generate the output from the comparison?.

I have pushed my updates.

skahn007gl · 2024-07-11T07:58:42Z

Hey @knqyf263
Sorry for the delays on my side, I've had some personal life stuff crop up.

I have simplified the integration test to use as much as existing as possible to understand how these tests hang together, I believe I am still missing something here.

Added a CVE to the db fixture
Added the CVE to the vulnerability fixture
As i re-used ubuntu 18.04, I did not add a new bucket to data-source

When i run the integration test's, there is no test failure for ubuntu-1804

I added some dummy data to to ubuntu-1804.json.golden This induces an error, but the error indicates it did not expect the new vulnerability.

	-   Vulnerabilities: ([]types.DetectedVulnerability) (len=6) {
	+   Vulnerabilities: ([]types.DetectedVulnerability) (len=5) {

pkg/vulnsrc/ubuntu/ubuntu.go

Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>

DmitriyLewen

LGTM

knqyf263 · 2024-08-26T02:23:48Z

pkg/vulnsrc/ubuntu/ubuntu.go

+func StatusFromUbuntuStatus(status string) types.Status {
+	switch status {
+	case "ignored":
+		return types.StatusWillNotFix


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.

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.

"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

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

@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.

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.

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 }

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.

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.

Signed-off-by: knqyf263 <knqyf263@gmail.com>

…)" This reverts commit 42b851c.

skahn007gl and others added 7 commits May 23, 2024 07:20

initial change

d7d0ec2

Status ingestion added.

d37a7d0

tests & code cleanup

b6e2928

chore(deps): bump golang.org/x/tools from 0.20.0 to 0.21.0 (aquasecur…

04f2549

…ity#403) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Merge branch 'aquasecurity:main' into ubuntu-fix-status

d27f9cd

reverting change to ubuntu test data

b17897b

lint

39d07a0

skahn007gl requested a review from knqyf263 as a code owner June 4, 2024 01:21

skahn007gl mentioned this pull request Jun 4, 2024

feat: Expose Ubuntu fix status for downstream consumtion. #408

Closed

jhebden-gl suggested changes Jun 4, 2024

View reviewed changes

skahn007gl added 2 commits June 10, 2024 15:00

Revert name change fo gopls

cc52460

Compress needed, pending status into FixDeferred.

ea1e8b8

knqyf263 reviewed Jun 20, 2024

View reviewed changes

skahn007gl and others added 4 commits June 20, 2024 17:48

Update .gitignore

7b934dd

Co-authored-by: Teppei Fukuda <knqyf263@gmail.com>

removed unneeded ubuntu status

afc2b63

removing space

be70bd0

missed 1 status from targetStatuses

d0e32c0

knqyf263 reviewed Jun 21, 2024

View reviewed changes

pkg/vulnsrc/ubuntu/ubuntu.go Outdated Show resolved Hide resolved

knqyf263 requested a review from DmitriyLewen June 21, 2024 02:42

Update pkg/vulnsrc/ubuntu/ubuntu.go

11edc67

Co-authored-by: Teppei Fukuda <knqyf263@gmail.com>

DmitriyLewen reviewed Jun 21, 2024

View reviewed changes

pkg/vulnsrc/ubuntu/ubuntu.go Outdated Show resolved Hide resolved

pkg/vulnsrc/ubuntu/ubuntu.go Outdated Show resolved Hide resolved

pkg/vulnsrc/ubuntu/ubuntu_test.go Outdated Show resolved Hide resolved

skahn007gl added 2 commits June 21, 2024 13:43

resolved type conflict. Added missing target status

a0de802

fix failing happy path test

f6a5b03

DmitriyLewen reviewed Jun 21, 2024

View reviewed changes

skahn007gl added 2 commits June 21, 2024 14:30

removed needs-triage as it is not needed

d3a544d

linting

ee6fbae

added test for non fixed status

15339ea

knqyf263 reviewed Jun 26, 2024

View reviewed changes

packagename update in tes data

9bdfe07

skahn007gl mentioned this pull request Jun 26, 2024

feat(ubuntu): Expose Ubuntu fix status. aquasecurity/trivy#7020

Open

5 tasks

skahn007gl and others added 2 commits July 23, 2024 15:35

Merge branch 'aquasecurity:main' into ubuntu-fix-status

4aafbcc

reapply test fix

a488d7f

DmitriyLewen reviewed Aug 20, 2024

View reviewed changes

pkg/vulnsrc/ubuntu/ubuntu.go Outdated Show resolved Hide resolved

skahn007gl and others added 2 commits August 20, 2024 14:20

Update pkg/vulnsrc/ubuntu/ubuntu.go

e4f74bd

Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>

var name correction

20b22f3

DmitriyLewen approved these changes Aug 21, 2024

View reviewed changes

knqyf263 reviewed Aug 26, 2024

View reviewed changes

skahn007gl and others added 2 commits September 5, 2024 12:46

remove ubuntu ignored status

a3e59a1

chore: update a comment

6bc4582

Signed-off-by: knqyf263 <knqyf263@gmail.com>

knqyf263 approved these changes Sep 6, 2024

View reviewed changes

knqyf263 merged commit 42b851c into aquasecurity:main Sep 6, 2024
2 checks passed

knqyf263 added a commit that referenced this pull request Sep 10, 2024

Revert "feat: Expose Ubuntu fix status for downstream consumption (#407…

3bab6bf

…)" This reverts commit 42b851c.

knqyf263 mentioned this pull request Sep 10, 2024

Revert "feat: Expose Ubuntu fix status for downstream consumption" #438

Merged

feat: Expose Ubuntu fix status for downstream consumption #407

feat: Expose Ubuntu fix status for downstream consumption #407

Conversation

skahn007gl commented Jun 4, 2024 • edited Loading

Summary

Changes

CLAassistant commented Jun 4, 2024 • edited Loading

jhebden-gl left a comment

Choose a reason for hiding this comment

skahn007gl commented Jun 10, 2024

jhebden-gl commented Jun 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DmitriyLewen left a comment

Choose a reason for hiding this comment

skahn007gl commented Jun 21, 2024

knqyf263 commented Jun 24, 2024

knqyf263 commented Jun 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skahn007gl commented Jun 26, 2024

knqyf263 commented Jun 26, 2024 • edited Loading

skahn007gl commented Jun 27, 2024 • edited Loading

knqyf263 commented Jun 27, 2024

skahn007gl commented Jun 27, 2024

knqyf263 commented Jun 27, 2024 • edited Loading

skahn007gl commented Jun 28, 2024

skahn007gl commented Jun 28, 2024

knqyf263 commented Jun 28, 2024

skahn007gl commented Jun 28, 2024

skahn007gl commented Jul 11, 2024

DmitriyLewen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skahn007gl commented Jun 4, 2024 •

edited

Loading

CLAassistant commented Jun 4, 2024 •

edited

Loading

knqyf263 commented Jun 26, 2024 •

edited

Loading

skahn007gl commented Jun 27, 2024 •

edited

Loading

knqyf263 commented Jun 27, 2024 •

edited

Loading