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

Policy for verifying with key #105

Closed
haydentherapper opened this issue Feb 22, 2024 · 3 comments
Closed

Policy for verifying with key #105

haydentherapper opened this issue Feb 22, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request v1.0 items we want to consider for a v1.0 release

Comments

@haydentherapper
Copy link
Contributor

Description

Currently, when verifying with a key, you must specify WithoutIdentitiesUnsafe() when creating the policy verifier because no identity is provided. It would be more accurate to have a method such as WithKeyHint or WithKeyID, since identities are not needed when verifying with a key. Either this method could take a key fingerprint, or it could take no fingerprint given the bundle should contain a key hint.

If verifying with a Sigstore bundle, a key hint should be provided as a public key identifier. As long as the trust root contains the public key, we can match based on this key hint (currently implemented here. If this is not populated, either a) we should err out, or b) we should use a hint provided via the policy method.

Related: sigstore/protobuf-specs#236 to simplify providing trusted keys.

cc @kommendorkapten @codysoyland @rdimitrov

@haydentherapper haydentherapper added the enhancement New feature or request label Feb 22, 2024
@kommendorkapten
Copy link
Member

Having either WithKeyHint or WithKey as the identity for a policy is what I would expect, as that's how the verification options in the protobuf specs repo are described: https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_verification.proto#L92

For a specific verification, I would prefer WithKey as it minimizes the risk of confusion. If WithKeyHint is used, I think it's preferable if that selection happens in a client controlled function, as an implementation may only have a single key, and always return that, regardless what the key hint is (the key hint not authenticated in the bundle).

That said, providing a matcher based on key hint (as @haydentherapper linked to) is a good utility option to provide, as it may be enough for certain scenarios.

@kommendorkapten
Copy link
Member

See my comment in sigstore/protobuf-specs#236 (comment) for more reasoning.
The short story is that public keys are only weakly references via unauthenticated hints, certificate identities are strongly as the matching happens on authenticated data in the certificate.

@haydentherapper haydentherapper added the v1.0 items we want to consider for a v1.0 release label Jun 5, 2024
@steiza steiza self-assigned this Jul 16, 2024
steiza added a commit that referenced this issue Jul 17, 2024
…ey (#235)

To address #105.

Right now, if you have a bundle signed with a key, your verification policy needs to include WithoutIdentitiesUnsafe(). This isn't great though because:

- bundles signed with keys aren't inherently unsafe
- there isn't a way to require your bundle was signed with a key; a bundle signed with a certificate would pass WithoutIdentitiesUnsafe().

---------

Signed-off-by: Zach Steindler <steiza@github.com>
@steiza
Copy link
Member

steiza commented Jul 17, 2024

This was added in #235

@steiza steiza closed this as completed Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v1.0 items we want to consider for a v1.0 release
Projects
None yet
Development

No branches or pull requests

3 participants