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(oracle-oval): Support multiple ELSAs per CVE #221

Merged
merged 19 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from 15 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
183 changes: 161 additions & 22 deletions pkg/vulnsrc/oracle-oval/oracle-oval.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ import (
"strings"

version "github.com/knqyf263/go-rpm-version"
"github.com/samber/lo"
bolt "go.etcd.io/bbolt"
"golang.org/x/exp/maps"
"golang.org/x/xerrors"

"github.com/aquasecurity/trivy-db/pkg/db"
"github.com/aquasecurity/trivy-db/pkg/types"
"github.com/aquasecurity/trivy-db/pkg/utils"
ustrings "github.com/aquasecurity/trivy-db/pkg/utils/strings"
"github.com/aquasecurity/trivy-db/pkg/vulnsrc/vulnerability"
)

Expand All @@ -34,10 +35,10 @@ var (
)

type PutInput struct {
VulnID string // CVE-ID or ELSA-ID
Vuln types.VulnerabilityDetail // vulnerability detail such as CVSS and description
Advisories map[AffectedPackage]types.Advisory // pkg => advisory
OVAL OracleOVAL // for extensibility, not used in trivy-db
VulnID string // CVE-ID or ELSA-ID
Vuln types.VulnerabilityDetail // vulnerability detail such as CVSS and description
Advisories map[Package]types.Advisories // pkg => advisories
OVALs []OracleOVAL // for extensibility, not used in trivy-db
}

type DB interface {
Expand Down Expand Up @@ -111,6 +112,8 @@ func (vs *VulnSrc) put(ovals []OracleOVAL) error {
}

func (vs *VulnSrc) commit(tx *bolt.Tx, ovals []OracleOVAL) error {
// CVE -> PutInput
putInputs := make(map[string]PutInput)
for _, oval := range ovals {
elsaID := strings.Split(oval.Title, ":")[0]

Expand All @@ -122,14 +125,14 @@ func (vs *VulnSrc) commit(tx *bolt.Tx, ovals []OracleOVAL) error {
vulnIDs = append(vulnIDs, elsaID)
}

advisories := map[AffectedPackage]types.Advisory{}
advisories := map[Package]types.Advisories{}
affectedPkgs := walkOracle(oval.Criteria, "", []AffectedPackage{})
for _, affectedPkg := range affectedPkgs {
if affectedPkg.Package.Name == "" {
continue
}

platformName := affectedPkg.PlatformName()
platformName := affectedPkg.Package.PlatformName()
if !slices.Contains(targetPlatforms, platformName) {
continue
}
Expand All @@ -138,9 +141,18 @@ func (vs *VulnSrc) commit(tx *bolt.Tx, ovals []OracleOVAL) error {
return xerrors.Errorf("failed to put data source: %w", err)
}

advisories[affectedPkg] = types.Advisory{
FixedVersion: affectedPkg.Package.FixedVersion,
advs := types.Advisories{
Entries: []types.Advisory{
{
FixedVersion: affectedPkg.FixedVersion,
},
},
}
if savedAdvs, ok := advisories[affectedPkg.Package]; ok {
advs.Entries = append(advs.Entries, savedAdvs.Entries...)
}
advisories[affectedPkg.Package] = advs

}

var references []string
Expand All @@ -156,21 +168,118 @@ func (vs *VulnSrc) commit(tx *bolt.Tx, ovals []OracleOVAL) error {
Severity: severityFromThreat(oval.Severity),
}

err := vs.Put(tx, PutInput{
input := PutInput{
VulnID: vulnID,
Vuln: vuln,
Advisories: advisories,
OVAL: oval,
})
if err != nil {
return xerrors.Errorf("db put error: %w", err)
Advisories: maps.Clone(advisories),
OVALs: []OracleOVAL{oval},
}

if savedInput, ok := putInputs[input.VulnID]; ok {
input.OVALs = append(input.OVALs, savedInput.OVALs...)

for inputPkg, inputAdvs := range input.Advisories {
if savedPkgAdvs, pkgFound := savedInput.Advisories[inputPkg]; pkgFound {
inputAdvs.Entries = append(savedPkgAdvs.Entries, inputAdvs.Entries...)
}
savedInput.Advisories[inputPkg] = inputAdvs
}
input.Advisories = savedInput.Advisories
}
putInputs[input.VulnID] = input
}
}

for _, input := range putInputs {
for pkg, advs := range input.Advisories {
input.Advisories[pkg] = resolveAdvisoriesEntries(advs)
}

err := vs.Put(tx, input)
if err != nil {
return xerrors.Errorf("db put error: %w", err)
}
}

return nil
}

// resolveAdvisoriesEntries removes entries with the same fixedVersion.
// Additionally, it only selects the latest fixedVersion for each flavor.
func resolveAdvisoriesEntries(advisories types.Advisories) types.Advisories {
fixedVersions := lo.Map(advisories.Entries, func(entry types.Advisory, _ int) string {
return entry.FixedVersion
})
fixedVer, resolvedVers := resolveVersions(fixedVersions)
entries := lo.Map(resolvedVers, func(ver string, _ int) types.Advisory {
return types.Advisory{
FixedVersion: ver,
}
})
return types.Advisories{
FixedVersion: fixedVer,
Entries: entries,
}
}

// resolveVersions removes duplicates and returns normal flavor + only one version for each flavor.
func resolveVersions(vers []string) (string, []string) {
vers = lo.Uniq(vers)

fixedVers := make(map[PkgFlavor]string)
for _, ver := range vers {
flavor := PackageFlavor(ver)
if savedVer, ok := fixedVers[flavor]; ok {
v := version.NewVersion(ver)
sv := version.NewVersion(savedVer)
if v.LessThan(sv) {
ver = savedVer
}
}
fixedVers[flavor] = ver
}

versions := lo.Values(fixedVers)
slices.Sort(versions)

fixedVersion := fixedVers[NormalPackageFlavor]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this case.
Should we use fips or ksplice version here?

example - https://linux.oracle.com/errata/ELSA-2016-3515.html

Copy link
Collaborator

Choose a reason for hiding this comment

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

The current DB contains fixed versions for fips or ksplice in this case, right? If so, we should keep it for compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not entirely correct (which is why this PR was created), but it is only needed for backward compatibility, so let's leave the previous logic
Updated in cb14132


return fixedVersion, versions
}

type PkgFlavor string

const (
NormalPackageFlavor PkgFlavor = "normal"
FipsPackageFlavor PkgFlavor = "fips"
Ksplice1PackageFlavor PkgFlavor = "ksplice1"
Ksplice2PackageFlavor PkgFlavor = "ksplice2"
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't use ksplice1 flavor for ksplice2 (and vice versa) in Trivy:
https://github.com/aquasecurity/trivy/blob/dd0a64a1cf0cd76e6f81e3ff55fa6ccb95ce3c3d/pkg/detector/ospkg/oracle/oracle.go#L72-L77

So i separated ksplice1 and ksplice2.
After these changes db size increased to 0.9MB:

➜  du -sh ./assets-with-flavors
539M	./assets-with-flavors
➜  du -sh ./assets             
540M	./assets 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's been long enough that I've lost track of all the details on this, but I did want to pass along some information I got from @tvierling (from the Oracle Linux support/development group) when I was working though this. I won't quote his email directly without his permission, but the gist of it is that ksplice is the flavor, and the number after ksplice is just a revision / build number (which should be considered.. x.y.z.ksplice2 > x.y.z.ksplice1). So I don't think you want to separate out as different flavors.

Copy link
Contributor

@DmitriyLewen DmitriyLewen Nov 5, 2024

Choose a reason for hiding this comment

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

Hello @bpfoster
I'm glad you're still here and helping with this task.
Sorry we're working on this after so long...

Also, I want to thank you for your work! Our work with you helped me remember the details and update this PR.

but the gist of it is that ksplice is the flavor, and the number after ksplice is just a revision / build number (which should be considered.. x.y.z.ksplice2 > x.y.z.ksplice1). So I don't think you want to separate out as different flavors.

I originally used your PackageFlavor function.
But I was confused that we separate ksplice1 and ksplice2 in Trivy and i updated PR.

But since the person from Oracle Linux says that we shouldn't divide them, then there is a mistake in our logic Trivy.

Thanks! I will revert last commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

done - a135499

)

// PackageFlavor determinants the package "flavor" based on its version string
// - normal
// - FIPS validated
// - ksplice1 userspace
// - ksplice2 userspace
func PackageFlavor(version string) PkgFlavor {
version = strings.ToLower(version)
if strings.HasSuffix(version, "_fips") {
return FipsPackageFlavor
}

subs := strings.Split(version, ".")
for _, s := range subs {
if strings.HasPrefix(s, "ksplice1") {
return Ksplice1PackageFlavor
}

if strings.HasPrefix(s, "ksplice2") {
return Ksplice2PackageFlavor
}
}
return NormalPackageFlavor
}

func (o *Oracle) Put(tx *bolt.Tx, input PutInput) error {
if err := o.PutVulnerabilityDetail(tx, input.VulnID, source.ID, input.Vuln); err != nil {
return xerrors.Errorf("failed to save Oracle Linux OVAL vulnerability: %w", err)
Expand All @@ -183,7 +292,7 @@ func (o *Oracle) Put(tx *bolt.Tx, input PutInput) error {

for pkg, advisory := range input.Advisories {
platformName := pkg.PlatformName()
if err := o.PutAdvisoryDetail(tx, input.VulnID, pkg.Package.Name, []string{platformName}, advisory); err != nil {
if err := o.PutAdvisoryDetail(tx, input.VulnID, pkg.Name, []string{platformName}, advisory); err != nil {
return xerrors.Errorf("failed to save Oracle Linux advisory: %w", err)
}
}
Expand All @@ -192,10 +301,36 @@ func (o *Oracle) Put(tx *bolt.Tx, input PutInput) error {

func (o *Oracle) Get(release string, pkgName string) ([]types.Advisory, error) {
bucket := fmt.Sprintf(platformFormat, release)
advisories, err := o.GetAdvisories(bucket, pkgName)
rawAdvisories, err := o.ForEachAdvisory([]string{bucket}, pkgName)
if err != nil {
return nil, xerrors.Errorf("failed to get Oracle Linux advisories: %w", err)
return nil, xerrors.Errorf("unable to iterate advisories: %w", err)
}
var advisories []types.Advisory
for vulnID, v := range rawAdvisories {
var adv types.Advisories
if err = json.Unmarshal(v.Content, &adv); err != nil {
return nil, xerrors.Errorf("failed to unmarshal advisory JSON: %w", err)
}

// For backward compatibility
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is needed only when users keep using old databases, right? I don't think people use the old database for a month. We can set a deadline.

Suggested change
// For backward compatibility
// For backward compatibility (This code can be deleted after Dec 19th, 2024)

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right.
Updated.

// The old trivy-db has no entries, but has fixed versions and custom fields.
if len(adv.Entries) == 0 {
advisories = append(advisories, types.Advisory{
VulnerabilityID: vulnID,
FixedVersion: adv.FixedVersion,
DataSource: &v.Source,
Custom: adv.Custom,
})
continue
}

for _, entry := range adv.Entries {
entry.VulnerabilityID = vulnID
entry.DataSource = &v.Source
advisories = append(advisories, entry)
}
}

return advisories, nil
}

Expand All @@ -211,11 +346,11 @@ func walkOracle(cri Criteria, osVer string, pkgs []AffectedPackage) []AffectedPa
}

pkgs = append(pkgs, AffectedPackage{
OSVer: osVer,
Package: Package{
Name: ss[0],
FixedVersion: version.NewVersion(ss[1]).String(),
Name: ss[0],
OSVer: osVer,
},
FixedVersion: version.NewVersion(ss[1]).String(),
})
}

Expand All @@ -234,7 +369,11 @@ func referencesFromContains(sources []string, matches []string) []string {
}
}
}
return ustrings.Unique(references)

references = lo.Uniq(references)
slices.Sort(references)

return references
}

func severityFromThreat(sev string) types.Severity {
Expand Down
Loading