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

Generated PEM labels should say "SIGSTORE" rather than "COSIGN" #2471

Closed
znewman01 opened this issue Nov 18, 2022 · 7 comments · Fixed by #2735
Closed

Generated PEM labels should say "SIGSTORE" rather than "COSIGN" #2471

znewman01 opened this issue Nov 18, 2022 · 7 comments · Fixed by #2735
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@znewman01
Copy link
Contributor

$ cd $(mktemp -d) && COSIGN_PASSWORD="" cosign generate-key-pair && head -n 1 cosign.key
-----BEGIN ENCRYPTED COSIGN PRIVATE KEY-----

This means that any Sigstore client implementation must:

  1. Accept the "COSIGN PRIVATE KEY" format, or else they won't be able to use Cosign keys
  2. Generate keys with "COSIGN PRIVATE KEY", or else Cosign won't be able to understand those keys. This is a little weird if the client has nothing to do with cosign.

I'd prefer "BEGIN ENCRYPTED SIGSTORE PRIVATE KEY" if this is going to be a shared format. (That's what sigstore-rs emits.)

I propose that Cosign:

  • Immediately start accepting "SIGSTORE" keys (in addition to "COSIGN" keys)
  • Eventually start emitting "SIGSTORE" keys.
    • I think we could definitely do this right away if we get it in before 2.0 (Cosign 2.0 Tracking #2376)
    • Otherwise, we may want to transition the behavior subject to a deprecation period (as in Proposal: Cosign Versioning). I can see an argument for skipping deprecation too.
  • Very long-term could stop accepting "COSIGN" public keys, probably doesn't need to do this though.
  • We could consider an option to emit "COSIGN" keys for backwards compat.

Other Sigstore clients:

  • MUST emit "SIGSTORE" keys by default
  • SHOULD accept "COSIGN" keys
  • MAY have an option to emit "COSIGN" keys

Context: sigstore/sigstore-rs#165

@haydentherapper
Copy link
Contributor

Do we need a Sigstore specific label? Can we just require that clients understand a few different encodings?

@znewman01
Copy link
Contributor Author

I would prefer not to have our own PEM labels at all. Conditional on having PEM labels, I want them to be "SIGSTORE" instead of "COSIGN".

I assume there's a good reason for having our own label; the history isn't well documented.

@haydentherapper
Copy link
Contributor

If I were to guess, it was maybe a way to enforce that the private key is password protected. Though there's already PEM headers for that, so maybe that would be sufficient? @dlorenc or @priyawadhwa do you recall why we chose our own PEM labels?

@priyawadhwa
Copy link
Contributor

No specific reason that I know of!

@Xynnn007
Copy link
Member

Xynnn007 commented Nov 21, 2022

If I were to guess, it was maybe a way to enforce that the private key is password protected. Though there's already PEM headers for that, so maybe that would be sufficient? @dlorenc or @priyawadhwa do you recall why we chose our own PEM labels?

Hi, FYI, there is already PEM headers for private keys like PRIVATE KEY or something else, but the private keys used in cosign and sigstore are encrypted in a special scheme. It is defined in TUF, and the scheme involves xsalsa20poly1305 as AEAD and scrypt as KDF. I think the algorithm combination may be the reason using a new tag.

cc @dekkagaijin

@ivanayov
Copy link
Contributor

Can I work on this?

@znewman01
Copy link
Contributor Author

Sure! Thanks for volunteering :) Ping if you need any help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants