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 support for Yocto scanner #1085

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

panicking
Copy link
Contributor

@panicking panicking commented Aug 31, 2024

Yocto project support CVE security vulnerabilities using cve-check in the specific image or target you are building, add the following setting to your configuration:

INHERIT += "cve-check"

status of each CVE: Patched, Unpatched or Ignored

The scanner look only for Unpatched package and calculate the severity using the score_v2 or score_v3

Testing done

Deployed in internal company jenkins infrastructure and verify on real yocto product
Screenshot from 2024-09-10 22-04-52
Screenshot from 2024-09-10 22-05-04
Screenshot from 2024-09-10 22-05-14

@panicking panicking force-pushed the feature/add-yocto-cve-parser branch 2 times, most recently from 04c2d27 to 464e127 Compare August 31, 2024 12:17
Copy link

codecov bot commented Aug 31, 2024

Codecov Report

Attention: Patch coverage is 78.78788% with 14 lines in your changes missing coverage. Please review.

Project coverage is 90.63%. Comparing base (7eb9a14) to head (ea514ba).
Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
.../hm/hafner/analysis/parser/YoctoScannerParser.java 85.71% 1 Missing and 7 partials ⚠️
...fner/analysis/registry/YoctoScannerDescriptor.java 40.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1085      +/-   ##
============================================
- Coverage     90.74%   90.63%   -0.12%     
- Complexity     2322     2340      +18     
============================================
  Files           355      357       +2     
  Lines          6551     6617      +66     
  Branches        675      686      +11     
============================================
+ Hits           5945     5997      +52     
- Misses          410      417       +7     
- Partials        196      203       +7     

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

@panicking panicking force-pushed the feature/add-yocto-cve-parser branch from 464e127 to d1751b8 Compare August 31, 2024 19:16
@panicking
Copy link
Contributor Author

@uhafner Trying to figure out how to test everything to the warning plugin, there are 3 components that are involved 1) the model, 2) model-api plugin 3) warning-ng plugin. Can I have a local setup where all of those components are testable together? The reason I'm asking is because I'm integrate another vulnerabiliy tools and testing of them look really importat to me to fine tuning the information needs to be shown

@panicking panicking force-pushed the feature/add-yocto-cve-parser branch from d1751b8 to c16b789 Compare September 2, 2024 06:47
@uhafner
Copy link
Member

uhafner commented Sep 9, 2024

@uhafner Trying to figure out how to test everything to the warning plugin, there are 3 components that are involved 1) the model, 2) model-api plugin 3) warning-ng plugin. Can I have a local setup where all of those components are testable together? The reason I'm asking is because I'm integrate another vulnerabiliy tools and testing of them look really importat to me to fine tuning the information needs to be shown

I prepared a development environment that should help.

@uhafner uhafner added the feature New features label Sep 9, 2024
@uhafner uhafner changed the title Add YoctoScannerParser Add support for Yocto scanner Sep 9, 2024
@panicking
Copy link
Contributor Author

@uhafner Ok, I need to push a new version. I will include some screenshot. I would like to ask you if I can use html href in description for the vulnerabilities hyperlink and when I don't have filename what is the best approach, because make it empy is not a real nice result

@uhafner
Copy link
Member

uhafner commented Sep 10, 2024

@uhafner Ok, I need to push a new version. I will include some screenshot. I would like to ask you if I can use html href in description for the vulnerabilities hyperlink

Yes, that is ok. See ErrorProneParser for an example.

and when I don't have filename what is the best approach, because make it empy is not a real nice result

Isn't the package in your case the the actual affected file name?

@panicking
Copy link
Contributor Author

@uhafner Ok, I need to push a new version. I will include some screenshot. I would like to ask you if I can use html href in description for the vulnerabilities hyperlink

Yes, that is ok. See ErrorProneParser for an example.

and when I don't have filename what is the best approach, because make it empy is not a real nice result

Ok

Isn't the package in your case the the actual affected file name?

image

This is how is shown

@panicking panicking force-pushed the feature/add-yocto-cve-parser branch from c16b789 to 560ae43 Compare September 10, 2024 20:00
@uhafner
Copy link
Member

uhafner commented Sep 11, 2024

Thanks for the PR, the screenshots look good!

From my understanding (and to be more compatible with other vulnerability scanners like OwaspDependencyCheck) it would make more sense to use your package name as file name property and the CVE as type?

Here the test from OwaspDependencyCheck:

softly.assertThat(report.get(0))
                .hasFileName("commons-beanutils-1.8.3.jar")
                .hasSeverity(Severity.WARNING_HIGH)
                .hasCategory("NETWORK")
                .hasType("CVE-2014-0114")
                .hasMessage("Apache Commons BeanUtils, as distributed in lib/commons-beanutils-1.8.0.jar in Apache Struts 1.x through 1.3.10 and in other products requiring commons-beanutils through 1.9.2, does not suppress the class property, which allows remote attackers to \"manipulate\" the ClassLoader and execute arbitrary code via the class parameter, as demonstrated by the passing of this parameter to the getClass method of the ActionForm object in Struts 1.")
                .hasDescription("<a href=\"https://nvd.nist.gov/vuln/detail/CVE-2014-0114\">https://nvd.nist.gov/vuln/detail/CVE-2014-0114</a>");

Then your test would look like:

        softly.assertThat(report.get(0))
                .hasSeverity(Severity.WARNING_LOW)
                .hasFileName("acl")
                .hasType("CVE-2009-4411")
                .hasDescription("<div><b>Package: </b>acl</div> <div><b>Version: </b>2.3.2</div>"
                                + " <div><b>Link: </b><a href=\"https://nvd.nist.gov/vuln/detail/CVE-2009-4411\""
                                + ">https://nvd.nist.gov/vuln/detail/CVE-2009-4411</a></div>"
                                + " <div><b>Yocto Layer: </b>meta</div> <div><b>Vector: </b>LOCAL</div> <p>"
                                + "The (1) setfacl and (2) getfacl commands in XFS acl 2.2.47, when running"
                                + " in recursive (-R) mode, follow symbolic links even when the --physical"
                                + " (aka -P) or -L option is specified, which might allow local users to modify"
                                + " the ACL for arbitrary files or directories via a symlink attack.</p>");

Would that work?

I think I can remove the :0 for the file name attribute in the warnings plugin to make it more readable.

@panicking
Copy link
Contributor Author

Thanks for the PR, the screenshots look good!

From my understanding (and to be more compatible with other vulnerability scanners like OwaspDependencyCheck) it would make more sense to use your package name as file name property and the CVE as type?

Here the test from OwaspDependencyCheck:

softly.assertThat(report.get(0))
                .hasFileName("commons-beanutils-1.8.3.jar")
                .hasSeverity(Severity.WARNING_HIGH)
                .hasCategory("NETWORK")
                .hasType("CVE-2014-0114")
                .hasMessage("Apache Commons BeanUtils, as distributed in lib/commons-beanutils-1.8.0.jar in Apache Struts 1.x through 1.3.10 and in other products requiring commons-beanutils through 1.9.2, does not suppress the class property, which allows remote attackers to \"manipulate\" the ClassLoader and execute arbitrary code via the class parameter, as demonstrated by the passing of this parameter to the getClass method of the ActionForm object in Struts 1.")
                .hasDescription("<a href=\"https://nvd.nist.gov/vuln/detail/CVE-2014-0114\">https://nvd.nist.gov/vuln/detail/CVE-2014-0114</a>");

Then your test would look like:

        softly.assertThat(report.get(0))
                .hasSeverity(Severity.WARNING_LOW)
                .hasFileName("acl")
                .hasType("CVE-2009-4411")
                .hasDescription("<div><b>Package: </b>acl</div> <div><b>Version: </b>2.3.2</div>"
                                + " <div><b>Link: </b><a href=\"https://nvd.nist.gov/vuln/detail/CVE-2009-4411\""
                                + ">https://nvd.nist.gov/vuln/detail/CVE-2009-4411</a></div>"
                                + " <div><b>Yocto Layer: </b>meta</div> <div><b>Vector: </b>LOCAL</div> <p>"
                                + "The (1) setfacl and (2) getfacl commands in XFS acl 2.2.47, when running"
                                + " in recursive (-R) mode, follow symbolic links even when the --physical"
                                + " (aka -P) or -L option is specified, which might allow local users to modify"
                                + " the ACL for arbitrary files or directories via a symlink attack.</p>");

Would that work?

I think I can remove the :0 for the file name attribute in the warnings plugin to make it more readable.

In the new screenshot I have put as filename the vulnerability (make sense?) I will change it as you ask if I can not use as filename the CVE

@panicking
Copy link
Contributor Author

Thanks for the PR, the screenshots look good!

From my understanding (and to be more compatible with other vulnerability scanners like OwaspDependencyCheck) it would make more sense to use your package name as file name property and the CVE as type?

Here the test from OwaspDependencyCheck:

softly.assertThat(report.get(0))
                .hasFileName("commons-beanutils-1.8.3.jar")
                .hasSeverity(Severity.WARNING_HIGH)
                .hasCategory("NETWORK")
                .hasType("CVE-2014-0114")
                .hasMessage("Apache Commons BeanUtils, as distributed in lib/commons-beanutils-1.8.0.jar in Apache Struts 1.x through 1.3.10 and in other products requiring commons-beanutils through 1.9.2, does not suppress the class property, which allows remote attackers to \"manipulate\" the ClassLoader and execute arbitrary code via the class parameter, as demonstrated by the passing of this parameter to the getClass method of the ActionForm object in Struts 1.")
                .hasDescription("<a href=\"https://nvd.nist.gov/vuln/detail/CVE-2014-0114\">https://nvd.nist.gov/vuln/detail/CVE-2014-0114</a>");

Then your test would look like:

        softly.assertThat(report.get(0))
                .hasSeverity(Severity.WARNING_LOW)
                .hasFileName("acl")
                .hasType("CVE-2009-4411")
                .hasDescription("<div><b>Package: </b>acl</div> <div><b>Version: </b>2.3.2</div>"
                                + " <div><b>Link: </b><a href=\"https://nvd.nist.gov/vuln/detail/CVE-2009-4411\""
                                + ">https://nvd.nist.gov/vuln/detail/CVE-2009-4411</a></div>"
                                + " <div><b>Yocto Layer: </b>meta</div> <div><b>Vector: </b>LOCAL</div> <p>"
                                + "The (1) setfacl and (2) getfacl commands in XFS acl 2.2.47, when running"
                                + " in recursive (-R) mode, follow symbolic links even when the --physical"
                                + " (aka -P) or -L option is specified, which might allow local users to modify"
                                + " the ACL for arbitrary files or directories via a symlink attack.</p>");

Would that work?

I think I can remove the :0 for the file name attribute in the warnings plugin to make it more readable.

Ok done and change as you suggest. I'm testing it and push a new version

Yocto project support CVE security vulnerabilities using cve-check in the specific
image or target you are building, add the following setting to your configuration:

INHERIT += "cve-check"

status of each CVE: Patched, Unpatched or Ignored

The scanner look only for Unpatched package and calculate the severity using
the score_v2 or score_v3.

The generated from Yocto can be collected in build/tmp/log/cve/cve-summury.json

Signed-off-by: Michael Trimarchi <[email protected]>
@panicking panicking force-pushed the feature/add-yocto-cve-parser branch from 560ae43 to ea514ba Compare September 12, 2024 06:10
@uhafner
Copy link
Member

uhafner commented Sep 13, 2024

Thanks!

@uhafner uhafner merged commit d24330c into jenkinsci:main Sep 13, 2024
28 of 30 checks passed

@Override
public String getIconUrl() {
return "https://www.yoctoproject.org/wp-content/uploads/sites/32/2023/09/YoctoProject_Logo_RGB_White_small.svg";
Copy link
Member

Choose a reason for hiding this comment

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

I changed that to https://upload.wikimedia.org/wikipedia/commons/0/00/Yocto_Project_logo.svg, otherwise the text is white on white background. It seems that the project needs to define a SVG logo that correctly assigns the colors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@uhafner very good, and sorry, you have done a fantastic job here in this plugin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants