Expose SkipExpiryCheck OIDC Config Option in Verifier (#1271)

* Expose SkipExpiryCheck OIDC Config Option in Verifier Signed-off-by: Priya Wadhwa <[email protected]> * go mod lint Signed-off-by: Priya Wadhwa <[email protected]> --------- Signed-off-by: Priya Wadhwa <[email protected]>
sigstore · Jul 13, 2023 · 0972dcc · 0972dcc
1 parent c62f745
commit 0972dcc
Show file tree

Hide file tree

Showing 23 changed files with 183 additions and 48 deletions.
diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -24,6 +24,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"reflect"
 	"regexp"
 	"strings"
 	"time"
@@ -41,6 +42,11 @@ const defaultOIDCDiscoveryTimeout = 10 * time.Second
 // top-level and second-level domain
 const minimumHostnameLength = 2
 
+type verifierWithConfig struct {
+	*oidc.IDTokenVerifier
+	*oidc.Config
+}
+
 type FulcioConfig struct {
 	OIDCIssuers map[string]OIDCIssuer `json:"OIDCIssuers,omitempty"`
 
@@ -54,7 +60,7 @@ type FulcioConfig struct {
 	MetaIssuers map[string]OIDCIssuer `json:"MetaIssuers,omitempty"`
 
 	// verifiers is a fixed mapping from our OIDCIssuers to their OIDC verifiers.
-	verifiers map[string]*oidc.IDTokenVerifier
+	verifiers map[string][]*verifierWithConfig
 	// lru is an LRU cache of recently used verifiers for our meta issuers.
 	lru *lru.TwoQueueCache
 }
@@ -129,25 +135,38 @@ func (fc *FulcioConfig) GetIssuer(issuerURL string) (OIDCIssuer, bool) {
 // GetVerifier fetches a token verifier for the given `issuerURL`
 // coming from an incoming OIDC token.  If no matching configuration
 // is found, then it returns `false`.
-func (fc *FulcioConfig) GetVerifier(issuerURL string) (*oidc.IDTokenVerifier, bool) {
+func (fc *FulcioConfig) GetVerifier(issuerURL string, opts ...InsecureOIDCConfigOption) (*oidc.IDTokenVerifier, bool) {
+	iss, ok := fc.GetIssuer(issuerURL)
+	if !ok {
+		return nil, false
+	}
+	cfg := &oidc.Config{ClientID: iss.ClientID}
+	for _, o := range opts {
+		o(cfg)
+	}
 	// Look up our fixed issuer verifiers
 	v, ok := fc.verifiers[issuerURL]
 	if ok {
-		return v, true
+		for _, c := range v {
+			if reflect.DeepEqual(c.Config, cfg) {
+				return c.IDTokenVerifier, true
+			}
+		}
 	}
 
 	// Look in the LRU cache for a verifier
 	untyped, ok := fc.lru.Get(issuerURL)
 	if ok {
-		return untyped.(*oidc.IDTokenVerifier), true
+		v := untyped.([]*verifierWithConfig)
+		for _, c := range v {
+			if reflect.DeepEqual(c.Config, cfg) {
+				return c.IDTokenVerifier, true
+			}
+		}
 	}
-	// If this issuer hasn't been recently used, then create a new verifier
-	// and add it to the LRU cache.
 
-	iss, ok := fc.GetIssuer(issuerURL)
-	if !ok {
-		return nil, false
-	}
+	// If this issuer hasn't been recently used, or we have special config options, then create a new verifier
+	// and add it to the LRU cache.
 
 	ctx, cancel := context.WithTimeout(context.Background(), defaultOIDCDiscoveryTimeout)
 	defer cancel()
@@ -156,9 +175,24 @@ func (fc *FulcioConfig) GetVerifier(issuerURL string) (*oidc.IDTokenVerifier, bo
 		log.Logger.Warnf("Failed to create provider for issuer URL %q: %v", issuerURL, err)
 		return nil, false
 	}
-	verifier := provider.Verifier(&oidc.Config{ClientID: iss.ClientID})
-	fc.lru.Add(issuerURL, verifier)
-	return verifier, true
+
+	vwf := &verifierWithConfig{provider.Verifier(cfg), cfg}
+	if untyped == nil {
+		v = []*verifierWithConfig{vwf}
+	} else {
+		v = append(v, vwf)
+	}
+
+	fc.lru.Add(issuerURL, v)
+	return vwf.IDTokenVerifier, true
+}
+
+type InsecureOIDCConfigOption func(opt *oidc.Config)
+
+func WithSkipExpiryCheck() InsecureOIDCConfigOption {
+	return func(c *oidc.Config) {
+		c.SkipExpiryCheck = true
+	}
 }
 
 // ToIssuers returns a proto representation of the OIDC issuer configuration.
@@ -189,15 +223,16 @@ func (fc *FulcioConfig) ToIssuers() []*fulciogrpc.OIDCIssuer {
 }
 
 func (fc *FulcioConfig) prepare() error {
-	fc.verifiers = make(map[string]*oidc.IDTokenVerifier, len(fc.OIDCIssuers))
+	fc.verifiers = make(map[string][]*verifierWithConfig, len(fc.OIDCIssuers))
 	for _, iss := range fc.OIDCIssuers {
 		ctx, cancel := context.WithTimeout(context.Background(), defaultOIDCDiscoveryTimeout)
 		defer cancel()
 		provider, err := oidc.NewProvider(ctx, iss.IssuerURL)
 		if err != nil {
 			return fmt.Errorf("provider %s: %w", iss.IssuerURL, err)
 		}
-		fc.verifiers[iss.IssuerURL] = provider.Verifier(&oidc.Config{ClientID: iss.ClientID})
+		cfg := &oidc.Config{ClientID: iss.ClientID}
+		fc.verifiers[iss.IssuerURL] = []*verifierWithConfig{{provider.Verifier(cfg), cfg}}
 	}
 
 	cache, err := lru.New2Q(100 /* size */)

diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
@@ -16,11 +16,14 @@
 package config
 
 import (
+	"context"
 	"fmt"
 	"net/url"
 	"reflect"
 	"testing"
 
+	"github.com/coreos/go-oidc/v3/oidc"
+	lru "github.com/hashicorp/golang-lru"
 	"github.com/sigstore/fulcio/pkg/generated/protobuf"
 )
 
@@ -548,3 +551,78 @@ func TestToIssuers(t *testing.T) {
 		t.Fatalf("expected issuer %v, got %v", iss, issuers[1])
 	}
 }
+
+func TestVerifierCache(t *testing.T) {
+	cache, err := lru.New2Q(100 /* size */)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	fc := &FulcioConfig{
+		OIDCIssuers: map[string]OIDCIssuer{
+			"issuer.dev": {
+				IssuerURL: "issuer.dev",
+				ClientID:  "sigstore",
+			},
+		},
+		verifiers: map[string][]*verifierWithConfig{},
+		lru:       cache,
+	}
+
+	// create a cache hit
+	cfg := &oidc.Config{ClientID: "sigstore"}
+	verifier := oidc.NewVerifier("issuer.dev", &mockKeySet{}, cfg)
+	fc.verifiers = map[string][]*verifierWithConfig{
+		"issuer.dev": {
+			{
+				Config:          cfg,
+				IDTokenVerifier: verifier,
+			},
+		},
+	}
+
+	// make sure we get a hit
+	v, ok := fc.GetVerifier("issuer.dev")
+	if !ok {
+		t.Fatal("unable to verifier")
+	}
+	if !reflect.DeepEqual(v, verifier) {
+		t.Fatal("got unexpected verifier")
+	}
+
+	// get verifier with SkipExpiryCheck set, should fail on cache miss
+	_, ok = fc.GetVerifier("issuer.dev", WithSkipExpiryCheck())
+	if ok {
+		t.Fatal("expected cache miss")
+	}
+
+	// create a cache hit with SkipExpiryCheck set
+	withExpiryCfg := &oidc.Config{ClientID: "sigstore", SkipExpiryCheck: true}
+	expiryVerifier := oidc.NewVerifier("issuer.dev", &mockKeySet{}, cfg)
+	fc.verifiers = map[string][]*verifierWithConfig{
+		"issuer.dev": {
+			{
+				Config:          cfg,
+				IDTokenVerifier: verifier,
+			}, {
+				Config:          withExpiryCfg,
+				IDTokenVerifier: expiryVerifier,
+			},
+		},
+	}
+	// make sure we get a hit and the correct verifier is returned
+	v, ok = fc.GetVerifier("issuer.dev", WithSkipExpiryCheck())
+	if !ok {
+		t.Fatal("unable to verifier")
+	}
+	if !reflect.DeepEqual(v, expiryVerifier) {
+		t.Fatal("got unexpected verifier")
+	}
+}
+
+type mockKeySet struct {
+}
+
+func (m *mockKeySet) VerifySignature(_ context.Context, _ string) (payload []byte, err error) {
+	return nil, nil
+}
diff --git a/pkg/identity/authorize.go b/pkg/identity/authorize.go
@@ -25,13 +25,13 @@ import (
 // We do this to bypass needing actual OIDC tokens for unit testing.
 var Authorize = actualAuthorize
 
-func actualAuthorize(ctx context.Context, token string) (*oidc.IDToken, error) {
+func actualAuthorize(ctx context.Context, token string, opts ...config.InsecureOIDCConfigOption) (*oidc.IDToken, error) {
 	issuer, err := extractIssuerURL(token)
 	if err != nil {
 		return nil, err
 	}
 
-	verifier, ok := config.FromContext(ctx).GetVerifier(issuer)
+	verifier, ok := config.FromContext(ctx).GetVerifier(issuer, opts...)
 	if !ok {
 		return nil, fmt.Errorf("unsupported issuer: %s", issuer)
 	}

diff --git a/pkg/identity/base/issuer.go b/pkg/identity/base/issuer.go
@@ -21,6 +21,7 @@ import (
 	"strings"
 
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/sigstore/fulcio/pkg/config"
 	"github.com/sigstore/fulcio/pkg/identity"
 )
 
@@ -38,7 +39,7 @@ func Issuer(issuerURL string) identity.Issuer {
 }
 
 // This is unimplemented for the base issuer, and should be implemented unique to each issuer
-func (e *baseIssuer) Authenticate(_ context.Context, token string) (identity.Principal, error) { //nolint: revive
+func (e *baseIssuer) Authenticate(ctx context.Context, token string, opts ...config.InsecureOIDCConfigOption) (identity.Principal, error) { //nolint: revive
 	return nil, fmt.Errorf("unimplemented")
 }
 

diff --git a/pkg/identity/buildkite/issuer.go b/pkg/identity/buildkite/issuer.go
@@ -17,6 +17,7 @@ package buildkite
 import (
 	"context"
 
+	"github.com/sigstore/fulcio/pkg/config"
 	"github.com/sigstore/fulcio/pkg/identity"
 	"github.com/sigstore/fulcio/pkg/identity/base"
 )
@@ -29,8 +30,8 @@ func Issuer(issuerURL string) identity.Issuer {
 	return &buildkiteIssuer{base.Issuer(issuerURL)}
 }
 
-func (e *buildkiteIssuer) Authenticate(ctx context.Context, token string) (identity.Principal, error) {
-	idtoken, err := identity.Authorize(ctx, token)
+func (e *buildkiteIssuer) Authenticate(ctx context.Context, token string, opts ...config.InsecureOIDCConfigOption) (identity.Principal, error) {
+	idtoken, err := identity.Authorize(ctx, token, opts...)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/identity/buildkite/issuer_test.go b/pkg/identity/buildkite/issuer_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"github.com/coreos/go-oidc/v3/oidc"
+	"github.com/sigstore/fulcio/pkg/config"
 	"github.com/sigstore/fulcio/pkg/identity"
 )
 
@@ -56,7 +57,7 @@ func TestIssuer(t *testing.T) {
 		}
 		withClaims(token, claims)
 
-		identity.Authorize = func(_ context.Context, _ string) (*oidc.IDToken, error) {
+		identity.Authorize = func(_ context.Context, _ string, _ ...config.InsecureOIDCConfigOption) (*oidc.IDToken, error) {
 			return token, nil
 		}
 		principal, err := issuer.Authenticate(ctx, "token")

diff --git a/pkg/identity/email/issuer.go b/pkg/identity/email/issuer.go
@@ -17,6 +17,7 @@ package email
 import (
 	"context"
 
+	"github.com/sigstore/fulcio/pkg/config"
 	"github.com/sigstore/fulcio/pkg/identity"
 	"github.com/sigstore/fulcio/pkg/identity/base"
 )
@@ -29,8 +30,8 @@ func Issuer(issuerURL string) identity.Issuer {
 	return &emailIssuer{base.Issuer(issuerURL)}
 }
 
-func (e *emailIssuer) Authenticate(ctx context.Context, token string) (identity.Principal, error) {
-	idtoken, err := identity.Authorize(ctx, token)
+func (e *emailIssuer) Authenticate(ctx context.Context, token string, opts ...config.InsecureOIDCConfigOption) (identity.Principal, error) {
+	idtoken, err := identity.Authorize(ctx, token, opts...)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/identity/email/issuer_test.go b/pkg/identity/email/issuer_test.go
@@ -66,7 +66,7 @@ func TestIssuer(t *testing.T) {
 			},
 		})
 
-		identity.Authorize = func(_ context.Context, _ string) (*oidc.IDToken, error) {
+		identity.Authorize = func(_ context.Context, _ string, _ ...config.InsecureOIDCConfigOption) (*oidc.IDToken, error) {
 			return token, nil
 		}
 		principal, err := issuer.Authenticate(ctx, "token")

diff --git a/pkg/identity/github/issuer.go b/pkg/identity/github/issuer.go
@@ -16,7 +16,9 @@ package github
 
 import (
 	"context"
+	"fmt"
 
+	"github.com/sigstore/fulcio/pkg/config"
 	"github.com/sigstore/fulcio/pkg/identity"
 	"github.com/sigstore/fulcio/pkg/identity/base"
 )
@@ -29,10 +31,10 @@ func Issuer(issuerURL string) identity.Issuer {
 	return &githubIssuer{base.Issuer(issuerURL)}
 }
 
-func (e *githubIssuer) Authenticate(ctx context.Context, token string) (identity.Principal, error) {
-	idtoken, err := identity.Authorize(ctx, token)
+func (e *githubIssuer) Authenticate(ctx context.Context, token string, opts ...config.InsecureOIDCConfigOption) (identity.Principal, error) {
+	idtoken, err := identity.Authorize(ctx, token, opts...)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("authorizing github issuer: %w", err)
 	}
 	return WorkflowPrincipalFromIDToken(ctx, idtoken)
 }
diff --git a/pkg/identity/github/issuer_test.go b/pkg/identity/github/issuer_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"github.com/coreos/go-oidc/v3/oidc"
+	"github.com/sigstore/fulcio/pkg/config"
 	"github.com/sigstore/fulcio/pkg/identity"
 )
 
@@ -69,7 +70,7 @@ func TestIssuer(t *testing.T) {
 		}
 		withClaims(token, claims)
 
-		identity.Authorize = func(_ context.Context, _ string) (*oidc.IDToken, error) {
+		identity.Authorize = func(_ context.Context, _ string, _ ...config.InsecureOIDCConfigOption) (*oidc.IDToken, error) {
 			return token, nil
 		}
 		principal, err := issuer.Authenticate(ctx, "token")

diff --git a/pkg/identity/gitlabcom/issuer.go b/pkg/identity/gitlabcom/issuer.go
@@ -17,6 +17,7 @@ package gitlabcom
 import (
 	"context"
 
+	"github.com/sigstore/fulcio/pkg/config"
 	"github.com/sigstore/fulcio/pkg/identity"
 	"github.com/sigstore/fulcio/pkg/identity/base"
 )
@@ -29,8 +30,8 @@ func Issuer(issuerURL string) identity.Issuer {
 	return &gitlabIssuer{base.Issuer(issuerURL)}
 }
 
-func (e *gitlabIssuer) Authenticate(ctx context.Context, token string) (identity.Principal, error) {
-	idtoken, err := identity.Authorize(ctx, token)
+func (e *gitlabIssuer) Authenticate(ctx context.Context, token string, opts ...config.InsecureOIDCConfigOption) (identity.Principal, error) {
+	idtoken, err := identity.Authorize(ctx, token, opts...)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/identity/gitlabcom/issuer_test.go b/pkg/identity/gitlabcom/issuer_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"github.com/coreos/go-oidc/v3/oidc"
+	"github.com/sigstore/fulcio/pkg/config"
 	"github.com/sigstore/fulcio/pkg/identity"
 )
 
@@ -74,7 +75,7 @@ func TestIssuer(t *testing.T) {
 		}
 		withClaims(token, claims)
 
-		identity.Authorize = func(_ context.Context, _ string) (*oidc.IDToken, error) {
+		identity.Authorize = func(_ context.Context, _ string, _ ...config.InsecureOIDCConfigOption) (*oidc.IDToken, error) {
 			return token, nil
 		}
 		principal, err := issuer.Authenticate(ctx, "token")