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

Samples: Scan dependencies with Trivy #3335

Closed
wants to merge 2 commits into from

Conversation

olljanat
Copy link

@olljanat olljanat commented Dec 12, 2021

Now when everyone is trying to figure out if/where their applications use log4j2 it is probably good time to propose that samples on here should contain security scan.

This PR enables RestorePackagesWithLockFile to samples which will instruct dotnet restore to generate packages.lock.json (like described on here) and add new stage where Trivy does security scan based on it as have been supported over year now aquasecurity/trivy#686

Current it is done only for Linux with amd64 and arm64* as Trivy docker image for arm32 or Windows does not exist.

To make sure that scan works correctly I also added latest version of Microsoft.AspNetCore.Diagnostics which it selves is not vulnerable but one of it's dependencies (System.Text.Encodings.Web version 4.5.0) is to aspnetapp and that same package to complexapp and dotnetapp as separate commit which why this PR is currently marked as WIP. I will remove that commit after first CI run.
CI failed as expected. Here is log file Linux_amd64 buster.log and here the relevant part of it:

2021-12-12T11:40:09.9754419Z  ---> Running in 6d8f1734c7f1
2021-12-12T11:40:09.9755159Z 2021-12-12T11:40:09.453Z	�[34mINFO�[0m	Need to update DB
2021-12-12T11:40:09.9755914Z 2021-12-12T11:40:09.453Z	�[34mINFO�[0m	Downloading DB...
2021-12-12T11:40:11.5711243Z 2021-12-12T11:40:11.569Z	�[34mINFO�[0m	Number of language-specific files: 1
2021-12-12T11:40:11.5713059Z 2021-12-12T11:40:11.569Z	�[34mINFO�[0m	Detecting nuget vulnerabilities...
2021-12-12T11:40:11.5723190Z 
2021-12-12T11:40:11.5723737Z packages.lock.json (nuget)
2021-12-12T11:40:11.5724105Z ==========================
2021-12-12T11:40:11.5724493Z Total: 1 (HIGH: 0, CRITICAL: 1)
2021-12-12T11:40:11.5724712Z 
2021-12-12T11:40:11.5725540Z +---------------------------+------------------+----------+-------------------+---------------------+---------------------------------------+
2021-12-12T11:40:11.5726385Z |          LIBRARY          | VULNERABILITY ID | SEVERITY | INSTALLED VERSION |    FIXED VERSION    |                 TITLE                 |
2021-12-12T11:40:11.5727226Z +---------------------------+------------------+----------+-------------------+---------------------+---------------------------------------+
2021-12-12T11:40:11.5735239Z | System.Text.Encodings.Web | CVE-2021-26701   | CRITICAL | 4.5.0             | 5.0.1, 4.7.2, 4.5.1 | dotnet: System.Text.Encodings.Web     |
2021-12-12T11:40:11.5736411Z |                           |                  |          |                   |                     | Remote Code Execution                 |
2021-12-12T11:40:11.5741160Z |                           |                  |          |                   |                     | -->avd.aquasec.com/nvd/cve-2021-26701 |
2021-12-12T11:40:11.5742552Z +---------------------------+------------------+----------+-------------------+---------------------+---------------------------------------+
2021-12-12T11:40:12.8080677Z -- EXECUTION ELAPSED TIME: 00:13:26.4639683
2021-12-12T11:40:12.8082013Z The command '/bin/sh -c trivy fs     --exit-code 1     --no-progress     --ignore-unfixed     --severity "HIGH,CRITICAL"     --security-checks vuln /source' returned a non-zero code: 1

Additionally I needed to leave this test out from generic Dockerfile now as it applies to Windows too. Will start working on Trivy side to trying get Windows support included.

EDIT: Converted back to draft because looks that I will get Trivy working on Windows containers too so will implement that first.

@olljanat olljanat force-pushed the include-trivy-to-samples branch from c882b3c to 7ff6d88 Compare December 12, 2021 12:21
@olljanat olljanat changed the title WIP: Samples: Scan dependencies with Trivy Samples: Scan dependencies with Trivy Dec 12, 2021
but currently only on Linux x64 and arm64
because Trivy Docker images are not available for arm32 or Windows
@olljanat olljanat force-pushed the include-trivy-to-samples branch from 7ff6d88 to afb1e58 Compare December 12, 2021 12:55
@olljanat olljanat marked this pull request as draft December 12, 2021 19:04
@olljanat olljanat force-pushed the include-trivy-to-samples branch from eec1dd4 to 208bf25 Compare December 12, 2021 22:34
@mthalman
Copy link
Member

Next time please log an issue for such changes so that discussion can occur to determine an appropriate implementation. That way we can avoid consuming unnecessary build resources.

Why are you using ollijanatuinen/trivy instead of aquasec/trivy?

Overall, I'm hesitant to be so explicitly promoting one security vendor over another. These are sample Dockerfiles which provide guidance to users so there is an implicit recommendation of security vendor here. At a minimum, there would need to be engagement with Aqua to determine their level of support for .NET.

cc @richlander

@olljanat
Copy link
Author

olljanat commented Dec 13, 2021

Why are you using ollijanatuinen/trivy instead of aquasec/trivy?

Because Trivy does not support Windows currently. Implemented it on aquasecurity/trivy#1469 which why this PR is also marked as draft currently.

Overall, I'm hesitant to be so explicitly promoting one security vendor over another. These are sample Dockerfiles which provide guidance to users so there is an implicit recommendation of security vendor here.

True but people are lazy and most will end up just those sample as is which why IMO there should be at least some example with open source security scanner. Trivy is quite widely used on open source projects. However one way to do it would be just add example new file like Dockerfile.Trivy so there would be room for others too.

At a minimum, there would need to be engagement with Aqua to determine their level of support for .NET.

Two most critical findings which I did as part of this was that RestorePackagesWithLockFile should be enable by default on these examples. Other why Trivy will not found any vulnerabilities even if there is any.

Other thing is that Trivy can work on Windows but support is currently missing which why opened that PR on their side.

@olljanat
Copy link
Author

Next time please log an issue for such changes so that discussion can occur to determine an appropriate implementation.

Truth is that when you got inspiration on your spare day there is no time open issue and wait responses.

That way we can avoid consuming unnecessary build resources.

One nice way to avoid that would be migrate example one Linux and one Windows build pipeline to GitHub workflows which can run also on forks and then enable policy to here which needs manual triggering for PRs created by new contributors. That is how it is done on Trivy repo where I was able to test whole solution and get those ollijanatuinen/trivy test images created before creating PR. Another nice thing on that solution is that then I can also disable not needed workflows on my fork.

Anyway, after all I trying to do a big favour for all .NET users here by using my spare time to creating open source, fully tested vulnerabilities scanning example configuration instead of just implementing it for company where I'm working on. If that does not get approved to this repo then I can publish that under my own but as this repo have a lot for bigger audience I decided to open it here as example (+ I like about ability to see how it works on different platforms as CI tells that).

PS. You most probably should report about that critical severity issue on Microsoft.AspNetCore.Diagnostics for someone who is able to fix is as I found it part of this work.

@mthalman
Copy link
Member

PS. You most probably should report about that critical severity issue on Microsoft.AspNetCore.Diagnostics for someone who is able to fix is as I found it part of this work.

What exactly is there to report? You configured the project to explicitly reference the vulnerable version, 4.5.0. As stated in the vulnerability report, the fixed version is 4.5.1:

2021-12-12T11:40:11.5725540Z +---------------------------+------------------+----------+-------------------+---------------------+---------------------------------------+
2021-12-12T11:40:11.5726385Z |          LIBRARY          | VULNERABILITY ID | SEVERITY | INSTALLED VERSION |    FIXED VERSION    |                 TITLE                 |
2021-12-12T11:40:11.5727226Z +---------------------------+------------------+----------+-------------------+---------------------+---------------------------------------+
2021-12-12T11:40:11.5735239Z | System.Text.Encodings.Web | CVE-2021-26701   | CRITICAL | 4.5.0             | 5.0.1, 4.7.2, 4.5.1 | dotnet: System.Text.Encodings.Web     |
2021-12-12T11:40:11.5736411Z |                           |                  |          |                   |                     | Remote Code Execution                 |
2021-12-12T11:40:11.5741160Z |                           |                  |          |                   |                     | -->avd.aquasec.com/nvd/cve-2021-26701 |
2021-12-12T11:40:11.5742552Z +---------------------------+------------------+----------+-------------------+---------------------+---------------------------------------+
2021-12-12T11:40:12.8080677Z 

Everything seems as expected to me.

@olljanat
Copy link
Author

olljanat commented Dec 13, 2021

PS. You most probably should report about that critical severity issue on Microsoft.AspNetCore.Diagnostics for someone who is able to fix is as I found it part of this work.

What exactly is there to report? You configured the project to explicitly reference the vulnerable version, 4.5.0. As stated in the vulnerability report, the fixed version is 4.5.1:

For complexapp and dotnetapp yes as those cannot use AspNetCore components. But for aspnetapp I used latest version of Microsoft.AspNetCore.Diagnostics on 9e5fa9e#diff-68729f8a38007c547cb9dc5731c75b2532dd4792dcb38195aae2987d6958f1a0R12 and it loads 4.5.0 version of System.Text.Encodings.Web

So in summary aspnetapp with package include like this fails on Trivy scan:

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.Diagnostics" Version="2.2.0" />
  </ItemGroup>

workaround is include 4.5.1 version of System.Text.Encodings.Web but user need to do that manually:

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.Diagnostics" Version="2.2.0" />
    <PackageReference Include="System.Text.Encodings.Web" Version="4.5.1" />
  </ItemGroup>

Full dependency chain is Microsoft.AspNetCore.Diagnostics 2.2.0 --> Microsoft.AspNetCore.Hosting.Abstractions 2.2.0 --> Microsoft.AspNetCore.Http.Abstractions 2.2.0 --> System.Text.Encodings.Web 4.5.0 which IMO proofs my point that it impossible to find these nowadays without automation which why samples should contains at least some open source tooling example.

@mthalman
Copy link
Member

PS. You most probably should report about that critical severity issue on Microsoft.AspNetCore.Diagnostics for someone who is able to fix is as I found it part of this work.

What exactly is there to report? You configured the project to explicitly reference the vulnerable version, 4.5.0. As stated in the vulnerability report, the fixed version is 4.5.1:

For complexapp and dotnetapp yes as those cannot use AspNetCore components. But for aspnetapp I used latest version of Microsoft.AspNetCore.Diagnostics on 9e5fa9e#diff-68729f8a38007c547cb9dc5731c75b2532dd4792dcb38195aae2987d6958f1a0R12 and it loads 4.5.0 version of System.Text.Encodings.Web

So in summary aspnetapp with package include like this fails on Trivy scan:

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.Diagnostics" Version="2.2.0" />
  </ItemGroup>

workaround is include 4.5.1 version of System.Text.Encodings.Web but user need to do that manually:

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.Diagnostics" Version="2.2.0" />
    <PackageReference Include="System.Text.Encodings.Web" Version="4.5.1" />
  </ItemGroup>

Full dependency chain is Microsoft.AspNetCore.Diagnostics 2.2.0 --> Microsoft.AspNetCore.Hosting.Abstractions 2.2.0 --> Microsoft.AspNetCore.Http.Abstractions 2.2.0 --> System.Text.Encodings.Web 4.5.0 which IMO proofs my point that it impossible to find these nowadays without automation which why samples should contains at least some open source tooling example.

@wtgodbe - Can you investigate why packages like Microsoft.AspNetCore.WebUtilities end up including version 4.5.0 of System.Text.Encodings.Web instead of version 4.5.1? The dependency syntax is >= 4.5.0 so my assumption is that 4.5.1 would be retrieved instead. The 4.5.0 version has a critical severity vulnerability and obviously not desirable.

@wtgodbe
Copy link
Member

wtgodbe commented Dec 13, 2021

Presumably it's because of the 4.5.0 dependency listed - that's the 2.2 version, which is out of support, so we can't patch it anyways. The correct workaround is either to add the explicit package dependency, or upgrade to 3.1 or newer.

@mthalman
Copy link
Member

@olljanat - I think it would be worthwhile if you could investigate the other security scanning solutions that exist. I question whether scanning as part of the Docker build is the best approach to take. Are there other solutions that support the scanning for .NET vulnerabilities in already built container images? To me, that would be ideal because it requires no special configuration in the Dockerfile and also provides OS vulnerabilities as well.

@olljanat
Copy link
Author

Presumably it's because of the 4.5.0 dependency listed - that's the 2.2 version, which is out of support, so we can't patch it anyways. The correct workaround is either to add the explicit package dependency, or upgrade to 3.1 or newer.

Ah, that is because package have been renamed. 2.2.0 was last one under name Microsoft.AspNetCore.Diagnostics and later versions are under name Microsoft.Extensions.Diagnostics.

I think it would be worthwhile if you could investigate the other security scanning solutions that exist.

I'm happy to take information about any alternative open source based security scanning tools available which are able to scan .NET dependencies? Most of them are commercial only and with very high price tag which why they are not option for everyone.

Also even those who have those can benefit from having multiple tools in software delivery chain. That is actually what Aquasec is doing when they offer Trivy as open source but make money with those extra tools which run production servers, etc but nothing prevents users to combine it with other tools.

I question whether scanning as part of the Docker build is the best approach to take. Are there other solutions that support the scanning for .NET vulnerabilities in already built container images? To me, that would be ideal because it requires no special configuration in the Dockerfile and also provides OS vulnerabilities as well.

For me that would be totally opposite than ideal as it means that those tools must exist on build servers. Also on environment where apps are build multiple times per day it is just waste of time and resources to do security scan again on every build if packages have not been changed.

IMO ideal is like it on complexapp example which have restore, (add security scan here), build and unit tests all run inside of Dockerfile so any build server which able to build container will run those everytime exactly same way and security scan will also utilize Docker cache when there is not chages on packages.

However, my Trivy side PR is prerequirement for this one so let's see which kind of feedback it gets after they have time review it.

@mthalman
Copy link
Member

@olljanat - Do you intend to continue to do further development on this PR? If not, I suggest that you close it for now.

As stated earlier, we'd prefer a scanning solution that wasn't contained in the Docker build but rather a post-build step on the entire container. This has the benefit of allowing additional scans of an already built image to be run and potentially detecting new vulnerabilities. The image itself remain static but scanning tools are always updated to detect new vulnerabilities.

We also don't want to be endorsing one scanning vendor over another.

If you have content (e.g. blog post, GitHub markdown) that you'd be willing to publish of your own, we may consider linking to that as an example that illustrates one way of integrating scanning into a workflow.

@olljanat
Copy link
Author

It looks to be that guys on Trivy project very rarely review PRs so I have not got any feedback to my PR on there yet and it unclear if that will ever go forward or not so will close this one now on here.

@olljanat olljanat closed this Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants