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

verify/-blob should provide more guidance in identity assertions #2804

Open
philpennock opened this issue Mar 15, 2023 · 5 comments
Open

verify/-blob should provide more guidance in identity assertions #2804

philpennock opened this issue Mar 15, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@philpennock
Copy link

Description

With cosign 2, when verifying a signature in the transparency logs, an OIDC issuer and subject must be matched, but there's very little guidance as to what the options are. The error messages just specify that the options are needed, and even when using --verbose with the -regexp variants, we can't see what was actually matched.

So when people do "the easy thing", they'll find that the easiest thing is to use:

cosign verify-blob --certificate-identity-regexp=.+ --certificate-oidc-issuer-regexp=.+ [...]

which is counterproductive.

My operating model here is that the first time people encounter this is when they've downloaded artifacts from safe locations and they're trying to ensure that future downloads, perhaps inside CI systems, as also safely downloaded. So if someone does not yet know what identity they should validate, then the data they see before them is reasonable to present as suggestions, together with "if this is reasonable and accurate" caveats.

Thus if neither option is given, then instead of complaining about first one being missing, then the other, they should be complained about together, plus seeing the actual data which can be matched against. Or if #2210 is implemented, then people can be pointed to that command.

Further, if the repository information truly is deprecated, so that the only fully supported option is to match against URLs, then the help text should warn against regexp metacharacters and present escape command-line options (or reconsider if perhaps the ability to match against a repo name is important, for safe usage).

I am not a cosign expert, but am more aware of the problem space and terminology than 95% of developers, and it took me far too long to get to an invocation of:

cosign verify-blob \
  --certificate-oidc-issuer https://token.actions.githubusercontent.com \
  --certificate-identity-regexp 'https://github\.com/goreleaser/goreleaser-pro-internal/\.github/.+' \ 
  --signature checksums.txt.sig --cert checksums.txt.pem \
  checksums.txt
@znewman01
Copy link
Contributor

So if someone does not yet know what identity they should validate, then the data they see before them is reasonable to present as suggestions, together with "if this is reasonable and accurate" caveats.

On the one hand, I agree that this is a common workflow and TOFU is often reasonable here. But TOFU-by-default is a little scary, even if we provide a warning message. In the past, a cosign inspect command (which you've found) was proposed for this situation (#2210) but it has many of the same problems. You've given me an idea...will present it below.

Thus if neither option is given, then instead of complaining about first one being missing, then the other, they should be complained about together, plus seeing the actual data which can be matched against.

Strongly agreed. The internal changes I propose in this comment would make this much easier to implement but they don't necessarily need to block this.

Further, if the repository information truly is deprecated, so that the only fully supported option is to match against URLs, then the help text should warn against regexp metacharacters and present escape command-line options (or reconsider if perhaps the ability to match against a repo name is important, for safe usage).

We may have to pull in some of the GitHub folks on this one; #2691 is a good place to discuss that.


Appreciate your patience so far with these workflows...we started with "really easy to shoot yourself in the foot" and the pendulum has swung to "footguns removed by default but the process is annoying enough you might take them back out." Hopefully the next round of changes will help restore usability without losing the security here.

CC @haydentherapper

@haydentherapper
Copy link
Contributor

Thanks for the feedback! Agreed that we need to think through the UX for CI systems more.

As a first step, warning on overly permissive regex (.*/.+) would be good.

@philpennock
Copy link
Author

For clarity, my TOFU-adjacent suggestion is not "accept it, with TOFU". Instead, it's to error out as unverified, but suggest in text that "it looks like this invocation below would pass, but it's on you to double-check that the values are reasonable, before adopting them into your workflows"

@znewman01
Copy link
Contributor

Understood, but I consider any workflow that relies on a human squinting at a string that has to be "just right" to be especially vulnerable to typosquatting attacks (and therefore, as you put it, TOFU-adjacent). In my ideal world, nobody would ever try to describe verification criteria to Cosign; they'd fetch a policy out-of-band. We're not there yet, so trying to strike a reasonable compromise.

@philpennock
Copy link
Author

philpennock commented Mar 22, 2023

So, error out as unverified, but:

To see which verification criteria would work for this signature, re-invoke with the --gullible-trust-suggest-verifications option.

and then that option should put in a big warning about "the below assumes that you are looking at an untampered image, use discretion before adopting".

That's really what I'm getting at. Someone needs to get the verification criteria and most projects aren't organized to do this, so the admins of systems pulling objects which need verifying (containers, blobs) using cosign need to be able to craft the criteria. Cosign can see what would work, so the issue is the UX around "okay, so what would work" and making sure that it's not something which gets put into CI workflows as the "--magic-make-this-all-just-exit-zero" option but is instead an enabling workflow for getting the correct values to be used later.

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

No branches or pull requests

3 participants