`cosign` and `fulcio` disagree on proof of possession

[mattmoor]

Description

I believe cosign uses this logic to extract the "subject" for proof of possession: https://github.com/sigstore/sigstore/blob/9a7027012170c0070b8842b1bda6f0050e420097/pkg/oauthflow/flow.go#L123-L144

This is inconsistent with how Fulcio extracts this on the server side here: https://github.com/sigstore/fulcio/blob/9f49a8b6e090ec4e01b77760fd8306dd3c2c1b84/pkg/server/grpc_server.go#L132-L135

Version

2.2.4

cc @haydentherapper @priyawadhwa

[mattmoor]

The answer could be: This is a Fulcio bug, it shouldn't be using principal.Name here, but I flipped a coin on where to open this to kick off the discussion.

[haydentherapper]

In Cosign, the proof of possession is signed over either the email, or if no email, the token subject.

For identity providers configured as email providers, when received by Fulcio, it will use the email claim based on https://github.com/sigstore/fulcio/blob/main/pkg/identity/email/principal.go#L66. For Chainguard's identity provider, Fulcio will use the subject - https://github.com/sigstore/fulcio/blob/main/pkg/identity/chainguard/principal.go#L38

The issue is that the token you're providing contains an email, so that is what Cosign chose, over the subject. Do you want to support tokens with emails? If your tokens will always contain emails, either we update Fulcio's Chainguard identity provider to use the email claim when verifying the PoP, or we add a flag to Cosign to specify the claim value to sign over. If your tokens will never contain emails, then there are no changes needed. If your tokens will sometimes contain emails, a flag to select the claim value would probably be easiest.

(As a later refactor, we really should standardize on what should be signed over, it should always be sub in my opinion. Or we switch to CSRs as the standard way to provide the public key and subject (and is self-signed). Or something like ACME with dynamic challenges.)

[mattmoor]

We want to support using our sub regardless of the token having or not having an email claim.

Given that this is just for the PoP, I don't understand why Fulcio wouldn't just use the exact-same logic from sigstore/sigstore as cosign?

It must be the case that today every provider either:

1. Leverages email, or
2. Never embeds an email claim, and uses sub for principal.Name.

... otherwise they wouldn't work, so simply having the Fulcio PoP logic call the same function as cosign seems like the right fix here to me...

[mattmoor]

it should always be sub in my opinion

+1 fwiw 😁

[haydentherapper]

email is more of an exception, all other providers use sub. We can't make that change any time soon unfortunately without breaking changes across clients.

I think it would be fine to change the CG provider in Fulcio to dynamically pick the PoP subject based on the token claims, same as Cosign.

[mattmoor]

@haydentherapper Yeah, I don't think we could switch to sub globally without breaking older cosign with key providers (like Google!), but my main point is:

There isn't a correct implementation of Name() that isn't oauthflow.SubjectFromToken(token) (or equivalent).

Changing to this is hard given the way the abstraction is formulated, and I'm going to adjust our Name() to match this, but IMO the PoP logic should be changed to simply call this function instead of relying on Name() because this is the only correct way to implement Name() today.