rhel: update RHEL matcher to account for CPE subset matching

Start matching repository CPEs based on the CPE subset relation. This change interprets VEX CPEs identifying Red Hat repositories as CPE matching expressions and looks for a subset relation with the record's repositoty CPE. This change also introduces a fallback to deal with CPEs in the VEX data that are expected to describe a subset relationship but don't use the correct matching syntax, in these cases matching is done with a crude string prefix match. Signed-off-by: crozzy <[email protected]>
quay · Aug 14, 2024 · 6063f57 · 6063f57
1 parent 93b1749
commit 6063f57
Show file tree

Hide file tree

Showing 3 changed files with 178 additions and 5 deletions.
diff --git a/internal/matcher/controller.go b/internal/matcher/controller.go
@@ -142,7 +142,7 @@ func (mc *Controller) filter(ctx context.Context, interested []*claircore.IndexR
 		if err != nil {
 			return nil, err
 		}
-		filtered[record.Package.ID] = match
+		filtered[record.Package.ID] = append(filtered[record.Package.ID], match...)
 	}
 	return filtered, nil
 }

diff --git a/rhel/matcher.go b/rhel/matcher.go
@@ -2,11 +2,14 @@ package rhel
 
 import (
 	"context"
+	"strings"
 
 	version "github.com/knqyf263/go-rpm-version"
+	"github.com/quay/zlog"
 
 	"github.com/quay/claircore"
 	"github.com/quay/claircore/libvuln/driver"
+	"github.com/quay/claircore/toolkit/types/cpe"
 )
 
 // Matcher implements driver.Matcher.
@@ -28,12 +31,50 @@ func (*Matcher) Filter(record *claircore.IndexRecord) bool {
 func (*Matcher) Query() []driver.MatchConstraint {
 	return []driver.MatchConstraint{
 		driver.PackageModule,
-		driver.RepositoryName,
 	}
 }
 
+// IsCPESubstringMatch is a Red Hat specific hack that handles the "CPE patterns" in the VEX
+// data. For historical/unfathomable reasons, Red Hat
+// doesn't use the syntax defined in the Matching Expression spec.
+// For example, "cpe:/a:redhat:openshift:4" is expected to match "cpe:/a:redhat:openshift:4.13::el8".
+//
+// This is defined (citation needed) to be a substring match on the "pattern" and "target" CPEs.
+// Since we always normalize CPEs into v2.3 "Formatted String" form, we need to trim the
+// added "ANY" attributes from the pattern.
+//
+// TODO(crozzy) Remove once RH VEX data updates CPEs with standard matching expressions.
+func isCPESubstringMatch(recordCPE cpe.WFN, vulnCPE cpe.WFN) bool {
+	return strings.HasPrefix(recordCPE.String(), strings.TrimRight(vulnCPE.String(), ":*"))
+}
+
 // Vulnerable implements driver.Matcher.
-func (m *Matcher) Vulnerable(_ context.Context, record *claircore.IndexRecord, vuln *claircore.Vulnerability) (bool, error) {
+//
+// Vulnerable will interpret the claircore.Vulnerability.Repo.CPE
+// as a CPE match expression, and to be considered vulnerable,
+// the relationship between claircore.IndexRecord.Repository.CPE and
+// the claircore.Vulnerability.Repo.CPE needs to be a CPE Name Comparison
+// Relation of SUPERSET(⊇)(Source is a superset or equal to the target).
+// https://nvlpubs.nist.gov/nistpubs/Legacy/IR/nistir7696.pdf Section 6.2.
+func (m *Matcher) Vulnerable(ctx context.Context, record *claircore.IndexRecord, vuln *claircore.Vulnerability) (bool, error) {
+	if vuln.Repo == nil || record.Repository == nil || vuln.Repo.Key != repositoryKey {
+		return false, nil
+	}
+	var err error
+	// This conversion has to be done because our current data structure doesn't
+	// support the claircore.Vulnerability.Repo.CPE field.
+	vuln.Repo.CPE, err = cpe.Unbind(vuln.Repo.Name)
+	if err != nil {
+		zlog.Warn(ctx).
+			Str("vulnerability name", vuln.Name).
+			Err(err).
+			Msg("unable to unbind repo CPE")
+		return false, nil
+	}
+	if !cpe.Compare(vuln.Repo.CPE, record.Repository.CPE).IsSuperset() && !isCPESubstringMatch(record.Repository.CPE, vuln.Repo.CPE) {
+		return false, nil
+	}
+
 	pkgVer := version.NewVersion(record.Package.Version)
 	var vulnVer version.Version
 	// Assume the vulnerability record we have is for the last known vulnerable

diff --git a/rhel/matcher_test.go b/rhel/matcher_test.go
@@ -21,6 +21,7 @@ import (
 	"github.com/quay/claircore/pkg/ctxlock"
 	"github.com/quay/claircore/test/integration"
 	pgtest "github.com/quay/claircore/test/postgres"
+	"github.com/quay/claircore/toolkit/types/cpe"
 )
 
 func TestMain(m *testing.M) {
@@ -118,43 +119,141 @@ func TestVulnerable(t *testing.T) {
 		Package: &claircore.Package{
 			Version: "0.33.0-6.el8",
 		},
+		Repository: &claircore.Repository{
+			CPE:  cpe.MustUnbind("cpe:/o:redhat:enterprise_linux:8::baseos"),
+			Name: "cpe:/o:redhat:enterprise_linux:8::baseos",
+			Key:  "rhel-cpe-repository",
+		},
+	}
+	openshiftRecord := &claircore.IndexRecord{
+		Package: &claircore.Package{
+			Version: "0.33.0-6.el8",
+		},
+		Repository: &claircore.Repository{
+			CPE:  cpe.MustUnbind("cpe:/a:redhat:openshift:4.13::el8"),
+			Name: "cpe:/a:redhat:openshift:4.13::el8",
+			Key:  "rhel-cpe-repository",
+		},
+	}
+	openshift5Record := &claircore.IndexRecord{
+		Package: &claircore.Package{
+			Version: "0.33.0-6.el8",
+		},
+		Repository: &claircore.Repository{
+			CPE:  cpe.MustUnbind("cpe:/a:redhat:openshift:5.1::el8"),
+			Name: "cpe:/a:redhat:openshift:5.1::el8",
+			Key:  "rhel-cpe-repository",
+		},
 	}
 	fixedVulnPast := &claircore.Vulnerability{
 		Package: &claircore.Package{
 			Version: "",
 		},
 		FixedInVersion: "0.33.0-5.el8",
+		Repo: &claircore.Repository{
+			Name: "cpe:/o:redhat:enterprise_linux:8::baseos",
+			Key:  "rhel-cpe-repository",
+		},
 	}
 	fixedVulnCurrent := &claircore.Vulnerability{
 		Package: &claircore.Package{
 			Version: "",
 		},
 		FixedInVersion: "0.33.0-6.el8",
+		Repo: &claircore.Repository{
+			Name: "cpe:/o:redhat:enterprise_linux:8::baseos",
+			Key:  "rhel-cpe-repository",
+		},
 	}
 	fixedVulnFuture := &claircore.Vulnerability{
 		Package: &claircore.Package{
 			Version: "",
 		},
 		FixedInVersion: "0.33.0-7.el8",
+		Repo: &claircore.Repository{
+			Name: "cpe:/o:redhat:enterprise_linux:8::baseos",
+			Key:  "rhel-cpe-repository",
+		},
 	}
 	unfixedVuln := &claircore.Vulnerability{
 		Package: &claircore.Package{
 			Version: "",
 		},
 		FixedInVersion: "",
+		Repo: &claircore.Repository{
+			Name: "cpe:/o:redhat:enterprise_linux:8::baseos",
+			Key:  "rhel-cpe-repository",
+		},
+	}
+	unfixedVulnBadCPE := &claircore.Vulnerability{
+		Package: &claircore.Package{
+			Version: "",
+		},
+		FixedInVersion: "",
+		Repo: &claircore.Repository{
+			Name: "cep:o:redhat:enterprise_linux:8::baseos",
+			Key:  "rhel-cpe-repository",
+		},
+	}
+	unfixedVulnRepoIsSubset := &claircore.Vulnerability{
+		Package: &claircore.Package{
+			Version: "",
+		},
+		FixedInVersion: "",
+		Repo: &claircore.Repository{
+			Name: "cpe:/o:redhat:enterprise_linux:8",
+			Key:  "rhel-cpe-repository",
+		},
+	}
+	unfixedVulnRepoNotSubset := &claircore.Vulnerability{
+		Package: &claircore.Package{
+			Version: "",
+		},
+		FixedInVersion: "",
+		Repo: &claircore.Repository{
+			Name: "cpe:/o:redhat:enterprise_linux:8::appstream",
+			Key:  "rhel-cpe-repository",
+		},
+	}
+	unfixedVulnRepoSubstring := &claircore.Vulnerability{
+		Package: &claircore.Package{
+			Version: "",
+		},
+		FixedInVersion: "",
+		Repo: &claircore.Repository{
+			Name: "cpe:/a:redhat:openshift:4",
+			Key:  "rhel-cpe-repository",
+		},
+	}
+	genericWilcardRepo := &claircore.Vulnerability{
+		Package: &claircore.Package{
+			Version: "",
+		},
+		FixedInVersion: "",
+		Repo: &claircore.Repository{
+			Name: "cpe:/a:redhat:openshift:4.%02::el8",
+			Key:  "rhel-cpe-repository",
+		},
 	}
 
 	testCases := []vulnerableTestCase{
 		{ir: record, v: fixedVulnPast, want: false, name: "vuln fixed in past version"},
 		{ir: record, v: fixedVulnCurrent, want: false, name: "vuln fixed in current version"},
 		{ir: record, v: fixedVulnFuture, want: true, name: "outdated package"},
 		{ir: record, v: unfixedVuln, want: true, name: "unfixed vuln"},
+		{ir: record, v: unfixedVulnBadCPE, want: false, name: "unfixed vuln, invalid CPE"},
+		{ir: record, v: unfixedVulnRepoIsSubset, want: true, name: "unfixed vuln, Repo is a subset"},
+		{ir: record, v: unfixedVulnRepoNotSubset, want: false, name: "unfixed vuln, Repo not a subset"},
+		{ir: openshiftRecord, v: unfixedVulnRepoSubstring, want: true, name: "unfixed vuln, Repo is a substring match"},
+		{ir: openshiftRecord, v: genericWilcardRepo, want: true, name: "unfixed vuln, Repo is a superset (with wildcard)"},
+		{ir: openshift5Record, v: genericWilcardRepo, want: false, name: "unfixed vuln, Repo isn't a superset (with wildcard)"},
 	}
 
 	m := &Matcher{}
-
+	ctx := context.Background()
+	ctx = zlog.Test(ctx, t)
 	for _, tc := range testCases {
-		got, err := m.Vulnerable(nil, tc.ir, tc.v)
+		got, err := m.Vulnerable(ctx, tc.ir, tc.v)
 		if err != nil {
 			t.Error(err)
 		}
@@ -163,3 +262,36 @@ func TestVulnerable(t *testing.T) {
 		}
 	}
 }
+
+func TestIsCPEStringSubsetMatch(t *testing.T) {
+	t.Parallel()
+
+	testcases := []struct {
+		name               string
+		recordCPE, vulnCPE cpe.WFN
+		match              bool
+	}{
+		{
+			name:      "simple_case",
+			recordCPE: cpe.MustUnbind("cpe:/a:redhat:openshift:4.13::el8"),
+			vulnCPE:   cpe.MustUnbind("cpe:/a:redhat:openshift:4"),
+			match:     true,
+		},
+		{
+			name:      "wrong_minor",
+			recordCPE: cpe.MustUnbind("cpe:/a:redhat:openshift:4.13::el8"),
+			vulnCPE:   cpe.MustUnbind("cpe:/a:redhat:openshift:4.1::el8"),
+			match:     false,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			tt := tc
+			matched := isCPESubstringMatch(tt.recordCPE, tt.vulnCPE)
+			if matched != tt.match {
+				t.Errorf("unexpected matching %s and %s", tt.recordCPE, tt.vulnCPE)
+			}
+		})
+	}
+}