diff --git a/pkg/challenges/challenges.go b/pkg/challenges/challenges.go index b1a23fdc6..8fa1a42d3 100644 --- a/pkg/challenges/challenges.go +++ b/pkg/challenges/challenges.go @@ -28,7 +28,7 @@ import ( "github.com/sigstore/fulcio/pkg/ca/x509ca" "github.com/sigstore/fulcio/pkg/config" "github.com/sigstore/fulcio/pkg/identity" - "github.com/spiffe/go-spiffe/v2/spiffeid" + "github.com/sigstore/fulcio/pkg/identity/spiffe" "github.com/coreos/go-oidc/v3/oidc" "github.com/sigstore/fulcio/pkg/oauthflow" @@ -40,7 +40,6 @@ type ChallengeType int const ( EmailValue ChallengeType = iota - SpiffeValue GithubWorkflowValue KubernetesValue URIValue @@ -82,12 +81,6 @@ func (cr *ChallengeResult) Embed(ctx context.Context, cert *x509.Certificate) er switch cr.TypeVal { case EmailValue: cert.EmailAddresses = []string{cr.Value} - case SpiffeValue: - challengeURL, err := url.Parse(cr.Value) - if err != nil { - return err - } - cert.URIs = []*url.URL{challengeURL} case GithubWorkflowValue: jobWorkflowURI, err := url.Parse(cr.Value) if err != nil { @@ -183,42 +176,6 @@ func email(ctx context.Context, principal *oidc.IDToken) (identity.Principal, er }, nil } -func spiffe(ctx context.Context, principal *oidc.IDToken) (identity.Principal, error) { - spiffeID := principal.Subject - - cfg, ok := config.FromContext(ctx).GetIssuer(principal.Issuer) - if !ok { - return nil, errors.New("invalid configuration for OIDC ID Token issuer") - } - - trustDomain, err := spiffeid.TrustDomainFromString(cfg.SPIFFETrustDomain) - if err != nil { - return nil, fmt.Errorf("unable to parse trust domain from configuration: %s", cfg.SPIFFETrustDomain) - } - - parsedSpiffeID, err := spiffeid.FromString(spiffeID) - if err != nil { - return nil, fmt.Errorf("invalid spiffe ID provided: %s", spiffeID) - } - - if parsedSpiffeID.TrustDomain().Compare(trustDomain) != 0 { - return nil, fmt.Errorf("spiffe ID trust domain %s doesn't match configured trust domain %s", parsedSpiffeID.TrustDomain(), trustDomain) - } - - issuer, err := oauthflow.IssuerFromIDToken(principal, cfg.IssuerClaim) - if err != nil { - return nil, err - } - - // Now issue cert! - return &ChallengeResult{ - Issuer: issuer, - TypeVal: SpiffeValue, - Value: spiffeID, - subject: spiffeID, - }, nil -} - func kubernetes(ctx context.Context, principal *oidc.IDToken) (identity.Principal, error) { k8sURI, err := kubernetesToken(principal) if err != nil { @@ -420,7 +377,7 @@ func PrincipalFromIDToken(ctx context.Context, tok *oidc.IDToken) (identity.Prin case config.IssuerTypeEmail: principal, err = email(ctx, tok) case config.IssuerTypeSpiffe: - principal, err = spiffe(ctx, tok) + principal, err = spiffe.PrincipalFromIDToken(ctx, tok) case config.IssuerTypeGithubWorkflow: principal, err = githubWorkflow(ctx, tok) case config.IssuerTypeKubernetes: diff --git a/pkg/challenges/challenges_test.go b/pkg/challenges/challenges_test.go index 369f42520..45b4339af 100644 --- a/pkg/challenges/challenges_test.go +++ b/pkg/challenges/challenges_test.go @@ -103,38 +103,6 @@ func TestEmbedChallengeResult(t *testing.T) { `Certificate should have issuer extension set`: factIssuerIs("example.com"), }, }, - `Good spiffe challenge`: { - Challenge: ChallengeResult{ - Issuer: `example.com`, - TypeVal: SpiffeValue, - Value: `spiffe://example.com/foo/bar`, - }, - WantErr: false, - WantFacts: map[string]func(x509.Certificate) error{ - `Issuer is example.com`: factIssuerIs(`example.com`), - `SAN is spiffe://example.com/foo/bar`: func(cert x509.Certificate) error { - WantURI, err := url.Parse("spiffe://example.com/foo/bar") - if err != nil { - return err - } - if len(cert.URIs) != 1 { - return errors.New("no URI SAN set") - } - if diff := cmp.Diff(cert.URIs[0], WantURI); diff != "" { - return errors.New(diff) - } - return nil - }, - }, - }, - `Spiffe value with bad URL fails`: { - Challenge: ChallengeResult{ - Issuer: `example.com`, - TypeVal: SpiffeValue, - Value: "\nbadurl", - }, - WantErr: true, - }, `Good Kubernetes value`: { Challenge: ChallengeResult{ Issuer: `k8s.example.com`, @@ -222,8 +190,8 @@ func TestEmbedChallengeResult(t *testing.T) { `No issuer should fail to render extensions`: { Challenge: ChallengeResult{ Issuer: ``, - TypeVal: SpiffeValue, - Value: "spiffe://foo.example.com/foo/bar", + TypeVal: URIValue, + Value: "https://foo.example.com/foo/bar", }, WantErr: true, }, @@ -436,113 +404,6 @@ func TestEmailWithClaims(t *testing.T) { } } -func TestSpiffe(t *testing.T) { - tests := map[string]struct { - Token *oidc.IDToken - Config *config.FulcioConfig - WantErr bool - }{ - "good token": { - Token: &oidc.IDToken{ - Subject: "spiffe://foo.com/bar", - Issuer: "id.foo.com", - }, - Config: &config.FulcioConfig{ - OIDCIssuers: map[string]config.OIDCIssuer{ - "id.foo.com": { - IssuerURL: "id.foo.com", - ClientID: "sigstore", - Type: config.IssuerTypeSpiffe, - SPIFFETrustDomain: "foo.com", - }, - }, - }, - WantErr: false, - }, - "spiffe id wrong trust domain": { - Token: &oidc.IDToken{ - Subject: "spiffe://baz.com/bar", - Issuer: "id.foo.com", - }, - Config: &config.FulcioConfig{ - OIDCIssuers: map[string]config.OIDCIssuer{ - "id.foo.com": { - IssuerURL: "id.foo.com", - ClientID: "sigstore", - Type: config.IssuerTypeSpiffe, - SPIFFETrustDomain: "foo.com", - }, - }, - }, - WantErr: true, - }, - "spiffe id no issuer configured": { - Token: &oidc.IDToken{ - Subject: "spiffe://foo.com/bar", - Issuer: "id.foo.com", - }, - Config: &config.FulcioConfig{ - OIDCIssuers: map[string]config.OIDCIssuer{ - "id.bar.com": { - IssuerURL: "id.bar.com", - ClientID: "sigstore", - Type: config.IssuerTypeSpiffe, - SPIFFETrustDomain: "foo.com", - }, - }, - }, - WantErr: true, - }, - "invalid spiffe id": { - Token: &oidc.IDToken{ - Subject: "spiffe://foo#com/bar", - Issuer: "id.foo.com", - }, - Config: &config.FulcioConfig{ - OIDCIssuers: map[string]config.OIDCIssuer{ - "id.foo.com": { - IssuerURL: "id.foo.com", - ClientID: "sigstore", - Type: config.IssuerTypeSpiffe, - SPIFFETrustDomain: "foo.com", - }, - }, - }, - WantErr: true, - }, - "invalid configured trust domain": { - Token: &oidc.IDToken{ - Subject: "spiffe://foo.com/bar", - Issuer: "id.foo.com", - }, - Config: &config.FulcioConfig{ - OIDCIssuers: map[string]config.OIDCIssuer{ - "id.foo.com": { - IssuerURL: "id.foo.com", - ClientID: "sigstore", - Type: config.IssuerTypeSpiffe, - SPIFFETrustDomain: "foo#com", - }, - }, - }, - WantErr: true, - }, - } - - for name, test := range tests { - t.Run(name, func(t *testing.T) { - ctx := config.With(context.Background(), test.Config) - _, err := spiffe(ctx, test.Token) - if err != nil && !test.WantErr { - t.Errorf("%s: %v", name, err) - } - if err == nil && test.WantErr { - t.Errorf("%s: expected error", name) - } - }) - } -} - func failErr(t *testing.T, err error) { if err != nil { t.Fatal(err) diff --git a/pkg/identity/spiffe/principal.go b/pkg/identity/spiffe/principal.go new file mode 100644 index 000000000..f98a59d22 --- /dev/null +++ b/pkg/identity/spiffe/principal.go @@ -0,0 +1,94 @@ +// Copyright 2021 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 spiffe + +import ( + "context" + "crypto/x509" + "errors" + "fmt" + "net/url" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/sigstore/fulcio/pkg/ca/x509ca" + "github.com/sigstore/fulcio/pkg/config" + "github.com/sigstore/fulcio/pkg/identity" + "github.com/spiffe/go-spiffe/v2/spiffeid" +) + +type principal struct { + // spiffe ID + id string + + // OIDC issuer url + issuer string +} + +func PrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (identity.Principal, error) { + cfg, ok := config.FromContext(ctx).GetIssuer(token.Issuer) + if !ok { + return nil, errors.New("invalid configuration for OIDC ID Token issuer") + } + + if err := validSpiffeID(token.Subject, cfg.SPIFFETrustDomain); err != nil { + return nil, err + } + + // Now issue cert! + return principal{ + id: token.Subject, + issuer: token.Issuer, + }, nil + +} + +func validSpiffeID(id, trustDomain string) error { + parsedTrustDomain, err := spiffeid.TrustDomainFromString(trustDomain) + if err != nil { + return fmt.Errorf("unable to parse trust domain from configuration %s: %w", trustDomain, err) + } + + parsedID, err := spiffeid.FromString(id) + if err != nil { + return fmt.Errorf("invalid spiffe ID provided: %s", id) + } + + if parsedID.TrustDomain().Compare(parsedTrustDomain) != 0 { + return fmt.Errorf("spiffe ID trust domain %s doesn't match configured trust domain %s", parsedID.TrustDomain(), trustDomain) + } + + return nil +} + +func (p principal) Name(context.Context) string { + return p.id +} + +func (p principal) Embed(ctx context.Context, cert *x509.Certificate) error { + parsed, err := url.Parse(p.id) + if err != nil { + return err + } + cert.URIs = []*url.URL{parsed} + + cert.ExtraExtensions, err = x509ca.Extensions{ + Issuer: p.issuer, + }.Render() + if err != nil { + return err + } + + return nil +} diff --git a/pkg/identity/spiffe/principal_test.go b/pkg/identity/spiffe/principal_test.go new file mode 100644 index 000000000..1a64aa8b3 --- /dev/null +++ b/pkg/identity/spiffe/principal_test.go @@ -0,0 +1,261 @@ +// Copyright 2021 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 spiffe + +import ( + "bytes" + "context" + "crypto/x509" + "encoding/asn1" + "errors" + "fmt" + "net/url" + "testing" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/google/go-cmp/cmp" + "github.com/sigstore/fulcio/pkg/config" +) + +func TestPrincipalFromIDToken(t *testing.T) { + tests := map[string]struct { + Token *oidc.IDToken + Principal principal + WantErr bool + }{ + `Valid token authenticates with correct claims`: { + Token: &oidc.IDToken{Issuer: "https://issuer.example.com", Subject: "spiffe://example.com/foo/bar"}, + Principal: principal{ + issuer: "https://issuer.example.com", + id: "spiffe://example.com/foo/bar", + }, + WantErr: false, + }, + `Issuer URL mismatch should error`: { + Token: &oidc.IDToken{Issuer: "https://foo.example.com", Subject: "spiffe://example.com/foo/bar"}, + WantErr: true, + }, + `Incorrect trust domain should error`: { + Token: &oidc.IDToken{Issuer: "https://issuer.example.com", Subject: "spiffe://foo.example.com/foo/bar"}, + WantErr: true, + }, + `Invalid ID should error`: { + Token: &oidc.IDToken{Issuer: "https://issuer.example.com", Subject: "not-a-spiffe-id"}, + WantErr: true, + }, + } + + cfg := &config.FulcioConfig{ + OIDCIssuers: map[string]config.OIDCIssuer{ + "https://issuer.example.com": { + IssuerURL: "https://issuer.example.com", + ClientID: "sigstore", + Type: "spiffe", + SPIFFETrustDomain: "example.com", + }, + }, + } + ctx := config.With(context.Background(), cfg) + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + untyped, err := PrincipalFromIDToken(ctx, test.Token) + if err != nil { + if !test.WantErr { + t.Fatal("didn't expect error", err) + } + return + } + if err == nil && test.WantErr { + t.Fatal("expected error but got none") + } + + p, ok := untyped.(principal) + if !ok { + t.Errorf("Got wrong principal type %v", untyped) + } + if p != test.Principal { + t.Errorf("got %v principal and expected %v", p, test.Principal) + } + }) + } +} + +func TestName(t *testing.T) { + tests := map[string]struct { + Token *oidc.IDToken + ExpectName string + }{ + `Valid token authenticates with correct claims`: { + Token: &oidc.IDToken{Issuer: "https://issuer.example.com", Subject: "spiffe://example.com/foo/bar"}, + ExpectName: "spiffe://example.com/foo/bar", + }, + } + + cfg := &config.FulcioConfig{ + OIDCIssuers: map[string]config.OIDCIssuer{ + "https://issuer.example.com": { + IssuerURL: "https://issuer.example.com", + ClientID: "sigstore", + Type: "spiffe", + SPIFFETrustDomain: "example.com", + }, + }, + } + ctx := config.With(context.Background(), cfg) + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + untyped, err := PrincipalFromIDToken(ctx, test.Token) + if err != nil { + t.Fatal(err) + } + + if gotName := untyped.Name(ctx); gotName != test.ExpectName { + t.Errorf("got %s and expected %s", gotName, test.ExpectName) + } + }) + } +} + +func TestEmbed(t *testing.T) { + tests := map[string]struct { + Principal principal + WantErr bool + WantFacts map[string]func(x509.Certificate) error + }{ + `Good spiffe challenge`: { + Principal: principal{ + issuer: `example.com`, + id: `spiffe://example.com/foo/bar`, + }, + WantErr: false, + WantFacts: map[string]func(x509.Certificate) error{ + `Issuer is example.com`: factIssuerIs(`example.com`), + `SAN is spiffe://example.com/foo/bar`: func(cert x509.Certificate) error { + WantURI, err := url.Parse("spiffe://example.com/foo/bar") + if err != nil { + return err + } + if len(cert.URIs) != 1 { + return errors.New("no URI SAN set") + } + if diff := cmp.Diff(cert.URIs[0], WantURI); diff != "" { + return errors.New(diff) + } + return nil + }, + }, + }, + `Spiffe value with bad URL fails`: { + Principal: principal{ + issuer: `example.com`, + id: "\nbadurl", + }, + WantErr: true, + }, + `Empty issuer url should fail to render extensions`: { + Principal: principal{ + issuer: "", + id: "spiffe://example.com/foo/bar", + }, + 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") + } +} + +func TestValidSpiffeID(t *testing.T) { + tests := map[string]struct { + ID string + TrustDomain string + WantErr bool + }{ + `Valid ID with matching trust domain results in no error`: { + ID: `spiffe://foo.com/bar`, + TrustDomain: `foo.com`, + WantErr: false, + }, + `Bad trust domain errors`: { + ID: `spiffe://foo.com/bar`, + TrustDomain: `not#a#trust#domain`, + WantErr: true, + }, + `Trust domain mismatch should error`: { + ID: `spiffe://foo.com/bar`, + TrustDomain: `bar.com`, + WantErr: true, + }, + `Bad spiffe id should error`: { + ID: `not#a#spiffe#id`, + TrustDomain: `bar.com`, + WantErr: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + err := validSpiffeID(test.ID, test.TrustDomain) + if err != nil { + if !test.WantErr { + t.Error("unepected error", err) + } + return + } + if err == nil && test.WantErr { + t.Error("expected err") + } + }) + } +}