Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expose SkipExpiryCheck OIDC Config Option in Verifier #1271

Merged
merged 2 commits into from
Jul 13, 2023
Merged

Expose SkipExpiryCheck OIDC Config Option in Verifier #1271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e0186a
Expose SkipExpiryCheck OIDC Config Option in Verifier
priyawadhwa Jul 11, 2023
a821199
go mod lint
priyawadhwa Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 50 additions & 15 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"net/http"
"net/url"
"os"
"reflect"
"regexp"
"strings"
"time"
Expand All @@ -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"`

Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a use case that needs to be handled? What's the case when you'd have a configuration that specifies both skipping the expiration and not skipping the expiration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yah, so our custom issuer does have this use case. Not sure if I can give more details though 😅

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()
Expand All @@ -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.
Expand Down Expand Up @@ -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 */)
Expand Down
78 changes: 78 additions & 0 deletions pkg/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions pkg/identity/authorize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/identity/base/issuer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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")
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/identity/buildkite/issuer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/identity/buildkite/issuer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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")
Expand Down
5 changes: 3 additions & 2 deletions pkg/identity/email/issuer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/identity/email/issuer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 5 additions & 3 deletions pkg/identity/github/issuer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
}
3 changes: 2 additions & 1 deletion pkg/identity/github/issuer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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")
Expand Down
5 changes: 3 additions & 2 deletions pkg/identity/gitlabcom/issuer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/identity/gitlabcom/issuer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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")
Expand Down