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

Idea: alias --cert-email as --cert-identity/--cert-san #108

Closed
woodruffw opened this issue Jun 2, 2022 · 7 comments · Fixed by #289
Closed

Idea: alias --cert-email as --cert-identity/--cert-san #108

woodruffw opened this issue Jun 2, 2022 · 7 comments · Fixed by #289
Labels
component:cli CLI components component:verification Core verification functionality enhancement New feature or request

Comments

@woodruffw
Copy link
Member

Breaking this out for discussion from #107: --cert-email can be misleading when the OIDC identity in question isn't an email, but rather a GitHub workflow or other URL. We could provide CLI aliases that make this clearer.

cc @di for thoughts.

@woodruffw woodruffw added enhancement New feature or request component:cli CLI components component:verification Core verification functionality labels Jun 2, 2022
@jku
Copy link
Member

jku commented Jun 6, 2022

Related: Github workflow certificate uses six other extensions in addition to SAN -- they must think verifying those have value, and the extensions there do seem useful. Other systems likely have different extensions they include.

So observations:

  • A more generic --cert-extension <id> <value> could be useful (there are some potential problems with more complex datatypes but for the GitHub example this would work fine)
  • --cert-identity or --cert-san probably still makes sense as a more readable short hand for this specific case

@woodruffw
Copy link
Member Author

Related: Github workflow certificate uses six other extensions in addition to SAN -- they must think verifying those have value, and the extensions there do seem useful. Other systems likely have different extensions they include.

Yep. We've added the OIDs for those extensions in #117. We need to figure out which set of CLI options we should expose for them, which will involve some coordination with cosign...

@woodruffw
Copy link
Member Author

Opened sigstore/cosign#1964 for cosign's side.

@woodruffw
Copy link
Member Author

  • A more generic --cert-extension <id> <value> could be useful (there are some potential problems with more complex datatypes but for the GitHub example this would work fine)

I think this is a good idea, although we probably want to think about the UX -- having <id> be a dotted-string is probably not ideal, so we'll probably want a standard set of human-friendly names that cosign and sigstore-python share.

@jku
Copy link
Member

jku commented Jun 7, 2022

You must have noticed this already but since it's not written anywhere:

We previously discussed that --cert-email might already work to verify whatever fields are in SAN: this is untrue as cryptography API has different types for different kinds of SANs. In practice san_ext.value.get_values_for_type(RFC822Name) will only return the SANs that are emails, not URIs like the GitHub identifier.

@woodruffw
Copy link
Member Author

Yeah, this is an unfortunate case where cryptography is more correct in a way that causes more work for us 🙂

We won't make any movement here until we get some consensus with cosign on sigstore/cosign#1964, so I'll follow up with some design thoughts there.

@woodruffw
Copy link
Member Author

It looks like cosign has merged partial support for --cert-identity, so we should probably go ahead and do the same.

See: sigstore/cosign#2056

woodruffw added a commit that referenced this issue Nov 3, 2022
Closes #108.

Signed-off-by: William Woodruff <william@trailofbits.com>
javanlacerda pushed a commit to javanlacerda/sigstore-python that referenced this issue Feb 23, 2024
* Add raises() contextmanager

This allows managing the "unexpected success" case, and printing
the same details as in "unexpected failure" case.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>

* Change the failure output slightly

Try to print a usable shell command:
 * use str() so that we don't get "PosixPath()" in the args list
 * join the arguments into a single string

This means that I can now copy paste the command and reproduce the
problem by just being in the same directory with assets.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>

---------

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:cli CLI components component:verification Core verification functionality enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants