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 Windows support without image scanning #1469

Closed
wants to merge 2 commits into from

Conversation

olljanat
Copy link

#278 removed Windows binaries to solve #73 but after that NuGet #686 and config scanning #931 supports have been added which are very useful for Windows users too.

This PR adds Windows binaries back on way that only trivy image command is disabled and in additional to that it does add Windows container image with Trivy. Because on Windows OS layer on Docker images must match to OS version all versions which are currently supported by Microsoft was included as I hoping to get Trivy included to official .NET Core samples on dotnet/dotnet-docker#3335

Those who want to test can find binaries from https://github.com/olljanat/trivy/releases/tag/v0031 and Docker image from ollijanatuinen/trivy:0031

Here is also full manifest exported with command docker manifest inspect ollijanatuinen/trivy:0031

{
   "schemaVersion": 2,
   "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
   "manifests": [
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 1159,
         "digest": "sha256:1cde68ec1acc6595a5b8887bcd7162320c0dc53383ce761432590502dfdff11c",
         "platform": {
            "architecture": "amd64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 1159,
         "digest": "sha256:f7a449f22d966b9365502330e4d5838ff64e91393f8633a5a39483a63dfdd2a4",
         "platform": {
            "architecture": "arm64",
            "os": "linux"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 3187,
         "digest": "sha256:5d1944509b78f82634b17a3fe55534e17a8ce40678d575dc98be5e18da4e6610",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.19041.1348"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 3187,
         "digest": "sha256:c4fe960f6248e576ad0312018bbe2e25683a8e88132613ef46b0eab997cf5349",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.19042.1348"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 3187,
         "digest": "sha256:4aeb26f8a3241183098c17a764ece803433f7ee15450d6a9daa53eb4b1b49b36",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.14393.4770"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 3186,
         "digest": "sha256:f169b99be74ce343d3f7459f4c4d22feeaadf0bbfdd0ccc43add90b54461f21f",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.17763.2300"
         }
      },
      {
         "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
         "size": 3186,
         "digest": "sha256:9e81fe84481bd13d97d467458bc2331d410ca5956092e561db3a15b5d85e6c15",
         "platform": {
            "architecture": "amd64",
            "os": "windows",
            "os.version": "10.0.20348.350"
         }
      }
   ]
}

Contrib files are not currently included because there looks to be some issue on goreleaser logic on https://github.com/goreleaser/goreleaser/blob/8c6742964c96ec3f78910aceb5e1b7319f129718/internal/pipe/docker/docker.go#L150-L162 when running on Windows so I prefer to leave those for next PR.

Partly solves #738 and #1392 as with this it is possible to include Trivy to Dockerfile like you can see from dotnet/dotnet-docker#3335

Signed-off-by: Olli Janatuinen <olli.janatuinen@gmail.com>
@CLAassistant
Copy link

CLAassistant commented Dec 13, 2021

CLA assistant check
All committers have signed the CLA.

@knqyf263
Copy link
Collaborator

Hi @olljanat, thanks for your contribution. I assume that even trivy fs doesn't work on Windows. Did you try it?

@olljanat
Copy link
Author

@knqyf263 yes I tested that at least scanning those packages.lock.json files which dotnet restore creates and Kubernetes deployment files works fine. Also those examples which I did on dotnet/dotnet-docker#3335 works with both Linux and Windows using same Dockerfile (can inside of container).

And I think that even image scanning is doable. It just need some extra logic for host OS and Docker engine OS detection because it possible to run both Linux and Windows containers on top of Windows. On #73 we can see that user was scanning "alpine:latest" which is very different thing than scanning actual Windows images.

@github-actions
Copy link

This PR is stale because it has been labeled with inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Feb 14, 2022
@owenrumney
Copy link
Contributor

@knqyf263 - I've checked out this branch and run it through on an Windows box

PowerShell

PS C:\Users\owen\code\github\aquasecurity\trivy> .\trivy.exe fs --security-checks config,vuln ..\trivy-ci-test\
2022-02-19T12:05:53.208Z        INFO    Number of language-specific files: 2
2022-02-19T12:05:53.208Z        INFO    Detecting pipenv vulnerabilities...
2022-02-19T12:05:53.216Z        INFO    Detecting cargo vulnerabilities...
2022-02-19T12:05:53.217Z        INFO    Detected config files: 1

Cargo.lock (cargo)
==================
Total: 9 (UNKNOWN: 2, LOW: 0, MEDIUM: 1, HIGH: 2, CRITICAL: 4)

+-----------+-------------------+----------+-------------------+---------------+--------------------------------------------+
|  LIBRARY  | VULNERABILITY ID  | SEVERITY | INSTALLED VERSION | FIXED VERSION |                   TITLE                    |
+-----------+-------------------+----------+-------------------+---------------+--------------------------------------------+
| ammonia   | CVE-2019-15542    | HIGH     | 1.9.0             | 2.1.0         | Uncontrolled recursion leads               |
|           |                   |          |                   |               | to abort in HTML serialization             |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2019-15542      |
+           +-------------------+----------+                   +---------------+--------------------------------------------+
|           | CVE-2021-38193    | MEDIUM   |                   | 2.1.3, 3.1.0  | Incorrect handling of embedded SVG         |
|           |                   |          |                   |               | and MathML leads to mutation XSS           |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2021-38193      |
+-----------+-------------------+----------+-------------------+---------------+--------------------------------------------+
| openssl   | CVE-2016-10931    | HIGH     | 0.8.3             | 0.9.0         | SSL/TLS MitM vulnerability                 |
|           |                   |          |                   |               | due to insecure defaults                   |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2016-10931      |
+-----------+-------------------+----------+-------------------+---------------+--------------------------------------------+
| rand_core | CVE-2020-25576    | CRITICAL | 0.4.0             | 0.3.1, 0.4.2  | Unaligned memory access                    |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2020-25576      |
+-----------+-------------------+          +-------------------+---------------+--------------------------------------------+
| smallvec  | CVE-2019-15551    |          | 0.6.9             | 0.6.10        | Double-free and use-after-free             |
|           |                   |          |                   |               | in SmallVec::grow()                        |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2019-15551      |
+           +-------------------+          +                   +               +--------------------------------------------+
|           | CVE-2019-15554    |          |                   |               | Memory corruption in SmallVec::grow()      |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2019-15554      |
.... cntd

Windows Command Line

C:\Users\owen\code\github\aquasecurity\trivy>trivy.exe fs --security-checks config,vuln ..\trivy-ci-test
2022-02-19T12:08:11.971Z        INFO    Number of language-specific files: 2
2022-02-19T12:08:11.971Z        INFO    Detecting cargo vulnerabilities...
2022-02-19T12:08:11.973Z        INFO    Detecting pipenv vulnerabilities...
2022-02-19T12:08:11.982Z        INFO    Detected config files: 1

Cargo.lock (cargo)
==================
Total: 9 (UNKNOWN: 2, LOW: 0, MEDIUM: 1, HIGH: 2, CRITICAL: 4)

+-----------+-------------------+----------+-------------------+---------------+--------------------------------------------+
|  LIBRARY  | VULNERABILITY ID  | SEVERITY | INSTALLED VERSION | FIXED VERSION |                   TITLE                    |
+-----------+-------------------+----------+-------------------+---------------+--------------------------------------------+
| ammonia   | CVE-2019-15542    | HIGH     | 1.9.0             | 2.1.0         | Uncontrolled recursion leads               |
|           |                   |          |                   |               | to abort in HTML serialization             |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2019-15542      |
+           +-------------------+----------+                   +---------------+--------------------------------------------+
|           | CVE-2021-38193    | MEDIUM   |                   | 2.1.3, 3.1.0  | Incorrect handling of embedded SVG         |
|           |                   |          |                   |               | and MathML leads to mutation XSS           |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2021-38193      |
+-----------+-------------------+----------+-------------------+---------------+--------------------------------------------+
| openssl   | CVE-2016-10931    | HIGH     | 0.8.3             | 0.9.0         | SSL/TLS MitM vulnerability                 |
|           |                   |          |                   |               | due to insecure defaults                   |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2016-10931      |
+-----------+-------------------+----------+-------------------+---------------+--------------------------------------------+
| rand_core | CVE-2020-25576    | CRITICAL | 0.4.0             | 0.3.1, 0.4.2  | Unaligned memory access                    |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2020-25576      |
+-----------+-------------------+          +-------------------+---------------+--------------------------------------------+
| smallvec  | CVE-2019-15551    |          | 0.6.9             | 0.6.10        | Double-free and use-after-free             |
|           |                   |          |                   |               | in SmallVec::grow()                        |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2019-15551      |
+           +-------------------+          +                   +               +--------------------------------------------+
|           | CVE-2019-15554    |          |                   |               | Memory corruption in SmallVec::grow()      |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2019-15554      |
+           +-------------------+          +                   +---------------+--------------------------------------------+
|           | CVE-2021-25900    |          |                   | 0.6.14, 1.6.1 | Buffer overflow in                         |
|           |                   |          |                   |               | SmallVec::insert_many                      |
|           |                   |          |                   |               | -->avd.aquasec.com/nvd/cve-2021-25900      |
...cntd

I've confirmed that trivy.exe conf works too

PS C:\Users\owen\code\github\aquasecurity\trivy> .\trivy conf ..\trivy-ci-test\
2022-02-19T12:09:02.033Z        INFO    Detected config files: 1

Dockerfile (dockerfile)
=======================
Tests: 23 (SUCCESSES: 22, FAILURES: 1, EXCEPTIONS: 0)
Failures: 1 (UNKNOWN: 0, LOW: 0, MEDIUM: 0, HIGH: 1, CRITICAL: 0)

+---------------------------+------------+-----------+----------+------------------------------------------+
|           TYPE            | MISCONF ID |   CHECK   | SEVERITY |                 MESSAGE                  |
+---------------------------+------------+-----------+----------+------------------------------------------+
| Dockerfile Security Check |   DS002    | root user |   HIGH   | Specify at least 1 USER                  |
|                           |            |           |          | command in Dockerfile with               |
|                           |            |           |          | non-root user as argument                |
|                           |            |           |          | -->avd.aquasec.com/appshield/ds002       |
+---------------------------+------------+-----------+----------+------------------------------------------+

I've also used the binary that is created by the additional goreleaser.windows.yml and that looks sound too

C:\Users\owen\code\github\aquasecurity\trivy>dist\trivy_windows_amd64\trivy.exe config ..\tfsec-example-project
2022-02-19T12:30:19.152Z        INFO    Detected config files: 6

modules\acme_bucket\main.tf (terraform)
=======================================
Tests: 14 (SUCCESSES: 9, FAILURES: 5, EXCEPTIONS: 0)
Failures: 5 (UNKNOWN: 0, LOW: 0, MEDIUM: 4, HIGH: 1, CRITICAL: 0)

+------------------------------------------+--------------+------------------------------------------+----------+-----------------------------------------------------------+
|                   TYPE                   |  MISCONF ID  |                  CHECK                   | SEVERITY |                          MESSAGE                          |
+------------------------------------------+--------------+------------------------------------------+----------+-----------------------------------------------------------+
|   Terraform Security Check powered by    | AVD-AWS-0088 | Unencrypted S3 bucket.                   |   HIGH   | Resource                                                  |
|                  tfsec                   |              |                                          |          | 'module.acme_finance_bucket:aws_s3_bucket.logging_bucket' |
|                                          |              |                                          |          | defines an unencrypted S3 bucket (missing                 |
|                                          |              |                                          |          | server_side_encryption_configuration block).              |
+                                          +--------------+------------------------------------------+----------+-----------------------------------------------------------+
|                                          | AVD-AWS-0090 | S3 Data should be versioned              |  MEDIUM  | Resource                            
                      |
|                                          |              |                                          |          | 'module.acme_finance_bucket:aws_s3_bucket.acme_bucket'    |
|                                          |              |                                          |          | does not have versioning enabled                          |
+                                          +              +                                          +          +-----------------------------------------------------------+
|                                          |              |                                          |          | Resource                            
                      |
|                                          |              |                                          |          | 'module.acme_finance_bucket:aws_s3_bucket.logging_bucket' |
|                                          |              |                                          |          | does not have versioning enabled                          |
+                                          +--------------+------------------------------------------+          +-----------------------------------------------------------+
|                                          | AVD-AWS-0094 | S3 buckets should each define an         |          | Resource                            
                      |
|                                          |              | aws_s3_bucket_public_access_block        |          | module.acme_finance_bucket:aws_s3_bucket.acme_bucket      |
|                                          |              |                                          |          | has no associated aws_s3_bucket_public_access_block.      |
+                                          +              +                                          +          +-----------------------------------------------------------+
|                                          |              |                                          |          | Resource                            
                      |
|                                          |              |                                          |          | module.acme_finance_bucket:aws_s3_bucket.logging_bucket   |
|                                          |              |                                          |          | has no associated aws_s3_bucket_public_access_block.      |
+------------------------------------------+--------------+------------------------------------------+----------+-----------------------------------------------------------+

modules\acme_bucket\variables.tf (terraform)
============================================
Tests: 6 (SUCCESSES: 6, FAILURES: 0, EXCEPTIONS: 0)
Failures: 0 (UNKNOWN: 0, LOW: 0, MEDIUM: 0, HIGH: 0, CRITICAL: 0)

Lastly, when the user tries to scan an image - it gracefully fails

C:\Users\owen\code\github\aquasecurity\trivy>dist\trivy_windows_amd64\trivy.exe image alpine:latest            
2022-02-19T12:32:26.183Z        FATAL   image scanning is not supported on windows

@owenrumney
Copy link
Contributor

I can't speak to the changes for the goreleaser manifest, it looks like its a good housekeeping change though

@github-actions
Copy link

This PR is stale because it has been labeled with inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Jul 21, 2022
@olljanat
Copy link
Author

This needs rebase but I hoping to get at least some kind of feedback first.

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Jul 22, 2022
@trungklam
Copy link

trungklam commented Jul 26, 2022

This needs rebase but I hoping to get at least some kind of feedback first.

Hi @olljanat , can you please continue with the rebasing ? I think it can make this PR to be likely have more chance to be reviewed again since people can try and give feedback with current version.

@olljanat olljanat mentioned this pull request Jul 26, 2022
9 tasks
@olljanat
Copy link
Author

Ended up actually open #2592 now and will close this one.

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.

6 participants