Skip to content

Commit d16be8d

Browse files
authored
Surface the right Name() from our principal. (#1726)
The cosign logic for interacting with Fulcio treats identity tokens as *largely* opaque, and most of the logic for how issuers and subjects and whatnot is handled happens server-side. However, for the "proof of possession" `cosign` has some logic (from `sigstore/sigstore`) that fumbles with `email` and `sub` claims in ways that have (until now) been compatible with Fulcio principals. The Chainguard provider is the first provider that optionally includes an `email` claim, but we always want the subject we use to be our opaque identifier string (from `sub`). This creates a tear in the fulcio/cosign continuum, and so we must surface what `cosign` is signing as `Name()` even though that isn't necessarily what we embed in the certificate. The only correct way to implement `Name()` today is to match what this function does, and current implementations happen to align, but unfortunately because of how this abstraction is formulated it is challenging to actually change how we confirm the proof of possession to use this directly in place of the principal itself. Fixes: sigstore/cosign#3777 Signed-off-by: Matt Moore <[email protected]>
1 parent 9f49a8b commit d16be8d

File tree

2 files changed

+42
-1
lines changed

2 files changed

+42
-1
lines changed

pkg/identity/chainguard/principal.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@ import (
2222
"github.com/coreos/go-oidc/v3/oidc"
2323
"github.com/sigstore/fulcio/pkg/certificate"
2424
"github.com/sigstore/fulcio/pkg/identity"
25+
"github.com/sigstore/sigstore/pkg/oauthflow"
2526
)
2627

2728
type workflowPrincipal struct {
2829
issuer string
2930
subject string
31+
name string
3032

3133
actor map[string]string
3234
servicePrincipal string
@@ -35,7 +37,7 @@ type workflowPrincipal struct {
3537
var _ identity.Principal = (*workflowPrincipal)(nil)
3638

3739
func (w workflowPrincipal) Name(_ context.Context) string {
38-
return w.subject
40+
return w.name
3941
}
4042

4143
func PrincipalFromIDToken(_ context.Context, token *oidc.IDToken) (identity.Principal, error) {
@@ -50,9 +52,19 @@ func PrincipalFromIDToken(_ context.Context, token *oidc.IDToken) (identity.Prin
5052
return nil, err
5153
}
5254

55+
// This is the exact function that cosign uses to extract the "subject"
56+
// (misnomer) from the token in order to establish "proof of possession".
57+
// We MUST use this to implement Name() or tokens that embed an email claim
58+
// will fail to sign because of this divergent logic.
59+
name, err := oauthflow.SubjectFromToken(token)
60+
if err != nil {
61+
return nil, err
62+
}
63+
5364
return &workflowPrincipal{
5465
issuer: token.Issuer,
5566
subject: token.Subject,
67+
name: name,
5668
actor: claims.Actor,
5769
servicePrincipal: claims.Internal.ServicePrincipal,
5870
}, nil

pkg/identity/chainguard/principal_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ func TestJobPrincipalFromIDToken(t *testing.T) {
6060
ExpectPrincipal: workflowPrincipal{
6161
issuer: "https://issuer.enforce.dev",
6262
subject: id.String(),
63+
name: id.String(),
6364
actor: map[string]string{
6465
"iss": "https://iss.example.com/",
6566
"sub": fmt.Sprintf("catalog-syncer:%s", group.String()),
@@ -85,6 +86,34 @@ func TestJobPrincipalFromIDToken(t *testing.T) {
8586
ExpectPrincipal: workflowPrincipal{
8687
issuer: "https://issuer.enforce.dev",
8788
subject: group.String(),
89+
name: group.String(),
90+
actor: map[string]string{
91+
"iss": "https://auth.chainguard.dev/",
92+
"sub": "google-oauth2|1234567890",
93+
"aud": "fdsaldfkjhasldf",
94+
},
95+
},
96+
WantErr: false,
97+
},
98+
`Human SSO token (with email)`: {
99+
Claims: map[string]interface{}{
100+
"iss": "https://issuer.enforce.dev",
101+
"sub": group.String(),
102+
"email": "[email protected]",
103+
"email_verified": true,
104+
// Actor claims track the identity that was used to assume the
105+
// Chainguard identity. In this case, it is the Catalog Syncer
106+
// service principal.
107+
"act": map[string]string{
108+
"iss": "https://auth.chainguard.dev/",
109+
"sub": "google-oauth2|1234567890",
110+
"aud": "fdsaldfkjhasldf",
111+
},
112+
},
113+
ExpectPrincipal: workflowPrincipal{
114+
issuer: "https://issuer.enforce.dev",
115+
subject: group.String(),
116+
88117
actor: map[string]string{
89118
"iss": "https://auth.chainguard.dev/",
90119
"sub": "google-oauth2|1234567890",

0 commit comments

Comments
 (0)