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 cosign init to initialize the SigStore root metadata #520

Merged
merged 7 commits into from
Aug 9, 2021

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Aug 4, 2021

  • A command cosign init that initializes the local trusted SigStore root and pulls the trusted targets into .sigstore/root
  • The initial root keys are added to .sigstore/keys.json. Clients need this to verify the root they pull
  • The targets are used for the Fulcio cert and rekor public key, falling back for now to the previous embedded string in case init hasn't been run

Details:

  • I implemented a "TUF remote store" located on a GCS bucket (for now it's a testing bucket I set up with the root metadata)
  • We only download new metadata/targets from the remote location:
    • initially
    • if, when we retrieve a target, the local metadata no longer verifies
    • the local target does not match hash/size of the signed targets

Fixes
#389

In practice, you can see if you run cosign without init'ing, you get log warnings asking "did you run cosign init?" otherwise, it's silent, we're using the local metadata.

asraa@asraa-glaptop:~/git/cosign$ COSIGN_EXPERIMENTAL=1 ./cosign verify gcr.io/asra-ali/base-builder-new
using embedded fulcio certificate. did you run `cosign init`? error retrieving target:  missing target metadata: tuf: no root keys found in local meta store
using embedded rekor public key. did you run `cosign init`? error retrieving target:  missing target metadata: tuf: no root keys found in local meta store

Verification for gcr.io/asra-ali/base-builder-new --
The following checks were performed on each of these signatures:
  - The cosign claims were validated
  - Existence of the claims in the transparency log was verified offline
  - Any certificates were verified against the Fulcio roots.
Certificate subject:  [[email protected]]
{"critical":{"identity":{"docker-reference":"gcr.io/asra-ali/base-builder-new"},"image":{"docker-manifest-digest":"sha256:291750a9758506b9b3cace9c43e29b1a0f81172f43e9df1d293cb486a5d90abc"},"type":"cosign container image signature"},"optional":null}
asraa@asraa-glaptop:~/git/cosign$ ./cosign init
asraa@asraa-glaptop:~/git/cosign$ COSIGN_EXPERIMENTAL=1 ./cosign verify gcr.io/asra-ali/base-builder-new

Verification for gcr.io/asra-ali/base-builder-new --
The following checks were performed on each of these signatures:
  - The cosign claims were validated
  - Existence of the claims in the transparency log was verified offline
  - Any certificates were verified against the Fulcio roots.
Certificate subject:  [[email protected]]
{"critical":{"identity":{"docker-reference":"gcr.io/asra-ali/base-builder-new"},"image":{"docker-manifest-digest":"sha256:291750a9758506b9b3cace9c43e29b1a0f81172f43e9df1d293cb486a5d90abc"},"type":"cosign container image signature"},"optional":null}

WIP because of tests and some defaults/globals I want to get rid of

@trishankatdatadog
Copy link

Hihi, would you need a review? Happy to help! 🙂

@asraa
Copy link
Contributor Author

asraa commented Aug 4, 2021

I have to do a bit of refactoring, but would love to get a review on the TUF code! I'll ping you

@@ -41,6 +44,19 @@ import (
// rekor.pub should be updated whenever the Rekor public key is rotated & the bundle annotation should be up-versioned
//go:embed rekor.pub
var rekorPub string
var rekorTargetStr = `rekor.pub`

func GetRekorPub() string {
Copy link
Member

Choose a reason for hiding this comment

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

IMHO we should put this somewhere under cli/ in order to keep logging and cosign binary-specific out of package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhh hmmmm you mean keeping fmt.Println out? It's used elsewhere in this file as well

Copy link
Member

Choose a reason for hiding this comment

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

hrm yeah

err := tuf.GetTarget(ctx, rekorTargetStr, &buf)
if err != nil {
// The user may not have initialized the local root metadata. Log the error and use the embedded root.
fmt.Println("using embedded rekor public key. did you run `cosign init`? error retrieving target: ", err)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fmt.Println("using embedded rekor public key. did you run `cosign init`? error retrieving target: ", err)
fmt.Fprintln(os.Stderr, "using embedded rekor public key. did you run `cosign init`? error retrieving target: ", err)

pkg/cosign/tuf/client.go Outdated Show resolved Hide resolved
@cpanato cpanato added this to the v1.1.0 milestone Aug 5, 2021
Copy link
Member

@dekkagaijin dekkagaijin left a comment

Choose a reason for hiding this comment

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

I'm fine with this for now, can always move packages around as we integrate more tuf stuff

@dekkagaijin
Copy link
Member

@asraa squashing your local go.sum from upstream/main and running go mod tidy again should fix the conflict

@asraa
Copy link
Contributor Author

asraa commented Aug 6, 2021

All set, a local test added, and the path to the remote repository is in production.
@trishankatdatadog want to take a quick look at the TUF code? should be pretty simple.
Only question I had was: should we be making a tuf.db file? it works fine without one, except for in the test.

@dlorenc dlorenc merged commit bfd42e5 into sigstore:main Aug 9, 2021
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.

5 participants