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

feat: filter out packages owned by OS packages #1387

Merged
merged 24 commits into from
Aug 18, 2023
Merged

Conversation

willmurphyscode
Copy link
Contributor

@willmurphyscode willmurphyscode commented Jul 13, 2023

For example, if the rpm "python3-rpm" is installed, it brings a python package called "rpm" with it, which is just python bindings to RPM. But this python package is part of "python3-rpm", and should not be matched against directly.

Fixes #1373
#1362 won't be fixed anymore because of the comprehensive distro check.

grype/pkg/package.go Outdated Show resolved Hide resolved
@willmurphyscode willmurphyscode self-assigned this Jul 13, 2023
@willmurphyscode willmurphyscode changed the title Filter out packages owned by OS packages feat: filter out packages owned by OS packages Jul 20, 2023
@willmurphyscode
Copy link
Contributor Author

The quality gate is failing in an interesting way. Let's take this example:

Quality gate failure snippet:

Testing image: docker.io/sonatype/nexus3@sha256:e8fea6b4279f2b5b24b36170459cb7aa3d6afe999f9d3e3713541be28bae8ec4
with syft@v0.74.1
with grype@65d27af2d161adce9e8c9cb972fc0e50aff6040f
with grype@latest

Running relative comparison...
Results used:
├── a10b0d0a-8944-4747-84c5-f83641583d65 : grype@v0.64.2-7-g65d27af against docker.io/sonatype/nexus3@sha256:e8fea6b4279f2b5b24b36170459cb7aa3d6afe999f9d3e3713541be28bae8ec4
└── 4142c1de-9334-476a-b077-804344861558 : grype@v0.64.2 against docker.io/sonatype/nexus3@sha256:e8fea6b4279f2b5b24b36170459cb7aa3d6afe999f9d3e3713541be28bae8ec4

Running comparison against labels...
Results used:
├── a10b0d0a-8944-4747-84c5-f83641583d65 : grype@v0.64.2-7-g65d27af against docker.io/sonatype/nexus3@sha256:e8fea6b4279f2b5b24b36170459cb7aa3d6afe999f9d3e3713541be28bae8ec4
└── 4142c1de-9334-476a-b077-804344861558 : grype@v0.64.2 against docker.io/sonatype/nexus3@sha256:e8fea6b4279f2b5b24b36170459cb7aa3d6afe999f9d3e3713541be28bae8ec4

Match differences between tooling (with labels):

   TOOL PARTITION      PACKAGE         VULNERABILITY        LABEL         COMMENTARY
   grype@v0.64.2 ONLY  rpm@4.14.3      CVE-2021-20266       TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  rpm@4.14.3      CVE-2021-3421        TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  rpm@4.14.3      CVE-2021-3521        TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  rpm@4.14.3      CVE-2021-35937       TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  rpm@4.14.3      CVE-2021-35938       TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  rpm@4.14.3      CVE-2021-35939       TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  urllib3@1.24.2  CVE-2019-11236       TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  urllib3@1.24.2  CVE-2020-26137       TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  urllib3@1.24.2  CVE-2021-33503       TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  urllib3@1.24.2  GHSA-r64q-w8jr-g9qp  TruePositive  (this is a new FN 😱)
   grype@v0.64.2 ONLY  urllib3@1.24.2  GHSA-wqvq-5m8c-6g24  TruePositive  (this is a new FN 😱)

But the code still finds these vulnerabilites. Let's take 2 examples:

❯ grype -q willtmp/sonatype-nexus.tar| rg CVE-2021-3421
python3-rpm                             4.14.3-4.el8             0:4.14.3-14.el8_4        rpm           CVE-2021-3421        Medium    
rpm                                     4.14.3                                            python        CVE-2021-3421        Medium    
rpm                                     4.14.3-4.el8             0:4.14.3-14.el8_4        rpm           CVE-2021-3421        Medium    
rpm-build-libs                          4.14.3-4.el8             0:4.14.3-14.el8_4        rpm           CVE-2021-3421        Medium    
rpm-libs                                4.14.3-4.el8             0:4.14.3-14.el8_4        rpm           CVE-2021-3421        Medium    

❯ go run cmd/grype/main.go -q willtmp/sonatype-nexus.tar| rg CVE-2021-3421
python3-rpm                             4.14.3-4.el8             0:4.14.3-14.el8_4        rpm           CVE-2021-3421        Medium    
rpm                                     4.14.3-4.el8             0:4.14.3-14.el8_4        rpm           CVE-2021-3421        Medium    
rpm-build-libs                          4.14.3-4.el8             0:4.14.3-14.el8_4        rpm           CVE-2021-3421        Medium    
rpm-libs                                4.14.3-4.el8             0:4.14.3-14.el8_4        rpm           CVE-2021-3421        Medium    

❯ grype -q willtmp/sonatype-nexus.tar| rg CVE-2021-3521                   
python3-rpm                             4.14.3-4.el8             0:4.14.3-19.el8_5.2      rpm           CVE-2021-3521        Medium    
rpm                                     4.14.3                                            python        CVE-2021-3521        Medium    
rpm                                     4.14.3-4.el8             0:4.14.3-19.el8_5.2      rpm           CVE-2021-3521        Medium    
rpm-build-libs                          4.14.3-4.el8             0:4.14.3-19.el8_5.2      rpm           CVE-2021-3521        Medium    
rpm-libs                                4.14.3-4.el8             0:4.14.3-19.el8_5.2      rpm           CVE-2021-3521        Medium    

❯ go run cmd/grype/main.go -q willtmp/sonatype-nexus.tar| rg CVE-2021-3521
python3-rpm                             4.14.3-4.el8             0:4.14.3-19.el8_5.2      rpm           CVE-2021-3521        Medium    
rpm                                     4.14.3-4.el8             0:4.14.3-19.el8_5.2      rpm           CVE-2021-3521        Medium    
rpm-build-libs                          4.14.3-4.el8             0:4.14.3-19.el8_5.2      rpm           CVE-2021-3521        Medium    
rpm-libs                                4.14.3-4.el8             0:4.14.3-19.el8_5.2      rpm           CVE-2021-3521        Medium

The image is vulnerability, and both the latest release of grype and this feature branch report the image as vulnerable. But this feature branch (correctly, I think) reports 1 fewer affected packages, because the python package rpm is merged into the RPM python3-rpm before matching. So the matches need to be updated, not because the feature branch no longer reports the vulnerability, but because it now reports the vulnerability for 1 fewer package, at least for the nvd:cpe ones.

@willmurphyscode
Copy link
Contributor Author

Let's look at the GHSAs, since this branch no longer finds them. First up is GHSA-r64q-w8jr-g9qp:

❯ go run cmd/grype/main.go -q willtmp/sonatype-nexus.tar| rg GHSA-r64q-w8jr-g9qp # prints nothing
❯ grype -q willtmp/sonatype-nexus.tar| rg GHSA-r64q-w8jr-g9qp                   
urllib3                                 1.24.2                   1.24.3                   python        GHSA-r64q-w8jr-g9qp  Medium    
sqlite> select id, package_name, namespace, package_qualifiers, version_constraint 
from vulnerability where 
(id like 'CVE-2019-11236' and (namespace like 'nvd:cpe' or namespace like 'redhat:distro:redhat:8'))
or id like 'GHSA-r64q-w8jr-g9qp';
id                   package_name    namespace               package_qualifiers  version_constraint
-----------------    --------------  ----------------------  ------------------  ------------------
CVE-2019-11236       urllib3         nvd:cpe                                     <= 1.24.2         
GHSA-r64q-w8jr-g9qp  urllib3         github:language:python                      <=1.24.2          
CVE-2019-11236       python-urllib3  redhat:distro:redhat:8                      < 0:1.24.2-2.el8  
CVE-2019-11236       python-pip      redhat:distro:redhat:8                      < 0:9.0.3-16.el8  

So let's take a look at the syft packages that are here:

❯ syft packages -q ../grype1373/willtmp/sonatype-nexus.tar| rg urllib3
python3-urllib3                                    1.24.2-4.el8                      rpm
urllib3                                            1.24.2                            python

So here the rpm python-urllib3@1.24.2-4.el8 is fixed, so this was a false positive introduced by the duplication
of treating the rpm python-urllib3 as if it were also urllib3 from PyPI. In other words, RedHat patched their
RPM, and our database knew it, but was reporting a false positive because this package was treated as if it was from
the RedHat RPM and from PyPI.

@willmurphyscode
Copy link
Contributor Author

Looking at the other GHSA, GHSA-wqvq-5m8c-6g24:

sqlite> select id, package_name, namespace, package_qualifiers, version_constraint 
from vulnerability where 
(id like 'CVE-2020-26137' and (namespace like 'nvd:cpe' or namespace like 'redhat:distro:redhat:8'))
or id like 'GHSA-wqvq-5m8c-6g24';
id                   package_name                                                  namespace               package_qualifiers  version_constraint
-------------------  ------------------------------------------------------------  ----------------------  ------------------  ------------------
CVE-2020-26137       communications_cloud_native_core_network_function_cloud_nati  nvd:cpe                                     = 22.2.0          
                  ve_environment                                                                                                              

CVE-2020-26137       zfs_storage_appliance_kit                                     nvd:cpe                                     = 8.8             

CVE-2020-26137       urllib3                                                       nvd:cpe                                     < 1.25.9          

GHSA-wqvq-5m8c-6g24  urllib3                                                       github:language:python                      <1.25.9           

CVE-2020-26137       python-urllib3                                                redhat:distro:redhat:8                      < 0:1.24.2-5.el8  

Again, the RPM has an earlier fix version than the PyPI record, so this finding was always a false positive for the same reason as described in the previous comment.

@willmurphyscode willmurphyscode force-pushed the feat-grype1373 branch 3 times, most recently from 04df898 to 84deda4 Compare August 4, 2023 20:51
@willmurphyscode willmurphyscode force-pushed the feat-grype1373 branch 2 times, most recently from 3cfc36b to e558222 Compare August 16, 2023 20:33
Copy link
Contributor

@spiffcs spiffcs left a comment

Choose a reason for hiding this comment

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

This is great work @willmurphyscode - the persistence in getting the vulnerability labels updated and following through on different examples made for an easy review. I made some nit comments for discussion.

cc @wagoodman for a second 👀

grype/pkg/package.go Show resolved Hide resolved

catalog = removePackagesByOverlap(catalog, relationships)
catalog = removePackagesByOverlap(s)
Copy link
Contributor

@spiffcs spiffcs Aug 16, 2023

Choose a reason for hiding this comment

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

nit: what do you think of the return type here? we are passing in an sbom s only for sbom to be remade with the modified catalog on line 47 down below - what do you think about removePackagesByOerlap as a method on s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this. I didn't notice that we were re-assembling another SBOM struct below. I'll see if this can be easily rearranged.

kzantow
kzantow previously approved these changes Aug 16, 2023
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.

LGTM -- minor comment, which is mostly a preferential thing

if r.Type == artifact.OwnershipByFileOverlapRelationship {
byOverlap[r.To.ID()] = r
}
}

out := pkg.NewCollection()

comprehensiveDistroFeed := distroFeedIsComprehensive(sbm.Artifacts.LinuxDistribution)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems a little odd to add this call here rather than, say, passing the sbom to excludePackages on line 121

Copy link
Contributor

Choose a reason for hiding this comment

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

the nice thing about calling it here though is that excludePackage is minimizing what it needs as inputs.

@kzantow kzantow dismissed their stale review August 16, 2023 21:31

Alex should probably give the final review of this one

For example, if the rpm "python3-rpm" is installed, it brings a python
package called "rpm" with it, which is just python bindings to RPM. But
this python package is part of "python3-rpm", and should not be matched
against directly.

Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
These matches are excluded by the new behavior, but the SBOM format
used in these tests doesn't preserve enough information about the match.

Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
From the list of packages that are considered OS packages. The reason is
that the APK data feed includes fix info, but not underlying metadata
info, so we shouldn't prefer APK packages to ecosystem specific packages
the way we do for RPMs, for example.

Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Make it easier to investigate quality gate failures by printing
yardstick scan result IDs with each failure reason.

Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@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.

nice work 🙌

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.

Filter out packages that are owned by OS packages (ownership overlap)
4 participants