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

Add java.Matcher configuration to includes maven upstream sha1 query #714

Merged
merged 17 commits into from
Apr 13, 2022

Conversation

spiffcs
Copy link
Contributor

@spiffcs spiffcs commented Apr 8, 2022

Add Grype Matcher configuration for upstream maven query based on SHA1

Syft is now able to generate sha1 digests for discovered java archives. These digests are included along with the java archive metadata output.
anchore/syft#941

This PR adds the ability for the user to configure the java.Matcher to query the maven upstream repository for additional data that may not be present at time of SBOM generation.

Specifically, sboms of java archives are sometimes limited in their details when pom.xml or pom.properties are not present at the time of analysis. Querying maven given an archives sha1 digest value allows users to add fidelity to their vulnerability report when matching against data not relying on CPE generation (GHSA or GL Advisory Database).

It should be noted that a NEW package is not generated for the grype output. Matches found with this method are assigned directly to the java package that presented the sha1 for search. No duplication should be caused from doing this additional query.

The .grype.yaml configuration has been updated to include the following block:

external-sources:
  enable: true
  maven:
    search-maven-upstream: true
    base-url: "https://search.maven.org/solrsearch/select"

Features added

  • Matcher Config options at the app config level
  • Maven upstream package search with vulnerability comparison

TODO

  • Test Updates

Demo

Disabled - 606 vulnerabilities reported:
Screen Shot 2022-04-11 at 3 25 58 PM

Enabled - 682 vulnerabilities reported:
Screen Shot 2022-04-11 at 3 26 07 PM

Yardstick Analysis vs current release and branch:

This shows all of the additional vulnerabilities captured when enabling upstream matching:

Screen Shot 2022-04-11 at 6 01 37 PM

None of these findings appear to be FP and can all be accurately attributed to the image.

What's interesting here is that after running yardstick the new branch shows 22 new vulnerabilities reported. The output of the respective commands (current release vs branch) shows a difference of 76 when comparing the Scanned image [xxx vulnerabilities] field (606 vs 682). This disparity has been found to be related to additional relatedVulnerabiities being populated as a result of enabling the upstream source. When searching by sha and matching with GHSA is also will generate a new relatedVulnerability for the NVD source. New vulnerabilities PLUS packages that can now be found via the GHSA and not just CPE account for this increase in the output.

I have found that the branch results will overwrite exact-direct-match with exact-indirect-match if a vulnerability is found upstream via the checksum search.

Signed-off-by: Christopher Phillips christopher.phillips@anchore.com

spiffcs added 4 commits April 8, 2022 11:32
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs changed the title update java metadata decoding for syft-json Add grype java.Matcher configuration with ability to query upstream maven repository Apr 8, 2022
@spiffcs spiffcs changed the title Add grype java.Matcher configuration with ability to query upstream maven repository Add grype java.Matcher configuration (Includes maven upstream sha1 query) Apr 8, 2022
spiffcs and others added 3 commits April 8, 2022 15:24
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs force-pushed the 711-External-Maven-Data-Source branch from 7fd3f09 to b5c98f1 Compare April 8, 2022 19:42
@spiffcs spiffcs linked an issue Apr 11, 2022 that may be closed by this pull request
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs force-pushed the 711-External-Maven-Data-Source branch from 8323017 to 2884e42 Compare April 11, 2022 18:59
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs force-pushed the 711-External-Maven-Data-Source branch from 2884e42 to c3b3276 Compare April 11, 2022 19:08
@spiffcs spiffcs marked this pull request as ready for review April 11, 2022 19:26
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs requested a review from a team April 11, 2022 19:29
@@ -9,8 +9,8 @@ require (
github.com/anchore/go-testutils v0.0.0-20200925183923-d5f45b0d3c04
github.com/anchore/go-version v1.2.2-0.20210903204242-51efa5b487c4
github.com/anchore/packageurl-go v0.1.1-0.20220314153042-1bcd40e5206b
github.com/anchore/stereoscope v0.0.0-20220330165332-7fc73ee7b0f0
github.com/anchore/syft v0.43.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update on next syft release

Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

Only other thing I'll note at the moment is that we'll probably want to update the README with this one. Removing global state is MEGA-👍, though!

spiffcs added 3 commits April 11, 2022 17:23
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs force-pushed the 711-External-Maven-Data-Source branch from 08bc8c6 to ab786ce Compare April 12, 2022 16:43
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Copy link
Contributor

@wagoodman wagoodman left a comment

Choose a reason for hiding this comment

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

great maven search addition, and love that this paves the way to exposing user-configuration for matchers in the future


func (cfg externalSources) loadDefaultValues(v *viper.Viper) {
v.SetDefault("external-sources.enable", false)
v.SetDefault("external-sources.maven.search-maven-upstream", false)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: since we have the master switch (which is great) we could have the maven search option default to on

Copy link
Contributor

Choose a reason for hiding this comment

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

I strongly +1 this

}

type maven struct {
SearchMavenUpstream bool `yaml:"search-maven-upstream" json:"search_maven_upstream" mapstructure:"search-maven-upstream"`
Copy link
Contributor

@wagoodman wagoodman Apr 12, 2022

Choose a reason for hiding this comment

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

nit: the full config path stutters: external-sources.maven.search-maven-upstream with maven showing up twice.
maybe rename the field to search-upstream? Or more descriptive to the what or how of the operation, such as search-by-sha1 or find-artifact-group-id (not saying these suggestions are necessarily better, but wanted to give an idea)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea nice suggestion. I'll update the config here.

@spiffcs
Copy link
Contributor Author

spiffcs commented Apr 12, 2022

TODO before merge and thanks for the 🟢 @wagoodman and @kzantow :

  • config update to not stutter
  • default updates
  • test behavior correction
  • update syft release

@kzantow
Copy link
Contributor

kzantow commented Apr 12, 2022

@spiffcs I'd like to see two more things:

  • make sure the appropriate environment variables work
  • make sure the README is updated

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs
Copy link
Contributor Author

spiffcs commented Apr 13, 2022

Still debugging these env variables

@spiffcs
Copy link
Contributor Author

spiffcs commented Apr 13, 2022

@kzantow this should work for you GRYPE_EXTERNAL_SOURCES_ENABLE=true I think the env variables need to be prefixed with GRYPE
Screen Shot 2022-04-13 at 11 57 52 AM

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs force-pushed the 711-External-Maven-Data-Source branch from dab92ea to 56bbeff Compare April 13, 2022 15:57
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs changed the title Add grype java.Matcher configuration (Includes maven upstream sha1 query) Add java.Matcher configuration to includes maven upstream sha1 query Apr 13, 2022
@spiffcs spiffcs merged commit 95f68b4 into main Apr 13, 2022
@spiffcs spiffcs deleted the 711-External-Maven-Data-Source branch April 13, 2022 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optional External Data Source Reference for Maven Packages
4 participants