From 4bdbd5e6db2e8e919938c3cb348350ba91966a12 Mon Sep 17 00:00:00 2001 From: Quentin Machu Date: Mon, 25 Jan 2016 14:50:48 -0500 Subject: [PATCH] *: fix several tests --- updater/fetchers.go | 35 --------- updater/fetchers/debian/debian.go | 14 ++-- updater/fetchers/debian/debian_test.go | 20 ++++- .../debian/testdata/fetcher_debian_test.json | 18 +---- updater/fetchers/rhel/rhel.go | 4 +- updater/fetchers/rhel/rhel_test.go | 4 +- updater/fetchers/ubuntu/ubuntu.go | 4 +- updater/updater.go | 39 +++++++++- updater/{fetchers_test.go => updater_test.go} | 6 +- worker/detectors/feature/dpkg/dpkg_test.go | 2 +- worker/detectors/feature/rpm/rpm_test.go | 2 +- worker/worker.go | 73 +------------------ 12 files changed, 81 insertions(+), 140 deletions(-) rename updater/{fetchers_test.go => updater_test.go} (88%) diff --git a/updater/fetchers.go b/updater/fetchers.go index f0295559..af09eeb9 100644 --- a/updater/fetchers.go +++ b/updater/fetchers.go @@ -49,38 +49,3 @@ func RegisterFetcher(name string, f Fetcher) { fetchers[name] = f } - -// DoVulnerabilityNamespacing is an helper function for fetchers. -// -// It takes a Vulnerability that doesn't have a Namespace and split it into -// potentially multiple vulnerabilities that have a Namespace and only contains the FixedIn -// FeatureVersions corresponding to their Namespace. -// -// It helps simplifying the fetchers that share the same metadata about a Vulnerability regardless -// of their actual namespace (ie. same vulnerability information for every version of a distro). -func DoVulnerabilityNamespacing(v database.Vulnerability) []database.Vulnerability { - vulnerabilitiesMap := make(map[string]*database.Vulnerability) - - featureVersions := v.FixedIn - v.FixedIn = []database.FeatureVersion{} - - for _, fv := range featureVersions { - if vulnerability, ok := vulnerabilitiesMap[fv.Feature.Namespace.Name]; !ok { - newVulnerability := v - newVulnerability.Namespace.Name = fv.Feature.Namespace.Name - newVulnerability.FixedIn = []database.FeatureVersion{fv} - - vulnerabilitiesMap[fv.Feature.Namespace.Name] = &newVulnerability - } else { - vulnerability.FixedIn = append(vulnerability.FixedIn, fv) - } - } - - // Convert map into a slice. - var vulnerabilities []database.Vulnerability - for _, vulnerability := range vulnerabilitiesMap { - vulnerabilities = append(vulnerabilities, *vulnerability) - } - - return vulnerabilities -} diff --git a/updater/fetchers/debian/debian.go b/updater/fetchers/debian/debian.go index b566c89c..7f1ec9c5 100644 --- a/updater/fetchers/debian/debian.go +++ b/updater/fetchers/debian/debian.go @@ -149,13 +149,10 @@ func parseDebianJSON(data *jsonData) (vulnerabilities []database.Vulnerability, } // Get or create the vulnerability. - namespaceName := "debian:" + database.DebianReleasesMapping[releaseName] - index := namespaceName + ":" + vulnName - vulnerability, vulnerabilityAlreadyExists := mvulnerabilities[index] + vulnerability, vulnerabilityAlreadyExists := mvulnerabilities[vulnName] if !vulnerabilityAlreadyExists { vulnerability = &database.Vulnerability{ Name: vulnName, - Namespace: database.Namespace{Name: namespaceName}, Link: strings.Join([]string{cveURLPrefix, "/", vulnName}, ""), Severity: types.Unknown, Description: vulnNode.Description, @@ -192,13 +189,18 @@ func parseDebianJSON(data *jsonData) (vulnerabilities []database.Vulnerability, // Create and add the feature version. pkg := database.FeatureVersion{ - Feature: database.Feature{Name: pkgName}, + Feature: database.Feature{ + Name: pkgName, + Namespace: database.Namespace{ + Name: "debian:" + database.DebianReleasesMapping[releaseName], + }, + }, Version: version, } vulnerability.FixedIn = append(vulnerability.FixedIn, pkg) // Store the vulnerability. - mvulnerabilities[index] = vulnerability + mvulnerabilities[vulnName] = vulnerability } } } diff --git a/updater/fetchers/debian/debian_test.go b/updater/fetchers/debian/debian_test.go index ae5b3ff1..e2b2eb52 100644 --- a/updater/fetchers/debian/debian_test.go +++ b/updater/fetchers/debian/debian_test.go @@ -31,7 +31,7 @@ func TestDebianParser(t *testing.T) { // Test parsing testdata/fetcher_debian_test.json testFile, _ := os.Open(path.Join(path.Dir(filename)) + "/testdata/fetcher_debian_test.json") response, err := buildResponse(testFile, "") - if assert.Nil(t, err) && assert.Len(t, response.Vulnerabilities, 2) { + if assert.Nil(t, err) && assert.Len(t, response.Vulnerabilities, 3) { for _, vulnerability := range response.Vulnerabilities { if vulnerability.Name == "CVE-2015-1323" { assert.Equal(t, "https://security-tracker.debian.org/tracker/CVE-2015-1323", vulnerability.Link) @@ -88,6 +88,24 @@ func TestDebianParser(t *testing.T) { }, } + for _, expectedFeatureVersion := range expectedFeatureVersions { + assert.Contains(t, vulnerability.FixedIn, expectedFeatureVersion) + } + } else if vulnerability.Name == "CVE-2013-2685" { + assert.Equal(t, "https://security-tracker.debian.org/tracker/CVE-2013-2685", vulnerability.Link) + assert.Equal(t, types.Negligible, vulnerability.Severity) + assert.Equal(t, "Un-affected packages.", vulnerability.Description) + + expectedFeatureVersions := []database.FeatureVersion{ + database.FeatureVersion{ + Feature: database.Feature{ + Namespace: database.Namespace{Name: "debian:8"}, + Name: "asterisk", + }, + Version: types.MinVersion, + }, + } + for _, expectedFeatureVersion := range expectedFeatureVersions { assert.Contains(t, vulnerability.FixedIn, expectedFeatureVersion) } diff --git a/updater/fetchers/debian/testdata/fetcher_debian_test.json b/updater/fetchers/debian/testdata/fetcher_debian_test.json index 740a44c0..71e0551e 100644 --- a/updater/fetchers/debian/testdata/fetcher_debian_test.json +++ b/updater/fetchers/debian/testdata/fetcher_debian_test.json @@ -54,7 +54,8 @@ }, "asterisk": { "CVE-2013-2685": { - "description": "Un-affected packages", + "_comment": "This CVE has a non-affected (anymore?) package.", + "description": "Un-affected packages.", "releases": { "jessie": { "fixed_version": "0", @@ -63,21 +64,6 @@ }, "status": "resolved", "urgency": "unimportant" - }, - "wheezy": { - "repositories": { - "sid": "1:13.1.0~dfsg-1.1" - }, - "status": "undetermined", - "urgency": "unimportant" - }, - "sid": { - "fixed_version": "0", - "repositories": { - "sid": "1:13.1.0~dfsg-1.1" - }, - "status": "resolved", - "urgency": "unimportant" } } }, diff --git a/updater/fetchers/rhel/rhel.go b/updater/fetchers/rhel/rhel.go index faa8818d..3bd28fc3 100644 --- a/updater/fetchers/rhel/rhel.go +++ b/updater/fetchers/rhel/rhel.go @@ -140,9 +140,9 @@ func (f *RHELFetcher) FetchUpdate(datastore database.Datastore) (resp updater.Fe return resp, err } - // Collect vulnerabilities, splitting them by Namespaces. + // Collect vulnerabilities. for _, v := range vs { - resp.Vulnerabilities = append(resp.Vulnerabilities, updater.DoVulnerabilityNamespacing(v)...) + resp.Vulnerabilities = append(resp.Vulnerabilities, v) } } diff --git a/updater/fetchers/rhel/rhel_test.go b/updater/fetchers/rhel/rhel_test.go index 2a25ed02..dac55372 100644 --- a/updater/fetchers/rhel/rhel_test.go +++ b/updater/fetchers/rhel/rhel_test.go @@ -33,7 +33,7 @@ func TestRHELParser(t *testing.T) { testFile, _ := os.Open(path + "/testdata/fetcher_rhel_test.1.xml") vulnerabilities, err := parseRHSA(testFile) if assert.Nil(t, err) && assert.Len(t, vulnerabilities, 1) { - assert.Equal(t, "RHSA-2015:1193", vulnerabilities[0].ID) + assert.Equal(t, "RHSA-2015:1193", vulnerabilities[0].Name) assert.Equal(t, "https://rhn.redhat.com/errata/RHSA-2015-1193.html", vulnerabilities[0].Link) assert.Equal(t, types.Medium, vulnerabilities[0].Severity) assert.Equal(t, `Xerces-C is a validating XML parser written in a portable subset of C++. A flaw was found in the way the Xerces-C XML parser processed certain XML documents. A remote attacker could provide specially crafted XML input that, when parsed by an application using Xerces-C, would cause that application to crash.`, vulnerabilities[0].Description) @@ -71,7 +71,7 @@ func TestRHELParser(t *testing.T) { testFile, _ = os.Open(path + "/testdata/fetcher_rhel_test.2.xml") vulnerabilities, err = parseRHSA(testFile) if assert.Nil(t, err) && assert.Len(t, vulnerabilities, 1) { - assert.Equal(t, "RHSA-2015:1207", vulnerabilities[0].ID) + assert.Equal(t, "RHSA-2015:1207", vulnerabilities[0].Name) assert.Equal(t, "https://rhn.redhat.com/errata/RHSA-2015-1207.html", vulnerabilities[0].Link) assert.Equal(t, types.Critical, vulnerabilities[0].Severity) assert.Equal(t, `Mozilla Firefox is an open source web browser. XULRunner provides the XUL Runtime environment for Mozilla Firefox. Several flaws were found in the processing of malformed web content. A web page containing malicious content could cause Firefox to crash or, potentially, execute arbitrary code with the privileges of the user running Firefox.`, vulnerabilities[0].Description) diff --git a/updater/fetchers/ubuntu/ubuntu.go b/updater/fetchers/ubuntu/ubuntu.go index 1f7d26bd..6e72658f 100644 --- a/updater/fetchers/ubuntu/ubuntu.go +++ b/updater/fetchers/ubuntu/ubuntu.go @@ -148,8 +148,8 @@ func (fetcher *UbuntuFetcher) FetchUpdate(datastore database.Datastore) (resp up return resp, err } - // Add the vulnerability to the response, splitting it by Namespaces. - resp.Vulnerabilities = append(resp.Vulnerabilities, updater.DoVulnerabilityNamespacing(v)...) + // Add the vulnerability to the response. + resp.Vulnerabilities = append(resp.Vulnerabilities, v) // Store any unknown releases as notes. for k := range unknownReleases { diff --git a/updater/updater.go b/updater/updater.go index 06c24405..cc0d3386 100644 --- a/updater/updater.go +++ b/updater/updater.go @@ -219,7 +219,7 @@ func fetch(datastore database.Datastore) (bool, []database.Vulnerability, map[st for i := 0; i < len(fetchers); i++ { resp := <-responseC if resp != nil { - vulnerabilities = append(vulnerabilities, resp.Vulnerabilities...) + vulnerabilities = append(vulnerabilities, doVulnerabilitiesNamespacing(resp.Vulnerabilities)...) notes = append(notes, resp.Notes...) if resp.FlagName != "" && resp.FlagValue != "" { flags[resp.FlagName] = resp.FlagValue @@ -239,3 +239,40 @@ func getLastUpdate(datastore database.Datastore) time.Time { } return time.Time{} } + +// doVulnerabilitiesNamespacing takes Vulnerabilities that don't have a Namespace and split them +// into multiple vulnerabilities that have a Namespace and only contains the FixedIn +// FeatureVersions corresponding to their Namespace. +// +// It helps simplifying the fetchers that share the same metadata about a Vulnerability regardless +// of their actual namespace (ie. same vulnerability information for every version of a distro). +func doVulnerabilitiesNamespacing(vulnerabilities []database.Vulnerability) []database.Vulnerability { + vulnerabilitiesMap := make(map[string]*database.Vulnerability) + + for _, v := range vulnerabilities { + featureVersions := v.FixedIn + v.FixedIn = []database.FeatureVersion{} + + for _, fv := range featureVersions { + index := fv.Feature.Namespace.Name + ":" + v.Name + + if vulnerability, ok := vulnerabilitiesMap[index]; !ok { + newVulnerability := v + newVulnerability.Namespace.Name = fv.Feature.Namespace.Name + newVulnerability.FixedIn = []database.FeatureVersion{fv} + + vulnerabilitiesMap[index] = &newVulnerability + } else { + vulnerability.FixedIn = append(vulnerability.FixedIn, fv) + } + } + } + + // Convert map into a slice. + var response []database.Vulnerability + for _, vulnerability := range vulnerabilitiesMap { + response = append(response, *vulnerability) + } + + return response +} diff --git a/updater/fetchers_test.go b/updater/updater_test.go similarity index 88% rename from updater/fetchers_test.go rename to updater/updater_test.go index 51dc92f0..5b6df0f0 100644 --- a/updater/fetchers_test.go +++ b/updater/updater_test.go @@ -1,6 +1,7 @@ package updater import ( + "fmt" "testing" "github.com/coreos/clair/database" @@ -8,7 +9,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestDoVulnerabilityNamespacing(t *testing.T) { +func TestDoVulnerabilitiesNamespacing(t *testing.T) { fv1 := database.FeatureVersion{ Feature: database.Feature{ Namespace: database.Namespace{Name: "Namespace1"}, @@ -38,7 +39,7 @@ func TestDoVulnerabilityNamespacing(t *testing.T) { FixedIn: []database.FeatureVersion{fv1, fv2, fv3}, } - vulnerabilities := DoVulnerabilityNamespacing(vulnerability) + vulnerabilities := doVulnerabilitiesNamespacing([]database.Vulnerability{vulnerability}) for _, vulnerability := range vulnerabilities { switch vulnerability.Namespace.Name { case fv1.Feature.Namespace.Name: @@ -50,6 +51,7 @@ func TestDoVulnerabilityNamespacing(t *testing.T) { assert.Contains(t, vulnerability.FixedIn, fv3) default: t.Errorf("Should not have a Vulnerability with '%s' as its Namespace.", vulnerability.Namespace.Name) + fmt.Printf("%#v\n", vulnerability) } } } diff --git a/worker/detectors/feature/dpkg/dpkg_test.go b/worker/detectors/feature/dpkg/dpkg_test.go index ba53905e..4a408f4a 100644 --- a/worker/detectors/feature/dpkg/dpkg_test.go +++ b/worker/detectors/feature/dpkg/dpkg_test.go @@ -41,7 +41,7 @@ var dpkgPackagesTests = []feature.FeatureVersionTest{ }, }, Data: map[string][]byte{ - "var/lib/dpkg/status": feature.LoadFileForTest("testdata/dpkg_status"), + "var/lib/dpkg/status": feature.LoadFileForTest("dpkg/testdata/status"), }, }, } diff --git a/worker/detectors/feature/rpm/rpm_test.go b/worker/detectors/feature/rpm/rpm_test.go index e4dda477..49862ea0 100644 --- a/worker/detectors/feature/rpm/rpm_test.go +++ b/worker/detectors/feature/rpm/rpm_test.go @@ -39,7 +39,7 @@ var rpmPackagesTests = []feature.FeatureVersionTest{ }, }, Data: map[string][]byte{ - "var/lib/rpm/Packages": feature.LoadFileForTest("testdata/rpm_Packages"), + "var/lib/rpm/Packages": feature.LoadFileForTest("rpm/testdata/Packages"), }, }, } diff --git a/worker/worker.go b/worker/worker.go index a43071f0..a01999dd 100644 --- a/worker/worker.go +++ b/worker/worker.go @@ -56,6 +56,8 @@ var ( // Process detects the Namespace of a layer, the features it adds/removes, and // then stores everything in the database. +// TODO(Quentin-M): We could have a goroutine that looks for layers that have been analyzed with an +// older engine version and that processes them. func Process(datastore database.Datastore, name, parentName, path, imageFormat string) error { // Verify parameters. if name == "" { @@ -222,74 +224,3 @@ func detectFeatures(name string, data map[string][]byte, namespace *database.Nam return } - -// // detectAndInsertInstalledAndRemovedPackages finds the installed and removed -// // package nodes and inserts the installed packages into the database. -// func detectAndInsertInstalledAndRemovedPackages(detectedOS string, packageList []database.FeatureVersion, parent *database.Layer) (installedNodes, removedNodes []string, err error) { -// // Get the parent layer's packages. -// parentPackageNodes, err := parent.AllPackages() -// if err != nil { -// return nil, nil, err -// } -// parentPackages, err := database.FindAllPackagesByNodes(parentPackageNodes, []string{database.FieldPackageName, database.FieldPackageVersion}) -// if err != nil { -// return nil, nil, err -// } -// -// // Map detected packages (name:version) string to packages. -// packagesNVMapToPackage := make(map[string]*database.Package) -// for _, p := range packageList { -// packagesNVMapToPackage[p.Name+":"+p.Version.String()] = p -// } -// -// // Map parent's packages (name:version) string to nodes. -// parentPackagesNVMapToNodes := make(map[string]string) -// for _, p := range parentPackages { -// parentPackagesNVMapToNodes[p.Name+":"+p.Version.String()] = p.Node -// } -// -// // Build a list of the parent layer's packages' node values. -// var parentPackagesNV []string -// for _, p := range parentPackages { -// parentPackagesNV = append(parentPackagesNV, p.Name+":"+p.Version.String()) -// } -// -// // Build a list of the layer packages' node values. -// var layerPackagesNV []string -// for _, p := range packageList { -// layerPackagesNV = append(layerPackagesNV, p.Name+":"+p.Version.String()) -// } -// -// // Calculate the installed and removed packages. -// removedPackagesNV := utils.CompareStringLists(parentPackagesNV, layerPackagesNV) -// installedPackagesNV := utils.CompareStringLists(layerPackagesNV, parentPackagesNV) -// -// // Build a list of all the installed packages. -// var installedPackages []database.FeatureVersion -// for _, nv := range installedPackagesNV { -// p, _ := packagesNVMapToPackage[nv] -// p.OS = detectedOS -// installedPackages = append(installedPackages, p) -// } -// -// // Insert that list into the database. -// err = database.InsertPackages(installedPackages) -// if err != nil { -// return nil, nil, err -// } -// -// // Build the list of installed package nodes. -// for _, p := range installedPackages { -// if p.Node != "" { -// installedNodes = append(installedNodes, p.Node) -// } -// } -// -// // Build the list of removed package nodes. -// for _, nv := range removedPackagesNV { -// node, _ := parentPackagesNVMapToNodes[nv] -// removedNodes = append(removedNodes, node) -// } -// -// return -// }