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.

Sign up for GitHub

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

Jump to bottom

feat(report): add support for Cosign vulnerability attestation #2567

Merged
merged 21 commits into from
Jul 27, 2022
Merged

feat(report): add support for Cosign vulnerability attestation #2567

merged 21 commits into from
Jul 27, 2022

Conversation

otms61
Copy link
Collaborator

@otms61 otms61 commented Jul 21, 2022 •
edited
Loading

Description

Support cosign-vuln format option for Cosing vulnerability predicate.

$  ./trivy image --format cosign-vuln alpine:3.10
Result 
2022-07-21T21:27:29.390+0900    INFO    Vulnerability scanning is enabled
2022-07-21T21:27:29.390+0900    INFO    Secret scanning is enabled
2022-07-21T21:27:29.390+0900    INFO    If your scanning is slow, please try '--security-checks vuln' to disable secret scanning
2022-07-21T21:27:29.390+0900    INFO    Please see also https://aquasecurity.github.io/trivy/v0.30.1-1-gf9abc93c/docs/secret/scanning/#recommendation for fast
er secret detection
2022-07-21T21:27:31.620+0900    INFO    Detected OS: alpine
2022-07-21T21:27:31.620+0900    INFO    Detecting Alpine vulnerabilities...
2022-07-21T21:27:31.621+0900    INFO    Number of language-specific files: 0
2022-07-21T21:27:31.621+0900    WARN    This OS version is no longer supported by the distribution: alpine 3.10.9
2022-07-21T21:27:31.621+0900    WARN    The vulnerability detection may be insufficient because security updates are not provided
{
  "invocation": {
    "parameters": null,
    "uri": "",
    "event_id": "",
    "builder.id": ""
  },
  "scanner": {
    "uri": "pkg:github/aquasecurity/trivy@v0.30.1-1-gf9abc93c",
    "version": "v0.30.1-1-gf9abc93c",
    "db": {
      "uri": "",
      "version": ""
    },
    "result": {
      "ArtifactName": "alpine:3.10",
      "ArtifactType": "container_image",
      "Metadata": {
        "DiffIDs": [
          "sha256:9fb3aa2f8b8023a4bebbf92aa567caf88e38e969ada9f0ac12643b2847391635"
        ],
        "ImageConfig": {
          "architecture": "amd64",
          "config": {
            "Cmd": [
              "/bin/sh"
            ],
            "Env": [
              "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
            ],
            "Image": "sha256:eb2080c455e94c22ae35b3aef9e078c492a00795412e026e4d6b41ef64bc7dd8"
          },
          "container": "fdb7e80e3339e8d0599282e606c907aa5881ee4c668a68136119e6dfac6ce3a4",
          "created": "2021-04-14T19:20:05.338397761Z",
          "docker_version": "19.03.12",
          "history": [
            {
              "created": "2021-04-14T19:20:04.987219124Z",
              "created_by": "/bin/sh -c #(nop) ADD file:c5377eaa926bf412dd8d4a08b0a1f2399cfd708743533b0aa03b53d14cb4bb4e in / "
            },
            {
              "created": "2021-04-14T19:20:05.338397761Z",
              "created_by": "/bin/sh -c #(nop)  CMD [\"/bin/sh\"]",
              "empty_layer": true
            }
          ],
          "os": "linux",
          "rootfs": {
            "diff_ids": [
              "sha256:9fb3aa2f8b8023a4bebbf92aa567caf88e38e969ada9f0ac12643b2847391635"
            ],
            "type": "layers"
          }
        },
        "ImageID": "sha256:e7b300aee9f9bf3433d32bc9305bfdd22183beb59d933b48d77ab56ba53a197a",
        "OS": {
          "EOSL": true,
          "Family": "alpine",
          "Name": "3.10.9"
        },
        "RepoDigests": [
          "alpine@sha256:451eee8bedcb2f029756dc3e9d73bab0e7943c1ac55cff3a4861c52a0fdd3e98"
        ],
        "RepoTags": [
          "alpine:3.10"
        ]
      },
      "Results": [
        {
          "Class": "os-pkgs",
          "Target": "alpine:3.10 (alpine 3.10.9)",
          "Type": "alpine",
          "Vulnerabilities": [
            {
              "CVSS": {
                "nvd": {
                  "V2Score": 6.4,
                  "V2Vector": "AV:N/AC:L/Au:N/C:P/I:N/A:P",
                  "V3Score": 9.1,
                  "V3Vector": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:H"
                }
              },
              "CweIDs": [
                "CWE-125"
              ],
              "DataSource": {
                "ID": "alpine",
                "Name": "Alpine Secdb",
                "URL": "https://secdb.alpinelinux.org/"
              },
              "Description": "libfetch before 2021-07-26, as used in apk-tools, xbps, and other products, mishandles numeric strings for the FTP and HTTP protocols. The FTP passive mode implementation allows an out-of-bounds read because strtol is used to parse the relevant numbers into address bytes. It does not check if the line ends prematurely. If it does, the for-loop condition checks for the '\\0' terminator one byte too late.",
              "FixedVersion": "2.10.7-r0",
              "InstalledVersion": "2.10.6-r0",
              "LastModifiedDate": "2021-10-18T12:19:00Z",
              "Layer": {
                "DiffID": "sha256:9fb3aa2f8b8023a4bebbf92aa567caf88e38e969ada9f0ac12643b2847391635",
                "Digest": "sha256:396c31837116ac290458afcb928f68b6cc1c7bdd6963fc72f52f365a2a89c1b5"
              },
              "PkgName": "apk-tools",
              "PrimaryURL": "https://avd.aquasec.com/nvd/cve-2021-36159",
              "PublishedDate": "2021-08-03T14:15:00Z",
              "References": [
                "https://github.com/freebsd/freebsd-src/commits/main/lib/libfetch",
                "https://gitlab.alpinelinux.org/alpine/apk-tools/-/issues/10749",
                "https://lists.apache.org/thread.html/r61db8e7dcb56dc000a5387a88f7a473bacec5ee01b9ff3f55308aacc@%3Cdev.kafka.apache.org%3E",
                "https://lists.apache.org/thread.html/r61db8e7dcb56dc000a5387a88f7a473bacec5ee01b9ff3f55308aacc@%3Cusers.kafka.apache.org%3E",
                "https://lists.apache.org/thread.html/rbf4ce74b0d1fa9810dec50ba3ace0caeea677af7c27a97111c06ccb7@%3Cdev.kafka.apache.org%3E",
                "https://lists.apache.org/thread.html/rbf4ce74b0d1fa9810dec50ba3ace0caeea677af7c27a97111c06ccb7@%3Cusers.kafka.apache.org%3E"
              ],
              "Severity": "CRITICAL",
              "SeveritySource": "nvd",
              "VulnerabilityID": "CVE-2021-36159"
            }
          ]
        }
      ],
      "SchemaVersion": 2
    }
  },
  "metadata": {
    "scanStartedOn": "2022-07-21T21:28:33.349428+09:00",
    "scanFinishedOn": "2022-07-21T21:28:33.349428+09:00"
  }
}

Related issues

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

knqyf263
knqyf263 reviewed Jul 24, 2022
View reviewed changes
go.mod Outdated
@@ -28,7 +28,7 @@ require (
github.com/go-redis/redis/v8 v8.11.5
github.com/golang-jwt/jwt v3.2.2+incompatible
github.com/golang/protobuf v1.5.2
github.com/google/go-containerregistry v0.7.1-0.20211214010025-a65b7844a475
github.com/google/go-containerregistry v0.8.1-0.20220209165246-a44adc326839
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to bump the version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, we don't.
I updated go.mod unintentionally. I checked again, and go.mod needs no modifications.

go.mod Outdated
@@ -49,7 +49,7 @@ require (
github.com/sosedoff/gitkit v0.3.0
github.com/spf13/cobra v1.5.0
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.8.1
github.com/spf13/viper v1.12.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same
#2567 (comment)

@otms61 otms61 marked this pull request as ready for review July 24, 2022 12:41
knqyf263
knqyf263 reviewed Jul 25, 2022
View reviewed changes
pkg/report/predicate/vuln.go Outdated
// CosignVulnPredicate represents the Cosign Vulnerability predicate.
// Cosign provides the CosignVulnPredicate structure in their repository.
// But the type of Scanner.Result is defined as map[string]interface{}, which is difficult to use,
// so we define our own.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add this link, please.
sigstore/cosign#2096

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright. I've added the link.

pkg/report/predicate/vuln_test.go Outdated
name string
detectedVulns []types.DetectedVulnerability
want predicate.CosignVulnPredicate
wantResult types.Report
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I forgot to remove it.

pkg/report/predicate/vuln_test.go Outdated
)

func TestWriter_Write(t *testing.T) {
testCases := []struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is a nitpick, but we're recently trying to comply with the same convention.

trivy/pkg/licensing/classifier_test.go

Line 16 in d93a997

tests := []struct {

Suggested change
testCases := []struct {
tests := []struct {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've fixed it.

pkg/report/predicate/vuln_test.go Outdated
},
}

for _, tc := range testCases {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: ditto

Suggested change
for _, tc := range testCases {
for _, tt := range testCases {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've fixed it.

pkg/report/predicate/vuln_test.go Outdated
writer := predicate.NewWriter(output, "dev")

err := writer.Write(inputResults)
assert.NoError(t, err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: IMHO, require is better here as all the subsequent assertions will fail anyway when it returns an error.

Suggested change
assert.NoError(t, err)
require.NoError(t, err)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It makes sense. I've fixed it.

pkg/report/predicate/vuln_test.go Outdated

var got predicate.CosignVulnPredicate
err = json.Unmarshal(output.Bytes(), &got)
assert.NoError(t, err, "invalid json written")
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've fixed it.

docs/docs/attestation/vuln.md
If you want to avoid writing an OCI registry and only want to see an attestation, add the `--no-upload` option to the `cosign` command.


Cosign can generate key pairs and use them for signing and verification. Read more about [how to generate key pairs](https://docs.sigstore.dev/cosign/key-generation).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a keyless section?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright. I've added a keyless signing section.

docs/docs/attestation/vuln.md Outdated

## Generate Cosign Vulnerability Predicate

Trivy generates reports in the [Cosign vulnerability predicate format](https://github.com/sigstore/cosign/blob/95b74db89941e8ec85e768f639efd4d948db06cd/specs/COSIGN_VULN_ATTESTATION_SPEC.md).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Trivy generates reports in the [Cosign vulnerability predicate format](https://github.com/sigstore/cosign/blob/95b74db89941e8ec85e768f639efd4d948db06cd/specs/COSIGN_VULN_ATTESTATION_SPEC.md).
Trivy generates reports in the [Cosign vulnerability predicate format][vuln-attest-spec]

And put it at the bottom.

[vuln-attest-spec]: https://github.com/sigstore/cosign/blob/95b74db89941e8ec85e768f639efd4d948db06cd/specs/COSIGN_VULN_ATTESTATION_SPEC.md

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've fixed it.

knqyf263
knqyf263 reviewed Jul 27, 2022
View reviewed changes
mkdocs.yml Outdated
@@ -74,6 +74,7 @@ nav:
- SPDX: docs/sbom/spdx.md
- Attestation:
- SBOM: docs/attestation/sbom.md
- Cosign Vulnerability Predicate: docs/attestation/vuln.md
Copy link
Collaborator

Choose a reason for hiding this comment

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

To align with their doc.

Cosign Vulnerability Scan Record Attestation Specification

https://github.com/sigstore/cosign/blob/main/specs/COSIGN_VULN_ATTESTATION_SPEC.md

Suggested change
- Cosign Vulnerability Predicate: docs/attestation/vuln.md
- Cosign Vulnerability Scan Record: docs/attestation/vuln.md

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

pkg/report/predicate/vuln.go Outdated
ScanFinishedOn time.Time `json:"scanFinishedOn"`
}

type Writer struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may have more predicates in the future.

Suggested change
type Writer struct {
type VulnWriter struct {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

pkg/report/predicate/vuln.go Outdated
version string
}

func NewWriter(output io.Writer, version string) Writer {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func NewWriter(output io.Writer, version string) Writer {
func NewVulnWriter(output io.Writer, version string) Writer {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

knqyf263
knqyf263 reviewed Jul 27, 2022
View reviewed changes
pkg/report/predicate/vuln.go Outdated
// But the type of Scanner.Result is defined as map[string]interface{}, which is difficult to use,
// so we define our own.
// The PR is in progress to replace Scanner.Result type to interface{}.
// https://github.com/sigstore/cosign/pull/2096
Copy link
Collaborator

@knqyf263 knqyf263 Jul 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This PR got merged. But we can keep our own structs so that we won't depend on cosign only for those small structs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have added comments on the background and references.

@knqyf263 knqyf263 changed the title feat(report): add support for Cosign vulnerability predicate feat(report): add support for Cosign vulnerability attestation Jul 27, 2022
@knqyf263 knqyf263 merged commit c2a7ad5 into aquasecurity:main Jul 27, 2022
@otms61 otms61 deleted the cosign_vuln branch July 28, 2022 11:17
@chipzoller chipzoller mentioned this pull request Jul 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knqyf263 knqyf263 knqyf263 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

provide vulnerability attestation based on cosign vuln spec
2 participants
@otms61 @knqyf263