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

835 - Keyless Support for SBOM Attestations #910

Merged
merged 44 commits into from
May 6, 2022

Conversation

spiffcs
Copy link
Contributor

@spiffcs spiffcs commented Mar 23, 2022

Summary:

Update the syft attest command to use the cosign keyless workflow.

Users should be able to run syft attest -o syft-json alpine:latest and generate an In-Toto Attestation serialized as a DSSE envelope defined here, which is signed by github, google, or microsoft OIDC providers. They should only be able to UPLOAD attestation for images they own.

Screen Shot 2022-05-03 at 12 40 02 PM

Users in this case will not have to bring their own key. Instead, ephemeral keys and certificates will be generated and signed automatically by the fulcio root CA. These signatures are then stored in the rekor transparency log which provides an attestation as to when the signature was created. The new feature supports Oauth flows through the browser. Device flows have not been tested and will be updated in follow up PR.

To test the new functionality on this branch you can run:
go run cmd/syft/main.go attest -o syft-json <IMAGE YOU HAVE WRITE ACCESS TO>
COSIGN_EXPERIMENTAL=1 cosign verify-attestation <IMAGE YOU HAVE WRITE ACCESS TO>

Documentation

  • Update README
  • Update ETUI to show progress for attestation flows

Signed-off-by: Christopher Phillips christopher.phillips@anchore.com

spiffcs added 15 commits March 22, 2022 12:26
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs force-pushed the 835-keyless-attestation-upgrade branch from 6050fad to 659e1da Compare March 23, 2022 18:48
@github-actions
Copy link

github-actions bot commented Mar 23, 2022

Benchmark Test Results

Benchmark results from the latest changes vs base branch
name                                                       old time/op    new time/op    delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2              1.14ms ± 0%    1.33ms ± 5%  +16.51%  (p=0.016 n=4+5)
ImagePackageCatalogers/python-package-cataloger-2            3.01ms ± 0%    3.13ms ± 8%   +4.18%  (p=0.016 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2     940µs ± 1%    1063µs ± 6%  +13.04%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         631µs ± 1%     706µs ± 7%  +11.95%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                     737µs ± 0%     912µs ± 5%  +23.68%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                      641µs ± 1%     780µs ± 7%  +21.74%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                      13.5ms ± 0%    15.3ms ±11%  +13.68%  (p=0.016 n=4+5)
ImagePackageCatalogers/apkdb-cataloger-2                     1.17ms ± 1%    1.29ms ± 4%  +10.21%  (p=0.008 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2          2.08µs ± 1%    2.31µs ± 6%  +10.96%  (p=0.008 n=5+5)
ImagePackageCatalogers/dotnet-deps-cataloger-2               1.26ms ± 1%    1.36ms ± 6%   +8.00%  (p=0.008 n=5+5)

name                                                       old alloc/op   new alloc/op   delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2               185kB ± 0%     185kB ± 0%     ~     (p=0.548 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2             900kB ± 0%     898kB ± 0%   -0.19%  (p=0.008 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2     197kB ± 0%     197kB ± 0%   -0.18%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         141kB ± 0%     141kB ± 0%     ~     (p=0.421 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                     177kB ± 0%     176kB ± 0%     ~     (p=0.056 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                      165kB ± 0%     164kB ± 0%   -0.07%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                      3.30MB ± 0%    3.30MB ± 0%     ~     (p=0.222 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                     1.24MB ± 0%    1.24MB ± 0%     ~     (p=0.056 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2            672B ± 0%      672B ± 0%     ~     (all equal)
ImagePackageCatalogers/dotnet-deps-cataloger-2                351kB ± 0%     352kB ± 0%   +0.13%  (p=0.008 n=5+5)

name                                                       old allocs/op  new allocs/op  delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2               3.70k ± 0%     3.70k ± 0%     ~     (all equal)
ImagePackageCatalogers/python-package-cataloger-2             14.9k ± 0%     14.9k ± 0%     ~     (p=0.286 n=4+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2     4.99k ± 0%     4.98k ± 0%     ~     (p=0.206 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         2.76k ± 0%     2.76k ± 0%     ~     (all equal)
ImagePackageCatalogers/dpkgdb-cataloger-2                     3.99k ± 0%     3.99k ± 0%     ~     (all equal)
ImagePackageCatalogers/rpmdb-cataloger-2                      4.06k ± 0%     4.06k ± 0%     ~     (all equal)
ImagePackageCatalogers/java-cataloger-2                       52.6k ± 0%     52.6k ± 0%     ~     (p=0.548 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                      4.87k ± 0%     4.87k ± 0%     ~     (p=1.000 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2            15.0 ± 0%      15.0 ± 0%     ~     (all equal)
ImagePackageCatalogers/dotnet-deps-cataloger-2                6.72k ± 0%     6.72k ± 0%     ~     (all equal)

@spiffcs spiffcs marked this pull request as draft March 24, 2022 15:30
spiffcs added 2 commits May 2, 2022 11:27
* main: (31 commits)
  reduce noise of log output (#976)
  add version info and remove double config call (#977)
  Rename syft-id to package-id (#970)
  update to cyclonedx-go 0.5.2 (#971)
  refactor command package to remove globals and add dependency injection
  fix: #953 Derive language from pURL - https://github.com/anchore/syft… (#957)
  Fix typo in CPE-parsing error (#966)
  Preserve syft IDs on SBOM decode (#963)
  Update GitHub format package_url and correlator (#961)
  Ensure SPDXIDs are valid (#955)
  Auto-PR needs to run go mod tidy (#958)
  Add workflow for automatic PR for new stereoscope updates (#954)
  Minor readme update to correct format information (#948)
  Update spdx22json to only take uppercase checksum algorithm (#946)
  add additional vendors for springframework (#945)
  Add digest property to parent and nested java package metadata (#941)
  Update write permissions and log into ghcr.io for release (#942)
  Retry auth URL lookup without docker credentialhelper workaround (#939)
  Ensure that all cyclonedx components have bom-refs (#914)
  Additionally publish docker images to GHCR (#934)
  ...

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs force-pushed the 835-keyless-attestation-upgrade branch from ecdbf62 to 4d302eb Compare May 2, 2022 19:23
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs force-pushed the 835-keyless-attestation-upgrade branch from 4d302eb to ebc96d3 Compare May 2, 2022 19:28
spiffcs added 3 commits May 2, 2022 15:40
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs marked this pull request as ready for review May 3, 2022 16:39
@spiffcs spiffcs marked this pull request as draft May 3, 2022 16:44
spiffcs added 2 commits May 3, 2022 15:17
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
return nil, err
}

log.Infof("tlog entry created with index: %v", *entry.LogIndex)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should this be debug? or if it's worthy information, should this information be elevated to the ETUI with upload progress (even if infinite spinner)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @luhring on this one. I'm on the fence about including this in the default output. I'll demote it to debug for now, but if we come up with a good reason in this thread to why it should stick around I can elevate it into the ETUI

Copy link
Contributor

Choose a reason for hiding this comment

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

Does cosign show this? I don't remember for sure but I thought it does — since it's a convenient way to find the Rekor entry that was just created (i.e. after you run this, you can use a Rekor client to just to a lookup by index)

if err != nil {
return err
}

bus.Publish(partybus.Event{
Type: event.Exit,
Value: func() error {
_, err := os.Stdout.Write(signedPayload)
Copy link
Contributor

Choose a reason for hiding this comment

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

if we're uploading should we be outputting the signed payload to the screen? or suppress this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should still be publishing the payload to the screen on upload so the user is able to inspect what was uploaded, but I can also see how suppressing is gives a cleaner execution and we don't pollute the screen since payloads are large and noticeably indecipherable on first glance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @luhring for input

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah no strong opinion, but I think I'd lean toward not outputting it for now for the "clean feel" reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed output on upload

"github.com/spf13/viper"
)

const DefaultFulcioURL = "https://fulcio.sigstore.dev"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this can be unexported

"identity token to use for certificate from fulcio")

cmd.Flags().BoolVar(&o.InsecureSkipFulcioVerify, "insecure-skip-verify", false,
"skip verifying fulcio published to the SCT (this should only be used for testing).")
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the SCT? The docs should probably reflect the full name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I used SCT incorrectly here so double good call out.

SCT is “Signed Certificate Timestamp”. We need this flag so that we don't have to setup a root CA for fulcio during testing. Fulcio returns a “Signed Certificate Timestamp”.

See here if you want the deep dive:
https://blog.chainguard.dev/a-fulcio-deep-dive/

}

sv, err := sign.SignerFromKeyOpts(ctx, "", "", ko)
sv, err := sign.SignerFromKeyOpts(ctx, "", "", *ko)
Copy link
Contributor

Choose a reason for hiding this comment

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

should nil check ko first

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's small so I updated it to pass by value instead of pointer here.

spiffcs and others added 17 commits May 6, 2022 11:29
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
* main:
  feat: add initial dotnet-support (#951)
  unblock timeout for power-user select CLI tests (#985)
  golang cataloger - main module version as is (#986)
  Fix `github-json` output option (#967)
  read Go main module version as is - (devel) (#981)
  reduce logging severity for non-Go binaries (#983)
  golang.org/x/crypto upgrade (#979)
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs merged commit d2d532f into main May 6, 2022
@spiffcs spiffcs deleted the 835-keyless-attestation-upgrade branch May 6, 2022 22:06
@developer-guy
Copy link
Contributor

This PR will also fix #990, by default, it'll attach attestation to an image requested to generate an attestation.
cc: @Dentrax @luhring

GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
Co-authored-by: Alex Goodman <alex.goodman@anchore.com>
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.

4 participants