From effad15ecc373beb46afd2915827247da51f399d Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Mon, 29 Oct 2018 20:45:10 -0700 Subject: [PATCH] patch webhook authenticator to support token review with arbitrary audiences --- pkg/kubeapiserver/authenticator/config.go | 6 +- .../authenticatorfactory/delegating.go | 4 +- .../authenticator/token/webhook/webhook.go | 52 ++++- .../token/webhook/webhook_test.go | 185 ++++++++++++++---- test/integration/auth/auth_test.go | 2 +- 5 files changed, 200 insertions(+), 49 deletions(-) diff --git a/pkg/kubeapiserver/authenticator/config.go b/pkg/kubeapiserver/authenticator/config.go index 359af1afb92..b34e6b8958a 100644 --- a/pkg/kubeapiserver/authenticator/config.go +++ b/pkg/kubeapiserver/authenticator/config.go @@ -179,7 +179,7 @@ func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, er tokenAuthenticators = append(tokenAuthenticators, oidcAuth) } if len(config.WebhookTokenAuthnConfigFile) > 0 { - webhookTokenAuth, err := newWebhookTokenAuthenticator(config.WebhookTokenAuthnConfigFile, config.WebhookTokenAuthnCacheTTL) + webhookTokenAuth, err := newWebhookTokenAuthenticator(config.WebhookTokenAuthnConfigFile, config.WebhookTokenAuthnCacheTTL, config.APIAudiences) if err != nil { return nil, nil, err } @@ -318,8 +318,8 @@ func newAuthenticatorFromClientCAFile(clientCAFile string) (authenticator.Reques return x509.New(opts, x509.CommonNameUserConversion), nil } -func newWebhookTokenAuthenticator(webhookConfigFile string, ttl time.Duration) (authenticator.Token, error) { - webhookTokenAuthenticator, err := webhook.New(webhookConfigFile) +func newWebhookTokenAuthenticator(webhookConfigFile string, ttl time.Duration, implicitAuds authenticator.Audiences) (authenticator.Token, error) { + webhookTokenAuthenticator, err := webhook.New(webhookConfigFile, implicitAuds) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/delegating.go b/staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/delegating.go index 74e583a2af4..67958c3639b 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/delegating.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory/delegating.go @@ -51,6 +51,8 @@ type DelegatingAuthenticatorConfig struct { // ClientCAFile is the CA bundle file used to authenticate client certificates ClientCAFile string + APIAudiences authenticator.Audiences + RequestHeaderConfig *RequestHeaderConfig } @@ -86,7 +88,7 @@ func (c DelegatingAuthenticatorConfig) New() (authenticator.Request, *spec.Secur } if c.TokenAccessReviewClient != nil { - tokenAuth, err := webhooktoken.NewFromInterface(c.TokenAccessReviewClient) + tokenAuth, err := webhooktoken.NewFromInterface(c.TokenAccessReviewClient, c.APIAudiences) if err != nil { return nil, nil, err } diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook.go index 3f1e1f92ed7..cf0a83b5d97 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook.go @@ -21,8 +21,6 @@ import ( "context" "time" - "k8s.io/klog" - authentication "k8s.io/api/authentication/v1beta1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -31,6 +29,7 @@ import ( "k8s.io/apiserver/pkg/util/webhook" "k8s.io/client-go/kubernetes/scheme" authenticationclient "k8s.io/client-go/kubernetes/typed/authentication/v1beta1" + "k8s.io/klog" ) var ( @@ -45,38 +44,58 @@ var _ authenticator.Token = (*WebhookTokenAuthenticator)(nil) type WebhookTokenAuthenticator struct { tokenReview authenticationclient.TokenReviewInterface initialBackoff time.Duration + implicitAuds authenticator.Audiences } // NewFromInterface creates a webhook authenticator using the given tokenReview // client. It is recommend to wrap this authenticator with the token cache // authenticator implemented in // k8s.io/apiserver/pkg/authentication/token/cache. -func NewFromInterface(tokenReview authenticationclient.TokenReviewInterface) (*WebhookTokenAuthenticator, error) { - return newWithBackoff(tokenReview, retryBackoff) +func NewFromInterface(tokenReview authenticationclient.TokenReviewInterface, implicitAuds authenticator.Audiences) (*WebhookTokenAuthenticator, error) { + return newWithBackoff(tokenReview, retryBackoff, implicitAuds) } -// New creates a new WebhookTokenAuthenticator from the provided kubeconfig file. -func New(kubeConfigFile string) (*WebhookTokenAuthenticator, error) { +// New creates a new WebhookTokenAuthenticator from the provided kubeconfig +// file. It is recommend to wrap this authenticator with the token cache +// authenticator implemented in +// k8s.io/apiserver/pkg/authentication/token/cache. +func New(kubeConfigFile string, implicitAuds authenticator.Audiences) (*WebhookTokenAuthenticator, error) { tokenReview, err := tokenReviewInterfaceFromKubeconfig(kubeConfigFile) if err != nil { return nil, err } - return newWithBackoff(tokenReview, retryBackoff) + return newWithBackoff(tokenReview, retryBackoff, implicitAuds) } // newWithBackoff allows tests to skip the sleep. -func newWithBackoff(tokenReview authenticationclient.TokenReviewInterface, initialBackoff time.Duration) (*WebhookTokenAuthenticator, error) { - return &WebhookTokenAuthenticator{tokenReview, initialBackoff}, nil +func newWithBackoff(tokenReview authenticationclient.TokenReviewInterface, initialBackoff time.Duration, implicitAuds authenticator.Audiences) (*WebhookTokenAuthenticator, error) { + return &WebhookTokenAuthenticator{tokenReview, initialBackoff, implicitAuds}, nil } // AuthenticateToken implements the authenticator.Token interface. func (w *WebhookTokenAuthenticator) AuthenticateToken(ctx context.Context, token string) (*authenticator.Response, bool, error) { + // We take implicit audiences of the API server at WebhookTokenAuthenticator + // construction time. The outline of how we validate audience here is: + // + // * if the ctx is not audience limited, don't do any audience validation. + // * if ctx is audience-limited, add the audiences to the tokenreview spec + // * if the tokenreview returns with audiences in the status that intersect + // with the audiences in the ctx, copy into the response and return success + // * if the tokenreview returns without an audience in the status, ensure + // the ctx audiences intersect with the implicit audiences, and set the + // intersection in the response. + // * otherwise return unauthenticated. + wantAuds, checkAuds := authenticator.AudiencesFrom(ctx) r := &authentication.TokenReview{ - Spec: authentication.TokenReviewSpec{Token: token}, + Spec: authentication.TokenReviewSpec{ + Token: token, + Audiences: wantAuds, + }, } var ( result *authentication.TokenReview err error + auds authenticator.Audiences ) webhook.WithExponentialBackoff(w.initialBackoff, func() error { result, err = w.tokenReview.Create(r) @@ -87,6 +106,18 @@ func (w *WebhookTokenAuthenticator) AuthenticateToken(ctx context.Context, token klog.Errorf("Failed to make webhook authenticator request: %v", err) return nil, false, err } + + if checkAuds { + gotAuds := w.implicitAuds + if len(result.Status.Audiences) > 0 { + gotAuds = result.Status.Audiences + } + auds = wantAuds.Intersect(gotAuds) + if len(auds) == 0 { + return nil, false, nil + } + } + r.Status = result.Status if !r.Status.Authenticated { return nil, false, nil @@ -107,6 +138,7 @@ func (w *WebhookTokenAuthenticator) AuthenticateToken(ctx context.Context, token Groups: r.Status.User.Groups, Extra: extra, }, + Audiences: auds, }, true, nil } diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook_test.go index 2544e2429d1..8c8855a894d 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook_test.go @@ -39,6 +39,8 @@ import ( "k8s.io/client-go/tools/clientcmd/api/v1" ) +var apiAuds = authenticator.Audiences{"api"} + // Service mocks a remote authentication service. type Service interface { // Review looks at the TokenReviewSpec and provides an authentication @@ -105,6 +107,7 @@ func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error type status struct { Authenticated bool `json:"authenticated"` User userInfo `json:"user"` + Audiences []string `json:"audiences"` } var extra map[string][]string @@ -130,6 +133,7 @@ func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error Groups: review.Status.User.Groups, Extra: extra, }, + review.Status.Audiences, }, } w.Header().Set("Content-Type", "application/json") @@ -168,7 +172,7 @@ func (m *mockService) HTTPStatusCode() int { return m.statusCode } // newTokenAuthenticator creates a temporary kubeconfig file from the provided // arguments and attempts to load a new WebhookTokenAuthenticator from it. -func newTokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration) (authenticator.Token, error) { +func newTokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, implicitAuds authenticator.Audiences) (authenticator.Token, error) { tempfile, err := ioutil.TempFile("", "") if err != nil { return nil, err @@ -196,7 +200,7 @@ func newTokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte, c return nil, err } - authn, err := newWithBackoff(c, 0) + authn, err := newWithBackoff(c, 0, implicitAuds) if err != nil { return nil, err } @@ -257,7 +261,7 @@ func TestTLSConfig(t *testing.T) { } defer server.Close() - wh, err := newTokenAuthenticator(server.URL, tt.clientCert, tt.clientKey, tt.clientCA, 0) + wh, err := newTokenAuthenticator(server.URL, tt.clientCert, tt.clientKey, tt.clientCA, 0, nil) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -312,23 +316,21 @@ func TestWebhookTokenAuthenticator(t *testing.T) { } defer s.Close() - wh, err := newTokenAuthenticator(s.URL, clientCert, clientKey, caCert, 0) - if err != nil { - t.Fatal(err) - } - expTypeMeta := metav1.TypeMeta{ APIVersion: "authentication.k8s.io/v1beta1", Kind: "TokenReview", } tests := []struct { + description string + implicitAuds, reqAuds authenticator.Audiences serverResponse v1beta1.TokenReviewStatus expectedAuthenticated bool expectedUser *user.DefaultInfo + expectedAuds authenticator.Audiences }{ - // Successful response should pass through all user info. { + description: "successful response should pass through all user info.", serverResponse: v1beta1.TokenReviewStatus{ Authenticated: true, User: v1beta1.UserInfo{ @@ -341,6 +343,7 @@ func TestWebhookTokenAuthenticator(t *testing.T) { }, }, { + description: "successful response should pass through all user info.", serverResponse: v1beta1.TokenReviewStatus{ Authenticated: true, User: v1beta1.UserInfo{ @@ -358,8 +361,8 @@ func TestWebhookTokenAuthenticator(t *testing.T) { Extra: map[string][]string{"foo": {"bar", "baz"}}, }, }, - // Unauthenticated shouldn't even include extra provided info. { + description: "unauthenticated shouldn't even include extra provided info.", serverResponse: v1beta1.TokenReviewStatus{ Authenticated: false, User: v1beta1.UserInfo{ @@ -372,37 +375,151 @@ func TestWebhookTokenAuthenticator(t *testing.T) { expectedUser: nil, }, { + description: "unauthenticated shouldn't even include extra provided info.", serverResponse: v1beta1.TokenReviewStatus{ Authenticated: false, }, expectedAuthenticated: false, expectedUser: nil, }, + { + description: "good audience", + implicitAuds: apiAuds, + reqAuds: apiAuds, + serverResponse: v1beta1.TokenReviewStatus{ + Authenticated: true, + User: v1beta1.UserInfo{ + Username: "somebody", + }, + }, + expectedAuthenticated: true, + expectedUser: &user.DefaultInfo{ + Name: "somebody", + }, + expectedAuds: apiAuds, + }, + { + description: "good audience", + implicitAuds: append(apiAuds, "other"), + reqAuds: apiAuds, + serverResponse: v1beta1.TokenReviewStatus{ + Authenticated: true, + User: v1beta1.UserInfo{ + Username: "somebody", + }, + }, + expectedAuthenticated: true, + expectedUser: &user.DefaultInfo{ + Name: "somebody", + }, + expectedAuds: apiAuds, + }, + { + description: "bad audiences", + implicitAuds: apiAuds, + reqAuds: authenticator.Audiences{"other"}, + serverResponse: v1beta1.TokenReviewStatus{ + Authenticated: false, + }, + expectedAuthenticated: false, + }, + { + description: "bad audiences", + implicitAuds: apiAuds, + reqAuds: authenticator.Audiences{"other"}, + // webhook authenticator hasn't been upgraded to support audience. + serverResponse: v1beta1.TokenReviewStatus{ + Authenticated: true, + User: v1beta1.UserInfo{ + Username: "somebody", + }, + }, + expectedAuthenticated: false, + }, + { + description: "audience aware backend", + implicitAuds: apiAuds, + reqAuds: apiAuds, + serverResponse: v1beta1.TokenReviewStatus{ + Authenticated: true, + User: v1beta1.UserInfo{ + Username: "somebody", + }, + Audiences: []string(apiAuds), + }, + expectedAuthenticated: true, + expectedUser: &user.DefaultInfo{ + Name: "somebody", + }, + expectedAuds: apiAuds, + }, + { + description: "audience aware backend", + serverResponse: v1beta1.TokenReviewStatus{ + Authenticated: true, + User: v1beta1.UserInfo{ + Username: "somebody", + }, + Audiences: []string(apiAuds), + }, + expectedAuthenticated: true, + expectedUser: &user.DefaultInfo{ + Name: "somebody", + }, + }, + { + description: "audience aware backend", + implicitAuds: apiAuds, + reqAuds: apiAuds, + serverResponse: v1beta1.TokenReviewStatus{ + Authenticated: true, + User: v1beta1.UserInfo{ + Username: "somebody", + }, + Audiences: []string{"other"}, + }, + expectedAuthenticated: false, + }, } token := "my-s3cr3t-t0ken" - for i, tt := range tests { - serv.response = tt.serverResponse - resp, authenticated, err := wh.AuthenticateToken(context.Background(), token) - if err != nil { - t.Errorf("case %d: authentication failed: %v", i, err) - continue - } - if serv.lastRequest.Spec.Token != token { - t.Errorf("case %d: Server did not see correct token. Got %q, expected %q.", - i, serv.lastRequest.Spec.Token, token) - } - if !reflect.DeepEqual(serv.lastRequest.TypeMeta, expTypeMeta) { - t.Errorf("case %d: Server did not see correct TypeMeta. Got %v, expected %v", - i, serv.lastRequest.TypeMeta, expTypeMeta) - } - if authenticated != tt.expectedAuthenticated { - t.Errorf("case %d: Plugin returned incorrect authentication response. Got %t, expected %t.", - i, authenticated, tt.expectedAuthenticated) - } - if resp != nil && tt.expectedUser != nil && !reflect.DeepEqual(resp.User, tt.expectedUser) { - t.Errorf("case %d: Plugin returned incorrect user. Got %#v, expected %#v", - i, resp.User, tt.expectedUser) - } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + wh, err := newTokenAuthenticator(s.URL, clientCert, clientKey, caCert, 0, tt.implicitAuds) + if err != nil { + t.Fatal(err) + } + + ctx := context.Background() + if tt.reqAuds != nil { + ctx = authenticator.WithAudiences(ctx, tt.reqAuds) + } + + serv.response = tt.serverResponse + resp, authenticated, err := wh.AuthenticateToken(ctx, token) + if err != nil { + t.Fatalf("authentication failed: %v", err) + } + if serv.lastRequest.Spec.Token != token { + t.Errorf("Server did not see correct token. Got %q, expected %q.", + serv.lastRequest.Spec.Token, token) + } + if !reflect.DeepEqual(serv.lastRequest.TypeMeta, expTypeMeta) { + t.Errorf("Server did not see correct TypeMeta. Got %v, expected %v", + serv.lastRequest.TypeMeta, expTypeMeta) + } + if authenticated != tt.expectedAuthenticated { + t.Errorf("Plugin returned incorrect authentication response. Got %t, expected %t.", + authenticated, tt.expectedAuthenticated) + } + if resp != nil && tt.expectedUser != nil && !reflect.DeepEqual(resp.User, tt.expectedUser) { + t.Errorf("Plugin returned incorrect user. Got %#v, expected %#v", + resp.User, tt.expectedUser) + } + if resp != nil && tt.expectedAuds != nil && !reflect.DeepEqual(resp.Audiences, tt.expectedAuds) { + t.Errorf("Plugin returned incorrect audiences. Got %#v, expected %#v", + resp.Audiences, tt.expectedAuds) + } + }) } } @@ -440,7 +557,7 @@ func TestWebhookCacheAndRetry(t *testing.T) { defer s.Close() // Create an authenticator that caches successful responses "forever" (100 days). - wh, err := newTokenAuthenticator(s.URL, clientCert, clientKey, caCert, 2400*time.Hour) + wh, err := newTokenAuthenticator(s.URL, clientCert, clientKey, caCert, 2400*time.Hour, nil) if err != nil { t.Fatal(err) } diff --git a/test/integration/auth/auth_test.go b/test/integration/auth/auth_test.go index 89ece409cf8..364f1d264d9 100644 --- a/test/integration/auth/auth_test.go +++ b/test/integration/auth/auth_test.go @@ -85,7 +85,7 @@ func getTestWebhookTokenAuth(serverURL string) (authenticator.Request, error) { if err := json.NewEncoder(kubecfgFile).Encode(config); err != nil { return nil, err } - webhookTokenAuth, err := webhook.New(kubecfgFile.Name()) + webhookTokenAuth, err := webhook.New(kubecfgFile.Name(), nil) if err != nil { return nil, err }