From 3e0aa002427fa3cd763478cd6c0a4c8f612bfffc Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Fri, 26 Jan 2024 09:18:11 -0500 Subject: [PATCH] Fix matching when RPM modularity is a factor (#1679) * allow for RPM modularity to be optional Signed-off-by: Alex Goodman * use latest syft from main Signed-off-by: Alex Goodman * bump syft Signed-off-by: Alex Goodman * remove lint ignores for CPEs Signed-off-by: Alex Goodman * update snapshot tests Signed-off-by: Alex Goodman * update tests Signed-off-by: Alex Goodman * fix: treat oraclelinux default appstream rpm modularity as missing for now For oraclelinux, the default stream of an installed appstream package does not currently set the MODULARITYLABEL property in the rpm metadata; however, in their advisory data they do specify modularity information, so this ends up in a case where the vuln entries have modularity but the packages coming from the sbom won't, so for now we need to treat the constraint as satisfied when the modularity label from an oraclelinux package is "". Signed-off-by: Weston Steimel * test: add new appstream images to quality gate and bump labels Signed-off-by: Weston Steimel * chore: bump quality gate labels Signed-off-by: Weston Steimel --------- Signed-off-by: Alex Goodman Signed-off-by: Weston Steimel Co-authored-by: Weston Steimel --- go.mod | 3 +- go.sum | 8 +-- grype/cpe/cpe.go | 4 +- grype/matcher/rpm/matcher_test.go | 8 ++- grype/pkg/package_test.go | 8 ++- .../pkg/qualifier/rpmmodularity/qualifier.go | 30 ++++++-- .../qualifier/rpmmodularity/qualifier_test.go | 72 +++++++++++++++++-- grype/pkg/rpm_metadata.go | 4 +- .../snapshot/TestJsonDirsPresenter.golden | 2 +- .../snapshot/TestJsonImgsPresenter.golden | 2 +- test/quality/.yardstick.yaml | 2 + test/quality/vulnerability-match-labels | 2 +- 12 files changed, 117 insertions(+), 28 deletions(-) diff --git a/go.mod b/go.mod index ce0dc6ecda7..5075f06bb6e 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/anchore/go-version v1.2.2-0.20210903204242-51efa5b487c4 github.com/anchore/packageurl-go v0.1.1-0.20230104203445-02e0a6721501 github.com/anchore/stereoscope v0.0.0-20240118133533-eb656fc71793 - github.com/anchore/syft v0.101.1 + github.com/anchore/syft v0.101.2-0.20240125140035-a32b8d7fc62d github.com/aquasecurity/go-pep440-version v0.0.0-20210121094942-22b2f8951d46 github.com/bmatcuk/doublestar/v2 v2.0.4 github.com/charmbracelet/bubbletea v0.25.0 @@ -233,7 +233,6 @@ require ( go.opentelemetry.io/otel/metric v1.19.0 // indirect go.opentelemetry.io/otel/trace v1.19.0 // indirect go.uber.org/atomic v1.9.0 // indirect - go.uber.org/goleak v1.2.0 // indirect go.uber.org/multierr v1.9.0 // indirect golang.org/x/crypto v0.18.0 // indirect golang.org/x/mod v0.14.0 // indirect diff --git a/go.sum b/go.sum index 695f65c2da3..22f93a822ff 100644 --- a/go.sum +++ b/go.sum @@ -251,8 +251,8 @@ github.com/anchore/packageurl-go v0.1.1-0.20230104203445-02e0a6721501 h1:AV7qjwM github.com/anchore/packageurl-go v0.1.1-0.20230104203445-02e0a6721501/go.mod h1:Blo6OgJNiYF41ufcgHKkbCKF2MDOMlrqhXv/ij6ocR4= github.com/anchore/stereoscope v0.0.0-20240118133533-eb656fc71793 h1:wji+qdjsV7ooolBwb3faVZnEK3WtY/kcT5473kxVZS4= github.com/anchore/stereoscope v0.0.0-20240118133533-eb656fc71793/go.mod h1:IylG7ofLoUKHwS1XDF6rPhOmaE3GgpAgsMdvvYfooTU= -github.com/anchore/syft v0.101.1 h1:PTh7XBdtXq3BYhuPz67rrC6AFPZxC1Rt8jgqv7Z75rA= -github.com/anchore/syft v0.101.1/go.mod h1:6rbrRWQN16TFENxXG1uFQOh9RCIp/UHJqPAJnHSKhjQ= +github.com/anchore/syft v0.101.2-0.20240125140035-a32b8d7fc62d h1:a0fxiaNnKAyGwFc5Cnc8bUmf2DlbPVjUYNZgOU5kfbc= +github.com/anchore/syft v0.101.2-0.20240125140035-a32b8d7fc62d/go.mod h1:H676A/X1oznNrYS6fv8XvxRn8l0s7vOLP97ta+jHOu0= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/andybalholm/brotli v1.0.1/go.mod h1:loMXtMfwqflxFJPmdbJO0a3KNoPuLBgiu3qAvBg8x/Y= github.com/andybalholm/brotli v1.0.4 h1:V7DdXeJtZscaqfNuAdSRuRFzuiKlHSC/Zh3zl9qY3JY= @@ -1070,8 +1070,8 @@ go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= -go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= -go.uber.org/goleak v1.2.0/go.mod h1:XJYK+MuIchqpmGmUSAzotztawfKvYLUIgg7guXrwVUo= +go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI= go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ= diff --git a/grype/cpe/cpe.go b/grype/cpe/cpe.go index 66ee2ad1dfc..7e7874b9427 100644 --- a/grype/cpe/cpe.go +++ b/grype/cpe/cpe.go @@ -23,9 +23,9 @@ func NewSlice(cpeStrs ...string) ([]cpe.CPE, error) { func MatchWithoutVersion(c cpe.CPE, candidates []cpe.CPE) []cpe.CPE { matches := make([]cpe.CPE, 0) - a := wfn.Attributes(c) // nolint:unconvert // TODO: remove nolint when syft upgrade in grype + a := wfn.Attributes(c) for _, candidate := range candidates { - canCopy := wfn.Attributes(candidate) // nolint:unconvert // TODO: remove nolint when syft upgrade in grype + canCopy := wfn.Attributes(candidate) if a.MatchWithoutVersion(&canCopy) { matches = append(matches, candidate) } diff --git a/grype/matcher/rpm/matcher_test.go b/grype/matcher/rpm/matcher_test.go index 0c0e9c1284d..f15a5dafb0e 100644 --- a/grype/matcher/rpm/matcher_test.go +++ b/grype/matcher/rpm/matcher_test.go @@ -253,7 +253,7 @@ func TestMatcherRpm(t *testing.T) { Version: "0.1", Type: syftPkg.RpmPkg, Metadata: pkg.RpmMetadata{ - ModularityLabel: "containertools:3:1234:5678", + ModularityLabel: strRef("containertools:3:1234:5678"), }, }, setup: func() (vulnerability.Provider, *distro.Distro, Matcher) { @@ -280,7 +280,7 @@ func TestMatcherRpm(t *testing.T) { Version: "0.1", Type: syftPkg.RpmPkg, Metadata: pkg.RpmMetadata{ - ModularityLabel: "containertools:1:abc:123", + ModularityLabel: strRef("containertools:1:abc:123"), }, }, setup: func() (vulnerability.Provider, *distro.Distro, Matcher) { @@ -381,3 +381,7 @@ func Test_addZeroEpicIfApplicable(t *testing.T) { }) } } + +func strRef(s string) *string { + return &s +} diff --git a/grype/pkg/package_test.go b/grype/pkg/package_test.go index 0218fb9e156..d43cea81563 100644 --- a/grype/pkg/package_test.go +++ b/grype/pkg/package_test.go @@ -160,10 +160,10 @@ func TestNew(t *testing.T) { Name: "sqlite", Metadata: syftPkg.RpmArchive{ SourceRpm: "sqlite-3.26.0-6.el8.src.rpm", - ModularityLabel: "abc:2", + ModularityLabel: strRef("abc:2"), }, }, - metadata: RpmMetadata{ModularityLabel: "abc:2"}, + metadata: RpmMetadata{ModularityLabel: strRef("abc:2")}, }, { name: "java pkg", @@ -837,3 +837,7 @@ func withDistro(s *sbom.SBOM, id string) *sbom.SBOM { } return s } + +func strRef(s string) *string { + return &s +} diff --git a/grype/pkg/qualifier/rpmmodularity/qualifier.go b/grype/pkg/qualifier/rpmmodularity/qualifier.go index 2a46ff076ae..e207152cdb7 100644 --- a/grype/pkg/qualifier/rpmmodularity/qualifier.go +++ b/grype/pkg/qualifier/rpmmodularity/qualifier.go @@ -16,7 +16,7 @@ func New(module string) qualifier.Qualifier { return &rpmModularity{module: module} } -func (r rpmModularity) Satisfied(_ *distro.Distro, p pkg.Package) (bool, error) { +func (r rpmModularity) Satisfied(d *distro.Distro, p pkg.Package) (bool, error) { if p.Metadata == nil { // If unable to determine package modularity, the constraint should be considered satisfied return true, nil @@ -27,14 +27,36 @@ func (r rpmModularity) Satisfied(_ *distro.Distro, p pkg.Package) (bool, error) return false, nil } - // If the package modularity is empty (""), the constraint should be considered satisfied - if m.ModularityLabel == "" { + if m.ModularityLabel == nil { + // If the package modularity is empty (null), the constraint should be considered satisfied. + // this is the case where the package source does not support expressing modularity. + return true, nil + } + + if d != nil && d.Type == distro.OracleLinux && *m.ModularityLabel == "" { + // For oraclelinux, the default stream of an installed appstream package does not currently set + // the MODULARITYLABEL property in the rpm metadata; however, in their advisory data they do specify + // modularity information, so this ends up in a case where the vuln entries have modularity but the + // packages coming from the sbom won't, so for now we need to treat the constraint as satisfied when the + // modularity label from an oraclelinux package is "". + // TODO: remove this once we have a way of obtaining and parsing the module information from the DISTTAG + // in syft. return true, nil } if r.module == "" { + if *m.ModularityLabel == "" { + // the DB has a modularity label, but it's empty... we also have a modularity label from a package source + // that supports being able to express modularity, but it's empty. This is a match. + return true, nil + } + + // The package source is able to express modularity, and the DB has a package quality that is empty. + // Since we are doing a prefix match against the modularity label (which is guaranteed to be non-empty), + // and we are checking for an empty prefix, this will always match, however, semantically this makes no sense. + // We don't want package modularities of any value to match this qualifier. return false, nil } - return strings.HasPrefix(m.ModularityLabel, r.module), nil + return strings.HasPrefix(*m.ModularityLabel, r.module), nil } diff --git a/grype/pkg/qualifier/rpmmodularity/qualifier_test.go b/grype/pkg/qualifier/rpmmodularity/qualifier_test.go index 49846b91b7f..66304b13342 100644 --- a/grype/pkg/qualifier/rpmmodularity/qualifier_test.go +++ b/grype/pkg/qualifier/rpmmodularity/qualifier_test.go @@ -5,15 +5,19 @@ import ( "github.com/stretchr/testify/assert" + "github.com/anchore/grype/grype/distro" "github.com/anchore/grype/grype/pkg" "github.com/anchore/grype/grype/pkg/qualifier" ) func TestRpmModularity_Satisfied(t *testing.T) { + oracle, _ := distro.New(distro.OracleLinux, "8") + tests := []struct { name string rpmModularity qualifier.Qualifier pkg pkg.Package + distro *distro.Distro satisfied bool }{ { @@ -22,18 +26,21 @@ func TestRpmModularity_Satisfied(t *testing.T) { pkg: pkg.Package{ Metadata: pkg.JavaMetadata{}, }, + distro: nil, satisfied: false, }, { name: "module with package rpm metadata lacking actual metadata 1", rpmModularity: New("test:1"), pkg: pkg.Package{Metadata: nil}, + distro: nil, satisfied: true, }, { name: "empty module with rpm metadata lacking actual metadata 2", rpmModularity: New(""), pkg: pkg.Package{Metadata: nil}, + distro: nil, satisfied: true, }, { @@ -42,6 +49,7 @@ func TestRpmModularity_Satisfied(t *testing.T) { pkg: pkg.Package{Metadata: pkg.RpmMetadata{ Epoch: nil, }}, + distro: nil, satisfied: true, }, { @@ -50,42 +58,92 @@ func TestRpmModularity_Satisfied(t *testing.T) { pkg: pkg.Package{Metadata: pkg.RpmMetadata{ Epoch: nil, }}, + distro: nil, satisfied: true, }, { name: "modularity label with no module", rpmModularity: New(""), pkg: pkg.Package{Metadata: pkg.RpmMetadata{ - Epoch: nil, - ModularityLabel: "x:3:1234567:abcd", + ModularityLabel: strRef("x:3:1234567:abcd"), }}, + distro: nil, satisfied: false, }, { name: "modularity label in module", rpmModularity: New("x:3"), pkg: pkg.Package{Metadata: pkg.RpmMetadata{ - Epoch: nil, - ModularityLabel: "x:3:1234567:abcd", + ModularityLabel: strRef("x:3:1234567:abcd"), }}, + distro: nil, satisfied: true, }, { name: "modularity label not in module", rpmModularity: New("x:3"), pkg: pkg.Package{Metadata: pkg.RpmMetadata{ - Epoch: nil, - ModularityLabel: "x:1:1234567:abcd", + ModularityLabel: strRef("x:1:1234567:abcd"), + }}, + distro: nil, + satisfied: false, + }, + { + name: "modularity label is positively blank", + rpmModularity: New(""), + pkg: pkg.Package{Metadata: pkg.RpmMetadata{ + ModularityLabel: strRef(""), + }}, + distro: nil, + satisfied: true, + }, + { + name: "modularity label is missing (assume we cannot verify that capability)", + rpmModularity: New(""), + pkg: pkg.Package{Metadata: pkg.RpmMetadata{ + ModularityLabel: nil, + }}, + distro: nil, + satisfied: true, + }, + { + name: "default appstream for oraclelinux (treat as missing)", + rpmModularity: New("nodejs:16"), + pkg: pkg.Package{Metadata: pkg.RpmMetadata{ + ModularityLabel: strRef(""), + }}, + distro: oracle, + satisfied: true, + }, + { + name: "non-default appstream for oraclelinux matches vuln modularity", + rpmModularity: New("nodejs:16"), + pkg: pkg.Package{Metadata: pkg.RpmMetadata{ + ModularityLabel: strRef("nodejs:16:blah"), }}, + distro: oracle, + satisfied: true, + }, + { + name: "non-default appstream for oraclelinux does not match vuln modularity", + rpmModularity: New("nodejs:17"), + pkg: pkg.Package{Metadata: pkg.RpmMetadata{ + ModularityLabel: strRef("nodejs:16:blah"), + }}, + distro: oracle, satisfied: false, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - s, err := test.rpmModularity.Satisfied(nil, test.pkg) + s, err := test.rpmModularity.Satisfied(test.distro, test.pkg) assert.NoError(t, err) assert.Equal(t, test.satisfied, s) }) } } + +func strRef(s string) *string { + return &s +} diff --git a/grype/pkg/rpm_metadata.go b/grype/pkg/rpm_metadata.go index da0cced5cfd..7905f3a0c3c 100644 --- a/grype/pkg/rpm_metadata.go +++ b/grype/pkg/rpm_metadata.go @@ -1,6 +1,6 @@ package pkg type RpmMetadata struct { - Epoch *int `json:"epoch"` - ModularityLabel string `json:"modularityLabel"` + Epoch *int `json:"epoch"` + ModularityLabel *string `json:"modularityLabel"` } diff --git a/grype/presenter/json/test-fixtures/snapshot/TestJsonDirsPresenter.golden b/grype/presenter/json/test-fixtures/snapshot/TestJsonDirsPresenter.golden index 15b97583a25..a318f72fc5d 100644 --- a/grype/presenter/json/test-fixtures/snapshot/TestJsonDirsPresenter.golden +++ b/grype/presenter/json/test-fixtures/snapshot/TestJsonDirsPresenter.golden @@ -61,7 +61,7 @@ "metadataType": "RpmMetadata", "metadata": { "epoch": 2, - "modularityLabel": "" + "modularityLabel": null } } }, diff --git a/grype/presenter/json/test-fixtures/snapshot/TestJsonImgsPresenter.golden b/grype/presenter/json/test-fixtures/snapshot/TestJsonImgsPresenter.golden index 3f0da406d75..61e53cc15ec 100644 --- a/grype/presenter/json/test-fixtures/snapshot/TestJsonImgsPresenter.golden +++ b/grype/presenter/json/test-fixtures/snapshot/TestJsonImgsPresenter.golden @@ -61,7 +61,7 @@ "metadataType": "RpmMetadata", "metadata": { "epoch": 2, - "modularityLabel": "" + "modularityLabel": null } } }, diff --git a/test/quality/.yardstick.yaml b/test/quality/.yardstick.yaml index 670345ceae5..eafa2164ded 100644 --- a/test/quality/.yardstick.yaml +++ b/test/quality/.yardstick.yaml @@ -13,6 +13,8 @@ x-ref: - docker.io/anchore/test_images:appstreams-centos-stream-8-1a287dd@sha256:808f6cf3cf4473eb39ff9bb47ead639d2ed71255b75b9b140162b58c6102bcc9 - docker.io/anchore/test_images:appstreams-oraclelinux-8-1a287dd@sha256:c8d664b0e728d52f57eeb98ed1899c16d3b265f02ddfb41303d7a16c31e0b0f1 - docker.io/anchore/test_images:appstreams-rhel-8-1a287dd@sha256:524ff8a75f21fd886ec7ed82387766df386671e8b77e898d05786118d5b7880b + - docker.io/anchore/test_images:appstreams-nodejs-base-rhel-9-1b0b1b4@sha256:fc6f7a37d7e320f6ff3643d4ec9a208adb1462cd16027f045b56563e12bb0461 + - docker.io/anchore/test_images:appstreams-nodejs-18-rhel-9-1b0b1b4@sha256:08dbfad2d6af9afe47f7647b0b8f38fd29fc9e89306cfc39c9509981f9388b7f - docker.io/anchore/test_images:java-56d52bc@sha256:10008791acbc5866de04108746a02a0c4029ce3a4400a9b3dad45d7f2245f9da - docker.io/anchore/test_images:npm-56d52bc@sha256:ba42ded8613fc643d407a050faf5ab48cfb405ad3ef2015bf6feeb5dff44738d - docker.io/anchore/test_images:gems-56d52bc@sha256:5763c8a225f950961bf01ddec68e36f18e236130e182f2b9290a6e03b9777bfe diff --git a/test/quality/vulnerability-match-labels b/test/quality/vulnerability-match-labels index 82385b09b0f..79ee9380f5e 160000 --- a/test/quality/vulnerability-match-labels +++ b/test/quality/vulnerability-match-labels @@ -1 +1 @@ -Subproject commit 82385b09b0fecf63adea2fc95b61b11dfc387714 +Subproject commit 79ee9380f5ee6fca2db7e707c1428eeb5ce84fd5