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

rhel: add csaf/vex updater #1165

Merged
merged 4 commits into from
Aug 19, 2024
Merged

rhel: add csaf/vex updater #1165

merged 4 commits into from
Aug 19, 2024

Conversation

crozzy
Copy link
Contributor

@crozzy crozzy commented Nov 29, 2023

TODO

  • Use specific product CVSS scores and normalized them when cvss: add package for dealing with CVSS scores #1143 is merged
  • Delta updating needs to be implemented before this will be mergable
  • Implement ingestion of compressed VEX data (once available)
  • Finalize fetcher logic
  • Write fetcher tests once logic is finalized
  • Extend parser tests
  • Revisit repo matching logic
  • Add CSAF-VEX updater to defaults
  • Remove rhel updater from defaults
  • Delete rhel vulnerabilities

Copy link

codecov bot commented Nov 29, 2023

Codecov Report

Attention: Patch coverage is 58.13953% with 270 lines in your changes missing coverage. Please review.

Project coverage is 55.35%. Comparing base (a3c7415) to head (8172e8b).
Report is 6 commits behind head on main.

Files Patch % Lines
rhel/vex/fetcher.go 53.16% 62 Missing and 49 partials ⚠️
rhel/vex/parser.go 64.56% 85 Missing and 22 partials ⚠️
rhel/vex/updater.go 53.73% 27 Missing and 4 partials ⚠️
rhel/matcherfactory.go 0.00% 14 Missing ⚠️
rhel/matcher.go 65.00% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1165      +/-   ##
==========================================
+ Coverage   55.25%   55.35%   +0.10%     
==========================================
  Files         278      282       +4     
  Lines       17199    17834     +635     
==========================================
+ Hits         9503     9872     +369     
- Misses       6738     6928     +190     
- Partials      958     1034      +76     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rhel/vex/fetcher.go Outdated Show resolved Hide resolved
rhel/vex/fetcher.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
@crozzy crozzy force-pushed the csaf-vex branch 2 times, most recently from 7d90161 to a5d8a67 Compare July 26, 2024 20:51
@crozzy crozzy requested a review from RTann July 26, 2024 21:09
@crozzy
Copy link
Contributor Author

crozzy commented Jul 26, 2024

@RTann 1d2ccd3 comments are addressed in this fixup or in-line

@crozzy crozzy force-pushed the csaf-vex branch 6 times, most recently from 272c600 to 7b20cac Compare July 29, 2024 22:10
rhel/matcher.go Outdated
// 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 SUBSET(⊂)(source is a subset of, or equal to the target).
Copy link
Member

Choose a reason for hiding this comment

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

Should be ⊇ (or the spec SUPERSET) -- "Source is a superset or equal to the target"

rhel/matcher.go Show resolved Hide resolved
@crozzy crozzy force-pushed the csaf-vex branch 5 times, most recently from d43b87e to ea4747f Compare August 2, 2024 21:00
rhel/vex/updater.go Show resolved Hide resolved
rhel/matcher_test.go Show resolved Hide resolved
Comment on lines 154 to 156
// Here we are construct new-line-delimited JSON by first compacting the
// JSON from the file, writing a newline to the bc buf and appending the
// compacted JSON, and finally writing all those bytes to the snappy.Writer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Here we are construct new-line-delimited JSON by first compacting the
// JSON from the file, writing a newline to the bc buf and appending the
// compacted JSON, and finally writing all those bytes to the snappy.Writer.
// Here we construct new-line-delimited JSON by first compacting the
// JSON from the file, writing a newline to the bc buf and appending the
// compacted JSON, and finally writing all those bytes to the snappy.Writer.

Also I think the ordering here is incorrect. First the compacted JSON is written, then the newline

rhel/vex/fetcher.go Show resolved Hide resolved
// DeltaParse implements [driver.DeltaUpdater].
func (u *Updater) DeltaParse(ctx context.Context, contents io.ReadCloser) ([]*claircore.Vulnerability, []string, error) {
ctx = zlog.ContextWithValues(ctx, "component", "rhel/vex/Updater.DeltaParse")
// This map is needed for deduplication purposes, the compressed CSAF data maybe include
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// This map is needed for deduplication purposes, the compressed CSAF data maybe include
// This map is needed for deduplication purposes, the compressed CSAF data may include

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

rhel/vex/parser.go Show resolved Hide resolved
Replace the Red Hat OVALv2 update source with the Red Hat CSAF/VEX data.

Signed-off-by: crozzy <joseph.crosland@gmail.com>
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 <joseph.crosland@gmail.com>
Previously the IgnoreUnpatched config key was a part of the RHEL
updater and would dictate whether or not the updater would ingest
unpatched vulnerabilities. This change moves that key to the RHEL
matcher and dictates whether the matcher should check for a
fixed_in_version when querying potential vulnerabilities. This makes the
config option more usable at the expense of DB size.

Signed-off-by: crozzy <joseph.crosland@gmail.com>
Given that the rhel-vex data will be responsible for Red Hat
vulnerabilities we no longer want the existing OVAL updater to be a
default (or even selectable). This patch also removes existing RHEL OVAL
data from the matcher DB.

Signed-off-by: crozzy <joseph.crosland@gmail.com>
@crozzy
Copy link
Contributor Author

crozzy commented Aug 14, 2024

@RTann I can't reply in-line to this comment

out of curiosity, if we were to decide to change this date, would there be any issues related to deleting old entries out-of-the-box? For example, if this were moved to 2015? Or perhaps even earlier? Would any special care be required (a new migration step)?

Yes, we can move it, if you're going earlier you shouldn't need to change anything, if you're going later and you want earlier vulns to not be available for matching anymore you can iterate the version (updaterVersion), then vulns should shuffle off the GC coil.

@crozzy crozzy requested review from hdonnay and RTann August 14, 2024 19:17
Comment on lines +63 to +66
links := []string{}
for _, r := range v.References {
links = append(links, r.URL)
}
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 referring to this "self" under vulnerabilities.references:

"references": [
        {
          "category": "self",
          "summary": "Canonical URL",
          "url": "https://access.redhat.com/security/cve/CVE-2024-3727"
        },
        {
          "category": "external",
          "summary": "RHBZ#2274767",
          "url": "https://bugzilla.redhat.com/show_bug.cgi?id=2274767"
        },

not sure if it's guaranteed to be first, but it's definitely the one users will care about most. Though, that can just be the client's problem to fetch it, if that's the one they want (for example StackRox can just search for the link prefixed with https://access.redhat.com/security/cve/CVE-

Copy link
Member

@hdonnay hdonnay left a comment

Choose a reason for hiding this comment

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

LGTM

@crozzy crozzy merged commit daaf5fd into quay:main Aug 19, 2024
8 checks passed
@crozzy crozzy removed the needs-changelog Label for PRs that need a changelog note. label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants