Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix resolution state for packages with modules #937

Merged
merged 10 commits into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions database/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,11 @@ type RHELv2PackageInfo struct {
type RHELv2Package struct {
Model

Name string `json:"name"`
Version string `json:"version,omitempty"`
Module string `json:"module,omitempty"`
Arch string `json:"arch,omitempty"`
Name string `json:"name"`
Version string `json:"version,omitempty"`
Module string `json:"module,omitempty"`
Arch string `json:"arch,omitempty"`
ResolutionState string `json:"resolution_state"`

// ExecutableToDependencies maps a feature provided executable to its dependencies.
// Eg, If executable E is provided by this feature, and it imports a library B, we will have a map for E -> [B]
Expand All @@ -177,8 +178,7 @@ type RHELv2Package struct {
// Executables lists the executables determined from ExecutableToDependencies and
// LibraryToDependencies. This is only populated when both ExecutableToDependencies and
// LibraryToDependencies are empty.
Executables []*v1.Executable `json:"executables,omitempty"`
ResolutionState string `json:"resolution_state"`
Executables []*v1.Executable `json:"executables,omitempty"`
}

func (p *RHELv2Package) String() string {
Expand Down
2 changes: 1 addition & 1 deletion database/pgsql/migrations/00019_add_vuln_state_rhel.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ func init() {
RegisterMigration(migrate.Migration{
ID: 19,
Up: migrate.Queries([]string{
`ALTER TABLE vuln_v2 ADD COLUMN IF NOT EXISTS package_resolution_state TEXT NOT NULL DEFAULT ''`,
`ALTER TABLE vuln_v2 ADD COLUMN IF NOT EXISTS resolution_state TEXT`,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to make this NOT NULL. Also, change the name to something more concise

Copy link
Contributor

@daynewlee daynewlee Sep 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the tests failed when I was trying to remove "NOT NULL". I am guessing it's because not all package has an resolution state. But it will be great if we can remove that constraint.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there are 1777588 rows in vuln_v2 with empty resolution_state :D . I don't think we need to worry about NOT NULL since Go would turn it into an empty string upon insert and read

}),
})
}
10 changes: 6 additions & 4 deletions database/pgsql/queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,17 @@ const (
link, severity, cvss3, cvss2,
package_name, package_module, package_arch,
cpe,
fixed_in_version, arch_operation, package_resolution_state
resolution_state,
fixed_in_version, arch_operation
) VALUES (
$1,
$2, $3, $4,
$5, $6,
$7, $8, $9, $10,
$11, $12, $13,
$14,
$15, $16, $17
$15,
$16, $17
)
ON CONFLICT (hash) DO NOTHING;`

Expand All @@ -250,9 +252,9 @@ const (
vuln.cvss2,
vuln.package_name,
vuln.package_arch,
vuln.resolution_state,
vuln.fixed_in_version,
vuln.arch_operation,
vuln.package_resolution_state
vuln.arch_operation
FROM
vuln_v2 as vuln
LEFT JOIN vuln_description ON vuln.description_hash = vuln_description.hash
Expand Down
5 changes: 3 additions & 2 deletions database/pgsql/rhelv2_vulnerability.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ func (pgSQL *pgSQL) insertRHELv2Vulnerability(vulnStatement, vulnDescriptionStat
vuln.Link, vuln.Severity, vuln.CVSSv3, vuln.CVSSv2,
pkg.Name, pkg.Module, pkg.Arch,
cpe,
pkgInfo.FixedInVersion, pkgInfo.ArchOperation,
pkg.ResolutionState,
pkgInfo.FixedInVersion, pkgInfo.ArchOperation,
)
if err != nil {
return errors.Wrapf(err, "inserting RHELv2 vulnerability %q for package %q with cpe %q into the DB", vuln.Name, pkg.Name, cpe)
Expand Down Expand Up @@ -174,6 +174,7 @@ func md5Vuln(v *database.RHELv2Vulnerability, pkgInfoIdx, pkgIdx, cpeIdx int) []
b.WriteString(pkg.Module)
b.WriteString(pkg.Arch)
b.WriteString(cpe)
b.WriteString(pkg.ResolutionState)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be part of the md5 hash

b.WriteString(pkgInfo.FixedInVersion)
b.WriteString(pkgInfo.ArchOperation.String())

Expand Down Expand Up @@ -267,9 +268,9 @@ func (pgSQL *pgSQL) getRHELv2Vulns(tx *sql.Tx, record *database.RHELv2Record) ([
&vuln.CVSSv2,
&pkg.Name,
&pkg.Arch,
&pkg.ResolutionState,
&pkgInfo.FixedInVersion,
&pkgInfo.ArchOperation,
&pkg.ResolutionState,
)
if err != nil {
return nil, errors.Wrapf(err, "Scanning row for package: %s, module: %s, cpe: %s", record.Pkg.Name, record.Pkg.Module, record.CPE)
Expand Down
10 changes: 6 additions & 4 deletions pkg/rhelv2/ovalutil/rpm.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ func RPMDefsToVulns(root *oval.Root, protoVuln ProtoVulnFunc) ([]*database.RHELv
continue
}

// parse unpatched CVE component resolution for each def
componentResolutions := ParseUnpatchedCVEComponents(def)
componentResolutions := getComponentResolutions(def)

// recursively collect criterions for this definition
cris := cris[:0]
Expand Down Expand Up @@ -232,8 +231,8 @@ func GetDefinitionType(def oval.Definition) (string, error) {
return match[1], nil
}

// ParseUnpatchedCVEComponents parse components and resolution states of these components in an OVAL definition
func ParseUnpatchedCVEComponents(def oval.Definition) map[string]string {
// getComponentResolutions parses the given oval.Definition to determine a mapping from component to its resolution state.
func getComponentResolutions(def oval.Definition) map[string]string {
resolutions := def.Advisory.Affected.Resolutions
if len(resolutions) == 0 {
return nil
Expand All @@ -247,6 +246,9 @@ func ParseUnpatchedCVEComponents(def oval.Definition) map[string]string {
for _, component := range components {
stringSlice := strings.Split(component, "/")
var componentName string
// Some components are prefixed with superfluous information followed by a /.
// For example, virt:rhel/qemu-guest-agent. This is prefixed with unnecessary information
// about the environment.
if len(stringSlice) > 1 {
componentName = stringSlice[len(stringSlice)-1]
} else {
Expand Down
40 changes: 19 additions & 21 deletions pkg/rhelv2/ovalutil/rpm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ package ovalutil
import (
"encoding/xml"
"os"
"path/filepath"
"runtime"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/quay/goval-parser/oval"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

type definitionTypeTestCase struct {
Expand Down Expand Up @@ -80,31 +83,26 @@ func TestGetDefinitionType(t *testing.T) {
}
}

func TestParseUnpatchedCVEComponents(t *testing.T) {
f, err := os.Open("../../../testdata/cve/RHEL-8-including-unpatched-test.xml")
if err != nil {
t.Fatal(err)
}
func TestGetComponentResolutions(t *testing.T) {
_, filename, _, _ := runtime.Caller(0)
path := filepath.Dir(filename)

dataFilePath := filepath.Join(path, "/testdata/RHEL-8-including-unpatched-test.xml")
f, err := os.Open(dataFilePath)
require.NoError(t, err)
defer f.Close()

var root oval.Root
if err := xml.NewDecoder(f).Decode(&root); err != nil {
t.Fatal(err)
}
require.NoError(t, xml.NewDecoder(f).Decode(&root))

definitions := root.Definitions
definitions := root.Definitions.Definitions
nDefinitions := len(definitions)
assert.Lenf(t, definitions, 3, "Definition list length is incorrect, current definition list size is: %d", nDefinitions)

arr := definitions.Definitions
m := len(arr)
if !cmp.Equal(m, 3) {
t.Errorf("Definition list length is incorrect, current definition list size is: %d", m)
}
exampleSize := [3]int{1, 9, 1}
for i := 0; i < m; i++ {
componentMap := ParseUnpatchedCVEComponents(arr[i])
if !cmp.Equal(exampleSize[i], len(componentMap)) {
t.Errorf("Parsed CVE component map size is incorrect")
}
exampleSize := []int{1, 9, 1}
for i := 0; i < nDefinitions; i++ {
componentMap := getComponentResolutions(definitions[i])
assert.Equal(t, exampleSize[i], len(componentMap))
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@


<?xml version="1.0" encoding="utf-8"?>
<oval_definitions xmlns="http://oval.mitre.org/XMLSchema/oval-definitions-5" xmlns:oval="http://oval.mitre.org/XMLSchema/oval-common-5" xmlns:unix-def="http://oval.mitre.org/XMLSchema/oval-definitions-5#unix" xmlns:red-def="http://oval.mitre.org/XMLSchema/oval-definitions-5#linux" xmlns:ind-def="http://oval.mitre.org/XMLSchema/oval-definitions-5#independent" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://oval.mitre.org/XMLSchema/oval-common-5 oval-common-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5 oval-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#unix unix-definitions-schema.xsd http://oval.mitre.org/XMLSchema/oval-definitions-5#linux linux-definitions-schema.xsd">
<generator>
Expand Down