From 2917b0e282e3ada974eaa7909a7fed8fce7685b9 Mon Sep 17 00:00:00 2001 From: Nathan Smith Date: Thu, 19 May 2022 17:06:06 -0700 Subject: [PATCH] Move github principal to its own package Remove github workflow logic from challenge result and put it in its own package. Signed-off-by: Nathan Smith --- pkg/challenges/challenges.go | 105 +------- pkg/challenges/challenges_test.go | 38 --- pkg/identity/github/principal.go | 127 ++++++++++ pkg/identity/github/principal_test.go | 333 ++++++++++++++++++++++++++ 4 files changed, 462 insertions(+), 141 deletions(-) create mode 100644 pkg/identity/github/principal.go create mode 100644 pkg/identity/github/principal_test.go diff --git a/pkg/challenges/challenges.go b/pkg/challenges/challenges.go index 9daa9815b..aea36c80e 100644 --- a/pkg/challenges/challenges.go +++ b/pkg/challenges/challenges.go @@ -28,6 +28,7 @@ import ( "github.com/sigstore/fulcio/pkg/ca/x509ca" "github.com/sigstore/fulcio/pkg/config" "github.com/sigstore/fulcio/pkg/identity" + "github.com/sigstore/fulcio/pkg/identity/github" "github.com/spiffe/go-spiffe/v2/spiffeid" "github.com/coreos/go-oidc/v3/oidc" @@ -41,23 +42,11 @@ type ChallengeType int const ( EmailValue ChallengeType = iota SpiffeValue - GithubWorkflowValue KubernetesValue URIValue UsernameValue ) -type AdditionalInfo int - -// Additional information that can be added as a cert extension. -const ( - GithubWorkflowTrigger AdditionalInfo = iota - GithubWorkflowSha - GithubWorkflowName - GithubWorkflowRepository - GithubWorkflowRef -) - type ChallengeResult struct { Issuer string TypeVal ChallengeType @@ -66,9 +55,6 @@ type ChallengeResult struct { // the certificate issued. Value string - // Extra information from the token that can be added to extensions. - AdditionalInfo map[AdditionalInfo]string - // subject or email from the id token. This must be the thing // signed in the proof of possession! subject string @@ -88,12 +74,6 @@ func (cr *ChallengeResult) Embed(ctx context.Context, cert *x509.Certificate) er return err } cert.URIs = []*url.URL{challengeURL} - case GithubWorkflowValue: - jobWorkflowURI, err := url.Parse(cr.Value) - if err != nil { - return err - } - cert.URIs = []*url.URL{jobWorkflowURI} case KubernetesValue: k8sURI, err := url.Parse(cr.Value) if err != nil { @@ -113,29 +93,6 @@ func (cr *ChallengeResult) Embed(ctx context.Context, cert *x509.Certificate) er exts := x509ca.Extensions{ Issuer: cr.Issuer, } - if cr.TypeVal == GithubWorkflowValue { - var ok bool - exts.GithubWorkflowTrigger, ok = cr.AdditionalInfo[GithubWorkflowTrigger] - if !ok { - return errors.New("github workflow missing trigger claim") - } - exts.GithubWorkflowSHA, ok = cr.AdditionalInfo[GithubWorkflowSha] - if !ok { - return errors.New("github workflow missing SHA claim") - } - exts.GithubWorkflowName, ok = cr.AdditionalInfo[GithubWorkflowName] - if !ok { - return errors.New("github workflow missing workflow name claim") - } - exts.GithubWorkflowRepository, ok = cr.AdditionalInfo[GithubWorkflowRepository] - if !ok { - return errors.New("github workflow missing repository claim") - } - exts.GithubWorkflowRef, ok = cr.AdditionalInfo[GithubWorkflowRef] - if !ok { - return errors.New("github workflow missing ref claim") - } - } var err error cert.ExtraExtensions, err = exts.Render() @@ -228,25 +185,6 @@ func kubernetes(ctx context.Context, principal *oidc.IDToken) (identity.Principa }, nil } -func githubWorkflow(ctx context.Context, principal *oidc.IDToken) (identity.Principal, error) { - workflowRef, err := workflowFromIDToken(principal) - if err != nil { - return nil, err - } - additionalInfo, err := workflowInfoFromIDToken(principal) - if err != nil { - return nil, err - } - - return &ChallengeResult{ - Issuer: principal.Issuer, - TypeVal: GithubWorkflowValue, - Value: workflowRef, - AdditionalInfo: additionalInfo, - subject: principal.Subject, - }, nil -} - func uri(ctx context.Context, principal *oidc.IDToken) (identity.Principal, error) { uriWithSubject := principal.Subject @@ -335,45 +273,6 @@ func kubernetesToken(token *oidc.IDToken) (string, error) { return "https://kubernetes.io/namespaces/" + claims.Kubernetes.Namespace + "/serviceaccounts/" + claims.Kubernetes.ServiceAccount.Name, nil } -func workflowFromIDToken(token *oidc.IDToken) (string, error) { - // Extract custom claims - var claims struct { - JobWorkflowRef string `json:"job_workflow_ref"` - // The other fields that are present here seem to depend on the type - // of workflow trigger that initiated the action. - } - if err := token.Claims(&claims); err != nil { - return "", err - } - - // We use this in URIs, so it has to be a URI. - return "https://github.com/" + claims.JobWorkflowRef, nil -} - -func workflowInfoFromIDToken(token *oidc.IDToken) (map[AdditionalInfo]string, error) { - // Extract custom claims - var claims struct { - Sha string `json:"sha"` - Trigger string `json:"event_name"` - Repository string `json:"repository"` - Workflow string `json:"workflow"` - Ref string `json:"ref"` - // The other fields that are present here seem to depend on the type - // of workflow trigger that initiated the action. - } - if err := token.Claims(&claims); err != nil { - return nil, err - } - - // We use this in URIs, so it has to be a URI. - return map[AdditionalInfo]string{ - GithubWorkflowSha: claims.Sha, - GithubWorkflowTrigger: claims.Trigger, - GithubWorkflowName: claims.Workflow, - GithubWorkflowRepository: claims.Repository, - GithubWorkflowRef: claims.Ref}, nil -} - func PrincipalFromIDToken(ctx context.Context, tok *oidc.IDToken) (identity.Principal, error) { iss, ok := config.FromContext(ctx).GetIssuer(tok.Issuer) if !ok { @@ -387,7 +286,7 @@ func PrincipalFromIDToken(ctx context.Context, tok *oidc.IDToken) (identity.Prin case config.IssuerTypeSpiffe: principal, err = spiffe(ctx, tok) case config.IssuerTypeGithubWorkflow: - principal, err = githubWorkflow(ctx, tok) + principal, err = github.WorkflowPrincipalFromIDToken(ctx, tok) case config.IssuerTypeKubernetes: principal, err = kubernetes(ctx, tok) case config.IssuerTypeURI: diff --git a/pkg/challenges/challenges_test.go b/pkg/challenges/challenges_test.go index 369f42520..b7513849f 100644 --- a/pkg/challenges/challenges_test.go +++ b/pkg/challenges/challenges_test.go @@ -45,44 +45,6 @@ func TestEmbedChallengeResult(t *testing.T) { WantErr bool WantFacts map[string]func(x509.Certificate) error }{ - `Github workflow challenge should have all Github workflow extensions and issuer set`: { - Challenge: ChallengeResult{ - Issuer: `https://token.actions.githubusercontent.com`, - TypeVal: GithubWorkflowValue, - Value: `https://github.com/foo/bar/`, - AdditionalInfo: map[AdditionalInfo]string{ - GithubWorkflowSha: "sha", - GithubWorkflowTrigger: "trigger", - GithubWorkflowName: "workflowname", - GithubWorkflowRepository: "repository", - GithubWorkflowRef: "ref", - }, - }, - WantErr: false, - WantFacts: map[string]func(x509.Certificate) error{ - `Certifificate should have correct issuer`: factIssuerIs(`https://token.actions.githubusercontent.com`), - `Certificate has correct trigger extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 2}, "trigger"), - `Certificate has correct SHA extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 3}, "sha"), - `Certificate has correct workflow extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 4}, "workflowname"), - `Certificate has correct repository extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 5}, "repository"), - `Certificate has correct ref extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 6}, "ref"), - }, - }, - `Github workflow value with bad URL fails`: { - Challenge: ChallengeResult{ - Issuer: `https://token.actions.githubusercontent.com`, - TypeVal: GithubWorkflowValue, - Value: "\nbadurl", - AdditionalInfo: map[AdditionalInfo]string{ - GithubWorkflowSha: "sha", - GithubWorkflowTrigger: "trigger", - GithubWorkflowName: "workflowname", - GithubWorkflowRepository: "repository", - GithubWorkflowRef: "ref", - }, - }, - WantErr: true, - }, `Email challenges should set issuer extension and email subject`: { Challenge: ChallengeResult{ Issuer: `example.com`, diff --git a/pkg/identity/github/principal.go b/pkg/identity/github/principal.go new file mode 100644 index 000000000..9af91b8c3 --- /dev/null +++ b/pkg/identity/github/principal.go @@ -0,0 +1,127 @@ +// Copyright 2022 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package github + +import ( + "context" + "crypto/x509" + "errors" + "net/url" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/sigstore/fulcio/pkg/ca/x509ca" + "github.com/sigstore/fulcio/pkg/identity" +) + +type workflowPrincipal struct { + // Subject matches the 'sub' claim from the OIDC ID token this is what is + // signed as proof of possession for Github workflow identities + subject string + + // OIDC Issuer URL. Matches 'iss' claim from ID token. The real issuer URL is + // https://token.actions.githubcontent.com/.well-known/openid-configution + issuer string + + // The full URL to the workflow. This will be the set as SubjectAlternativeName URI in + // the final certificate. + url string + + // Commit SHA being built + sha string + + // Event that triggered this workflow run. E.g "push", "tag" etc + trigger string + + // Repository building built + repository string + + // Workflow that is running + workflow string + + // Git ref being built + ref string +} + +func WorkflowPrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Principal, error) { + var claims struct { + JobWorkflowRef string `json:"job_workflow_ref"` + Sha string `json:"sha"` + Trigger string `json:"event_name"` + Repository string `json:"repository"` + Workflow string `json:"workflow"` + Ref string `json:"ref"` + } + if err := token.Claims(&claims); err != nil { + return nil, err + } + + if claims.JobWorkflowRef == "" { + return nil, errors.New("missing job_workflow_ref claim in ID token") + } + if claims.Sha == "" { + return nil, errors.New("missing sha claim in ID token") + } + if claims.Trigger == "" { + return nil, errors.New("missing event_name claim in ID token") + } + if claims.Repository == "" { + return nil, errors.New("missing repository claim in ID token") + } + if claims.Workflow == "" { + return nil, errors.New("missing workflow claim in ID token") + } + if claims.Ref == "" { + return nil, errors.New("missing ref claim in ID token") + } + + return &workflowPrincipal{ + subject: token.Subject, + issuer: token.Issuer, + url: `https://github.com/` + claims.JobWorkflowRef, + sha: claims.Sha, + trigger: claims.Trigger, + repository: claims.Repository, + workflow: claims.Workflow, + ref: claims.Ref, + }, nil +} + +func (w workflowPrincipal) Name(ctx context.Context) string { + return w.subject +} + +func (w workflowPrincipal) Embed(ctx context.Context, cert *x509.Certificate) error { + // Set workflow URL to SubjectAlternativeName on certificate + parsed, err := url.Parse(w.url) + if err != nil { + return err + } + cert.URIs = []*url.URL{parsed} + + // Embed additional information into custom extensions + cert.ExtraExtensions, err = x509ca.Extensions{ + Issuer: w.issuer, + GithubWorkflowTrigger: w.trigger, + GithubWorkflowSHA: w.sha, + GithubWorkflowName: w.workflow, + GithubWorkflowRepository: w.repository, + GithubWorkflowRef: w.ref, + }.Render() + if err != nil { + return err + } + + return nil +} diff --git a/pkg/identity/github/principal_test.go b/pkg/identity/github/principal_test.go new file mode 100644 index 000000000..3dbabcea3 --- /dev/null +++ b/pkg/identity/github/principal_test.go @@ -0,0 +1,333 @@ +// Copyright 2022 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package github + +import ( + "bytes" + "context" + "crypto/x509" + "encoding/asn1" + "encoding/json" + "errors" + "fmt" + "reflect" + "strings" + "testing" + "unsafe" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/sigstore/fulcio/pkg/identity" +) + +func TestWorkflowPrincipalFromIDToken(t *testing.T) { + tests := map[string]struct { + Claims map[string]interface{} + ExpectPrincipal workflowPrincipal + WantErr bool + ErrContains string + }{ + `Valid token authenticates with correct claims`: { + Claims: map[string]interface{}{ + "aud": "sigstore", + "event_name": "push", + "exp": 0, + "iss": "https://token.actions.githubusercontent.com", + "job_workflow_ref": "sigstore/fulcio/.github/workflows/foo.yaml@refs/heads/main", + "ref": "refs/heads/main", + "repository": "sigstore/fulcio", + "sha": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "sub": "repo:sigstore/fulcio:ref:refs/heads/main", + "workflow": "foo", + }, + ExpectPrincipal: workflowPrincipal{ + issuer: "https://token.actions.githubusercontent.com", + subject: "repo:sigstore/fulcio:ref:refs/heads/main", + url: "https://github.com/sigstore/fulcio/.github/workflows/foo.yaml@refs/heads/main", + sha: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + trigger: "push", + repository: "sigstore/fulcio", + workflow: "foo", + ref: "refs/heads/main", + }, + WantErr: false, + }, + `Token missing job_workflow_ref claim should be rejected`: { + Claims: map[string]interface{}{ + "aud": "sigstore", + "event_name": "push", + "exp": 0, + "iss": "https://token.actions.githubusercontent.com", + "ref": "refs/heads/main", + "repository": "sigstore/fulcio", + "sha": "bf96275471e83ff04ce5c8eb515c04a75d43f854", + "sub": "repo:sigstore/fulcio:ref:refs/heads/main", + "workflow": "foo", + }, + WantErr: true, + ErrContains: "job_workflow_ref", + }, + `Token missing sha should be rejected`: { + Claims: map[string]interface{}{ + "aud": "sigstore", + "event_name": "push", + "exp": 0, + "iss": "https://token.actions.githubusercontent.com", + "job_workflow_ref": "sigstore/fulcio/.github/workflows/foo.yaml@refs/heads/main", + "ref": "refs/heads/main", + "repository": "sigstore/fulcio", + "sub": "repo:sigstore/fulcio:ref:refs/heads/main", + "workflow": "foo", + }, + WantErr: true, + ErrContains: "sha", + }, + `Token missing event_name claim should be rejected`: { + Claims: map[string]interface{}{ + "aud": "sigstore", + "exp": 0, + "iss": "https://token.actions.githubusercontent.com", + "job_workflow_ref": "sigstore/fulcio/.github/workflows/foo.yaml@refs/heads/main", + "ref": "refs/heads/main", + "repository": "sigstore/fulcio", + "sha": "bf96275471e83ff04ce5c8eb515c04a75d43f854", + "sub": "repo:sigstore/fulcio:ref:refs/heads/main", + "workflow": "foo", + }, + WantErr: true, + ErrContains: "event_name", + }, + `Token missing repository claim should be rejected`: { + Claims: map[string]interface{}{ + "aud": "sigstore", + "event_name": "push", + "exp": 0, + "iss": "https://token.actions.githubusercontent.com", + "job_workflow_ref": "sigstore/fulcio/.github/workflows/foo.yaml@refs/heads/main", + "ref": "refs/heads/main", + "sha": "bf96275471e83ff04ce5c8eb515c04a75d43f854", + "sub": "repo:sigstore/fulcio:ref:refs/heads/main", + "workflow": "foo", + }, + WantErr: true, + ErrContains: "repository", + }, + `Token missing workflow claim should be rejected`: { + Claims: map[string]interface{}{ + "aud": "sigstore", + "event_name": "push", + "exp": 0, + "iss": "https://token.actions.githubusercontent.com", + "job_workflow_ref": "sigstore/fulcio/.github/workflows/foo.yaml@refs/heads/main", + "ref": "refs/heads/main", + "repository": "sigstore/fulcio", + "sha": "bf96275471e83ff04ce5c8eb515c04a75d43f854", + "sub": "repo:sigstore/fulcio:ref:refs/heads/main", + }, + WantErr: true, + ErrContains: "workflow", + }, + `Token missing ref claim should be rejected`: { + Claims: map[string]interface{}{ + "aud": "sigstore", + "event_name": "push", + "exp": 0, + "iss": "https://token.actions.githubusercontent.com", + "job_workflow_ref": "sigstore/fulcio/.github/workflows/foo.yaml@refs/heads/main", + "repository": "sigstore/fulcio", + "sha": "bf96275471e83ff04ce5c8eb515c04a75d43f854", + "sub": "repo:sigstore/fulcio:ref:refs/heads/main", + "workflow": "foo", + }, + WantErr: true, + ErrContains: "ref", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + token := &oidc.IDToken{ + Issuer: test.Claims["iss"].(string), + Subject: test.Claims["sub"].(string), + } + claims, err := json.Marshal(test.Claims) + if err != nil { + t.Fatal(err) + } + withClaims(token, claims) + + untyped, err := WorkflowPrincipalFromIDToken(context.TODO(), token) + if err != nil { + if !test.WantErr { + t.Fatal("didn't expect error", err) + } + if !strings.Contains(err.Error(), test.ErrContains) { + t.Fatalf("expected error %s to contain %s", err, test.ErrContains) + } + return + } + if err == nil && test.WantErr { + t.Fatal("expected error but got none") + } + + principal, ok := untyped.(*workflowPrincipal) + if !ok { + t.Errorf("Got wrong principal type %v", untyped) + } + if *principal != test.ExpectPrincipal { + t.Errorf("got %v principal and expected %v", *principal, test.ExpectPrincipal) + } + }) + } +} + +// reflect hack because "claims" field is unexported by oidc IDToken +// https://github.com/coreos/go-oidc/pull/329 +func withClaims(token *oidc.IDToken, data []byte) { + val := reflect.Indirect(reflect.ValueOf(token)) + member := val.FieldByName("claims") + pointer := unsafe.Pointer(member.UnsafeAddr()) + realPointer := (*[]byte)(pointer) + *realPointer = data +} + +func TestName(t *testing.T) { + tests := map[string]struct { + Claims map[string]interface{} + ExpectName string + }{ + `Valid token authenticates with correct claims`: { + Claims: map[string]interface{}{ + "aud": "sigstore", + "event_name": "push", + "exp": 0, + "iss": "https://token.actions.githubusercontent.com", + "job_workflow_ref": "sigstore/fulcio/.github/workflows/foo.yaml@refs/heads/main", + "ref": "refs/heads/main", + "repository": "sigstore/fulcio", + "sha": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "sub": "repo:sigstore/fulcio:ref:refs/heads/main", + "workflow": "foo", + }, + ExpectName: "repo:sigstore/fulcio:ref:refs/heads/main", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + token := &oidc.IDToken{ + Issuer: test.Claims["iss"].(string), + Subject: test.Claims["sub"].(string), + } + claims, err := json.Marshal(test.Claims) + if err != nil { + t.Fatal(err) + } + withClaims(token, claims) + + principal, err := WorkflowPrincipalFromIDToken(context.TODO(), token) + if err != nil { + t.Fatal(err) + } + + gotName := principal.Name(context.TODO()) + if gotName != test.ExpectName { + t.Error("name should match sub claim") + } + }) + } + +} + +func TestEmbed(t *testing.T) { + tests := map[string]struct { + Principal identity.Principal + WantErr bool + WantFacts map[string]func(x509.Certificate) error + }{ + `Github workflow challenge should have all Github workflow extensions and issuer set`: { + Principal: &workflowPrincipal{ + issuer: "https://token.actions.githubusercontent.com", + subject: "doesntmatter", + url: `https://github.com/foo/bar/`, + sha: "sha", + trigger: "trigger", + workflow: "workflowname", + repository: "repository", + ref: "ref", + }, + WantErr: false, + WantFacts: map[string]func(x509.Certificate) error{ + `Certifificate should have correct issuer`: factIssuerIs(`https://token.actions.githubusercontent.com`), + `Certificate has correct trigger extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 2}, "trigger"), + `Certificate has correct SHA extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 3}, "sha"), + `Certificate has correct workflow extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 4}, "workflowname"), + `Certificate has correct repository extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 5}, "repository"), + `Certificate has correct ref extension`: factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 6}, "ref"), + }, + }, + `Github workflow value with bad URL fails`: { + Principal: &workflowPrincipal{ + subject: "doesntmatter", + url: "\nbadurl", + sha: "sha", + trigger: "trigger", + workflow: "workflowname", + repository: "repository", + ref: "ref", + }, + WantErr: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + var cert x509.Certificate + err := test.Principal.Embed(context.TODO(), &cert) + if err != nil { + if !test.WantErr { + t.Error(err) + } + return + } else if test.WantErr { + t.Error("expected error") + } + for factName, fact := range test.WantFacts { + t.Run(factName, func(t *testing.T) { + if err := fact(cert); err != nil { + t.Error(err) + } + }) + } + }) + } +} + +func factIssuerIs(issuer string) func(x509.Certificate) error { + return factExtensionIs(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 1}, issuer) +} + +func factExtensionIs(oid asn1.ObjectIdentifier, value string) func(x509.Certificate) error { + return func(cert x509.Certificate) error { + for _, ext := range cert.ExtraExtensions { + if ext.Id.Equal(oid) { + if !bytes.Equal(ext.Value, []byte(value)) { + return fmt.Errorf("expected oid %v to be %s, but got %s", oid, value, ext.Value) + } + return nil + } + } + return errors.New("extension not set") + } +}