Fix matching when RPM modularity is a factor (#1679)

* allow for RPM modularity to be optional Signed-off-by: Alex Goodman <[email protected]> * use latest syft from main Signed-off-by: Alex Goodman <[email protected]> * bump syft Signed-off-by: Alex Goodman <[email protected]> * remove lint ignores for CPEs Signed-off-by: Alex Goodman <[email protected]> * update snapshot tests Signed-off-by: Alex Goodman <[email protected]> * update tests Signed-off-by: Alex Goodman <[email protected]> * 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 <[email protected]> * test: add new appstream images to quality gate and bump labels Signed-off-by: Weston Steimel <[email protected]> * chore: bump quality gate labels Signed-off-by: Weston Steimel <[email protected]> --------- Signed-off-by: Alex Goodman <[email protected]> Signed-off-by: Weston Steimel <[email protected]> Co-authored-by: Weston Steimel <[email protected]>
anchore · Jan 26, 2024 · 3e0aa00 · 3e0aa00
1 parent 73cb5f6
commit 3e0aa00
Show file tree

Hide file tree

Showing 12 changed files with 117 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -61,7 +61,7 @@
     "metadataType": "RpmMetadata",
     "metadata": {
      "epoch": 2,
-     "modularityLabel": ""
+     "modularityLabel": null
     }
    }
   },

diff --git 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