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 a policy-init using TUF metadata and Fulcio signers #469

Merged
merged 4 commits into from
Sep 30, 2021

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Jul 22, 2021

Signed-off-by: Asra Ali [email protected]

Hoping to iterate on this. This only initializes the root policy. We need to:

  • Add the sign command
  • Add ISSUER information to trust?

Replaces/uses code from

Current usage:
./cosign policy-init --namespace gcr.io/asra-ali/busybox --maintainers [email protected],[email protected] --threshold 2 --out root.json

./cosign policy-init --namespace gcr.io/asra-ali/busybox --maintainers [email protected],[email protected] --threshold 2 --out root.json
Uploading file from [root.json] to [gcr.io/asra-ali/busybox/root.json:latest] with media type [text/plain]
File [root.json] is available directly at [gcr.io/v2/asra-ali/busybox/root.json/blobs/sha256:973a21f41a3884b9f40e70df6bfdd19b04c3ddcbd8935b82a09fd9bdac2ed8c8]
Uploaded image to:
gcr.io/asra-ali/busybox/root.json@sha256:6e6251e7f656c207e265d0629e9cf9d1fb9539b12f3505ceebb90bf16be9befc

The output file is (currently unsigned)

{
  "signatures": null,
  "signed": {
    "_type": "root",
    "consistent_snapshot": true,
    "expires": "2021-12-29T19:48:52Z",
    "keys": {
      "94e61dba379a148be37639c8bf7ece0087df6dd59733d0a3fd21da7ad1a0ac29": {
        "keyid_hash_algorithms": [
          "sha256",
          "sha512"
        ],
        "keytype": "sigstore-oidc",
        "keyval": {
          "identity": "[email protected]",
          "issuer": ""
        },
        "scheme": "https://fulcio.sigstore.dev"
      },
      "d9e0fe253edc4349d26dcdac691dad02c142be05e959b11577d151551c34ab8c": {
        "keyid_hash_algorithms": [
          "sha256",
          "sha512"
        ],
        "keytype": "sigstore-oidc",
        "keyval": {
          "identity": "[email protected]",
          "issuer": ""
        },
        "scheme": "https://fulcio.sigstore.dev"
      }
    },
    "namespace": "gcr.io/asra-ali/busybox",
    "roles": {
      "root": {
        "keyids": [
          "d9e0fe253edc4349d26dcdac691dad02c142be05e959b11577d151551c34ab8c",
          "94e61dba379a148be37639c8bf7ece0087df6dd59733d0a3fd21da7ad1a0ac29"
        ],
        "threshold": 2
      }
    },
    "spec_version": "1.0",
    "version": 1
  }
}

@lukehinds
Copy link
Member

nice, thanks @asraa . I am not to familiar with the go-tuf client, so excuse my greenness. Is there anyway this root policy is fixed to the registry namespace (in the signed envelope), what would stop someone lifting this and putting into a different registry namespace? I was just thinking if we should stash the registry in the signed section.

@lukehinds
Copy link
Member

lukehinds commented Jul 22, 2021

nice, thanks @asraa . I am not to familiar with the go-tuf client, so excuse my greenness. Is there anyway this root policy is fixed to the registry namespace (in the signed envelope), what would stop someone lifting this and putting into a different registry namespace? I was just thinking if we should stash the registry in the signed section.

one more question, the cert is the fulcio cert and we can pull the email from there? how is the keyid generated?

@asraa
Copy link
Contributor Author

asraa commented Jul 22, 2021

one more question, the cert is the fulcio cert and we can pull the email from there? how is the keyid generated?

Oops! The public value is the email, I accidentally hex encoded it. I'll update that and the output. The keyid is the hash over all of the key data (type, public value, scheme). This is to prevent someone a double signing if someone uses a different fulcio server.

Is there anyway this root policy is fixed to the registry namespace (in the signed envelope), what would stop someone lifting this and putting into a different registry namespace? I was just thinking if we should stash the registry in the signed section.

That indeed is a TODO in the code! It seemed like the targets that are being signed (whose paths are specified) are always relative to the root... The root keys are supposed to be provided out of band, to establish initial trust. I have to give this some more thought.

@lukehinds
Copy link
Member

Thanks @asraa , happy to chat it over anytime.

go.mod Outdated Show resolved Hide resolved
@asraa
Copy link
Contributor Author

asraa commented Jul 28, 2021

Thanks @asraa , happy to chat it over anytime.

BTW some thoughts on this: the signing event sent to rekor should include the namespace (verifying that the signing happened in that registry namespace). verifying a policy added to a random registry should fail when verifying the signing event on rekor. WDYT?

@lukehinds
Copy link
Member

Thanks @asraa , happy to chat it over anytime.

BTW some thoughts on this: the signing event sent to rekor should include the namespace (verifying that the signing happened in that registry namespace). verifying a policy added to a random registry should fail when verifying the signing event on rekor. WDYT?

Yep, I agree. If the namespace is already in signed envelope that would mean it ends up in the log anyway?

@asraa
Copy link
Contributor Author

asraa commented Jul 28, 2021

@lukehinds PR is updated so it at least points to me local fork with the changes in go-tuf that are needed

Still working on:

  • Updating the key field to actually display an email
  • Adding a namespace to Root signed payload

@asraa
Copy link
Contributor Author

asraa commented Sep 29, 2021

@lukehinds I decided for ease of getting this in, I'll simplify and remove go-tuf dependency. PTAL just does the metadata generation, not the signing.

Signed-off-by: Asra Ali <[email protected]>
@asraa
Copy link
Contributor Author

asraa commented Sep 29, 2021

@lukehinds let me know if the CLI I implemented was wrong -- I think we actually want a subcommand for policy with commands like init, sign, etc like

cosign policy init ...
cosign policy sign ...

@lukehinds
Copy link
Member

lukehinds commented Sep 30, 2021

Looks good @Asra

Yep , we need a sign operation next. Should we leverage an existing signer / json parse in go-tuf or implement our own?

p.s. bit of coverage if you like https://github.com/lukehinds/cosign/blob/2256bb6a101e8a5f3d0710f524fd6104406690dd/cmd/cosign/cli/policy_init_test.go

@lukehinds
Copy link
Member

lukehinds commented Sep 30, 2021

It's an improve from me, but i don't normally review a lot in cosign, so happy for @dlorenc @dekkagaijin and others to pitch in.

@asraa
Copy link
Contributor Author

asraa commented Sep 30, 2021

Should we leverage an existing signer / json parse in go-tuf or implement our own?

I have a Sign method:

sig, err := key.SignMessage(bytes.NewReader(s.Signed))

I actually will punt this to the sign command PR

Signed-off-by: Asra Ali <[email protected]>
@asraa
Copy link
Contributor Author

asraa commented Sep 30, 2021

Cleaned up, tests added, subcommand added so you run cosign policy init ... and I'll add cosign policy sign in a follow up

@asraa asraa requested a review from dekkagaijin September 30, 2021 17:14
@dlorenc dlorenc merged commit a568dad into sigstore:main Sep 30, 2021
@github-actions github-actions bot added this to the v1.3.0 milestone Sep 30, 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.

4 participants