From 5dd4c89df38d4a5389c0cbf2c7fe4f6a5d5534ce Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Tue, 6 Apr 2021 11:04:05 -0400 Subject: [PATCH 1/2] oidc authenticator: allow passing in CA via bytes This change updates the OIDC authenticator code to use a subset of the dynamiccertificates.CAContentProvider interface to provide the root CA bytes. This removes the hard dependency on a file based CA and makes it easier to use this code as a library. Signed-off-by: Monis Khan --- pkg/kubeapiserver/authenticator/config.go | 12 +++++++++++- .../plugin/pkg/authenticator/token/oidc/oidc.go | 16 +++++++++++----- .../pkg/authenticator/token/oidc/oidc_test.go | 7 ++++++- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/pkg/kubeapiserver/authenticator/config.go b/pkg/kubeapiserver/authenticator/config.go index bc8033c04ca..ce4be0fda3b 100644 --- a/pkg/kubeapiserver/authenticator/config.go +++ b/pkg/kubeapiserver/authenticator/config.go @@ -151,10 +151,20 @@ func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, er // simply returns an error, the OpenID Connect plugin may query the provider to // update the keys, causing performance hits. if len(config.OIDCIssuerURL) > 0 && len(config.OIDCClientID) > 0 { + // TODO(enj): wire up the Notifier and ControllerRunner bits when OIDC supports CA reload + var oidcCAContent oidc.CAContentProvider + if len(config.OIDCCAFile) != 0 { + var oidcCAErr error + oidcCAContent, oidcCAErr = dynamiccertificates.NewDynamicCAContentFromFile("oidc-authenticator", config.OIDCCAFile) + if oidcCAErr != nil { + return nil, nil, oidcCAErr + } + } + oidcAuth, err := newAuthenticatorFromOIDCIssuerURL(oidc.Options{ IssuerURL: config.OIDCIssuerURL, ClientID: config.OIDCClientID, - CAFile: config.OIDCCAFile, + CAContentProvider: oidcCAContent, UsernameClaim: config.OIDCUsernameClaim, UsernamePrefix: config.OIDCUsernamePrefix, GroupsClaim: config.OIDCGroupsClaim, diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go index 8840b65f733..1fb5e70c679 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go @@ -78,8 +78,8 @@ type Options struct { // See: https://openid.net/specs/openid-connect-core-1_0.html#IDToken ClientID string - // Path to a PEM encoded root certificate of the provider. - CAFile string + // PEM encoded root certificate contents of the provider. + CAContentProvider CAContentProvider // UsernameClaim is the JWT field to use as the user's username. UsernameClaim string @@ -116,6 +116,11 @@ type Options struct { now func() time.Time } +// Subset of dynamiccertificates.CAContentProvider that can be used to dynamically load root CAs. +type CAContentProvider interface { + CurrentCABundleContent() []byte +} + // initVerifier creates a new ID token verifier for the given configuration and issuer URL. On success, calls setVerifier with the // resulting verifier. func initVerifier(ctx context.Context, config *oidc.Config, iss string) (*oidc.IDTokenVerifier, error) { @@ -273,10 +278,11 @@ func newAuthenticator(opts Options, initVerifier func(ctx context.Context, a *Au } var roots *x509.CertPool - if opts.CAFile != "" { - roots, err = certutil.NewPool(opts.CAFile) + if opts.CAContentProvider != nil { + // TODO(enj): make this reload CA data dynamically + roots, err = certutil.NewPoolFromBytes(opts.CAContentProvider.CurrentCABundleContent()) if err != nil { - return nil, fmt.Errorf("Failed to read the CA file: %v", err) + return nil, fmt.Errorf("Failed to read the CA contents: %v", err) } } else { klog.Info("OIDC: No x509 certificates provided, will use host's root CA set") diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go index 9841225df49..93189d0ee63 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go @@ -38,6 +38,7 @@ import ( oidc "github.com/coreos/go-oidc" jose "gopkg.in/square/go-jose.v2" "k8s.io/apiserver/pkg/authentication/user" + "k8s.io/apiserver/pkg/server/dynamiccertificates" "k8s.io/klog/v2" ) @@ -254,7 +255,11 @@ func (c *claimsTest) run(t *testing.T) { // by writing its root CA certificate into a temporary file. tempFileName := writeTempCert(t, ts.TLS.Certificates[0].Certificate[0]) defer os.Remove(tempFileName) - c.options.CAFile = tempFileName + caContent, err := dynamiccertificates.NewDynamicCAContentFromFile("oidc-authenticator", tempFileName) + if err != nil { + t.Fatalf("initialize ca: %v", err) + } + c.options.CAContentProvider = caContent // Allow claims to refer to the serving URL of the test server. For this, // substitute all references to {{.URL}} in appropriate places. From b5a1a45d48b4e90e54f512fc829b2ab9866b282e Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Tue, 6 Apr 2021 12:20:57 -0400 Subject: [PATCH 2/2] oidc authenticator: allow specifying a KeySet directly This change updates the oidc authenticator to allow specifying an oidc.KeySet as an input option. This makes it possible to synchronously initialize the KeySet instead of relying on the asynchronous initialization that is normally done to support self-hosted providers. This makes it easier to use this code as a library. Signed-off-by: Monis Khan --- .../pkg/authenticator/token/oidc/oidc.go | 43 ++++++++++--------- .../pkg/authenticator/token/oidc/oidc_test.go | 13 ++---- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go index 1fb5e70c679..2027c0ae4dc 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go @@ -70,6 +70,9 @@ type Options struct { // See: https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig IssuerURL string + // Optional KeySet to allow for synchronous initlization instead of fetching from the remote issuer. + KeySet oidc.KeySet + // ClientID the JWT must be issued for, the "sub" field. This plugin only trusts a single // client to ensure the plugin can be used with public providers. // @@ -219,24 +222,6 @@ func (a *Authenticator) Close() { a.cancel() } -func New(opts Options) (*Authenticator, error) { - return newAuthenticator(opts, func(ctx context.Context, a *Authenticator, config *oidc.Config) { - // Asynchronously attempt to initialize the authenticator. This enables - // self-hosted providers, providers that run on top of Kubernetes itself. - go wait.PollImmediateUntil(time.Second*10, func() (done bool, err error) { - provider, err := oidc.NewProvider(ctx, a.issuerURL) - if err != nil { - klog.Errorf("oidc authenticator: initializing plugin: %v", err) - return false, nil - } - - verifier := provider.Verifier(config) - a.setVerifier(verifier) - return true, nil - }, ctx.Done()) - }) -} - // whitelist of signing algorithms to ensure users don't mistakenly pass something // goofy. var allowedSigningAlgs = map[string]bool{ @@ -251,7 +236,7 @@ var allowedSigningAlgs = map[string]bool{ oidc.PS512: true, } -func newAuthenticator(opts Options, initVerifier func(ctx context.Context, a *Authenticator, config *oidc.Config)) (*Authenticator, error) { +func New(opts Options) (*Authenticator, error) { url, err := url.Parse(opts.IssuerURL) if err != nil { return nil, err @@ -327,7 +312,25 @@ func newAuthenticator(opts Options, initVerifier func(ctx context.Context, a *Au resolver: resolver, } - initVerifier(ctx, authenticator, verifierConfig) + if opts.KeySet != nil { + // We already have a key set, synchronously initialize the verifier. + authenticator.setVerifier(oidc.NewVerifier(opts.IssuerURL, opts.KeySet, verifierConfig)) + } else { + // Asynchronously attempt to initialize the authenticator. This enables + // self-hosted providers, providers that run on top of Kubernetes itself. + go wait.PollImmediateUntil(10*time.Second, func() (done bool, err error) { + provider, err := oidc.NewProvider(ctx, opts.IssuerURL) + if err != nil { + klog.Errorf("oidc authenticator: initializing plugin: %v", err) + return false, nil + } + + verifier := provider.Verifier(verifierConfig) + authenticator.setVerifier(verifier) + return true, nil + }, ctx.Done()) + } + return authenticator, nil } diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go index 93189d0ee63..23d851a959e 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go @@ -35,7 +35,6 @@ import ( "text/template" "time" - oidc "github.com/coreos/go-oidc" jose "gopkg.in/square/go-jose.v2" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/server/dynamiccertificates" @@ -271,15 +270,11 @@ func (c *claimsTest) run(t *testing.T) { c.claimToResponseMap[claim] = replace(response, &v) } + // Set the verifier to use the public key set instead of reading from a remote. + c.options.KeySet = &staticKeySet{keys: c.pubKeys} + // Initialize the authenticator. - a, err := newAuthenticator(c.options, func(ctx context.Context, a *Authenticator, config *oidc.Config) { - // Set the verifier to use the public key set instead of reading from a remote. - a.setVerifier(oidc.NewVerifier( - c.options.IssuerURL, - &staticKeySet{keys: c.pubKeys}, - config, - )) - }) + a, err := New(c.options) if err != nil { if !c.wantInitErr { t.Fatalf("initialize authenticator: %v", err)