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

Allow webhook authorizer to use SubjectAccessReviewInterface #34071

Merged
merged 1 commit into from
Oct 12, 2016
Merged

Allow webhook authorizer to use SubjectAccessReviewInterface #34071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
97 changes: 63 additions & 34 deletions plugin/pkg/auth/authorizer/webhook/webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ package webhook

import (
"encoding/json"
"fmt"
"time"

"github.com/golang/glog"

"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/apis/authorization"
"k8s.io/kubernetes/pkg/apis/authorization/v1beta1"
"k8s.io/kubernetes/pkg/auth/authorizer"
"k8s.io/kubernetes/pkg/client/restclient"
authorizationclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authorization/unversioned"
"k8s.io/kubernetes/pkg/util/cache"
"k8s.io/kubernetes/plugin/pkg/webhook"

Expand All @@ -44,10 +44,16 @@ const retryBackoff = 500 * time.Millisecond
var _ authorizer.Authorizer = (*WebhookAuthorizer)(nil)

type WebhookAuthorizer struct {
*webhook.GenericWebhook
responseCache *cache.LRUExpireCache
authorizedTTL time.Duration
unauthorizedTTL time.Duration
subjectAccessReview authorizationclient.SubjectAccessReviewInterface
responseCache *cache.LRUExpireCache
authorizedTTL time.Duration
unauthorizedTTL time.Duration
initialBackoff time.Duration
}

// NewFromInterface creates a WebhookAuthorizer using the given subjectAccessReview client
func NewFromInterface(subjectAccessReview authorizationclient.SubjectAccessReviewInterface, authorizedTTL, unauthorizedTTL time.Duration) (*WebhookAuthorizer, error) {
return newWithBackoff(subjectAccessReview, authorizedTTL, unauthorizedTTL, retryBackoff)
}

// New creates a new WebhookAuthorizer from the provided kubeconfig file.
Expand All @@ -71,16 +77,22 @@ type WebhookAuthorizer struct {
// For additional HTTP configuration, refer to the kubeconfig documentation
// http://kubernetes.io/v1.1/docs/user-guide/kubeconfig-file.html.
func New(kubeConfigFile string, authorizedTTL, unauthorizedTTL time.Duration) (*WebhookAuthorizer, error) {
return newWithBackoff(kubeConfigFile, authorizedTTL, unauthorizedTTL, retryBackoff)
}

// newWithBackoff allows tests to skip the sleep.
func newWithBackoff(kubeConfigFile string, authorizedTTL, unauthorizedTTL, initialBackoff time.Duration) (*WebhookAuthorizer, error) {
gw, err := webhook.NewGenericWebhook(kubeConfigFile, groupVersions, initialBackoff)
subjectAccessReview, err := subjectAccessReviewInterfaceFromKubeconfig(kubeConfigFile)
if err != nil {
return nil, err
}
return &WebhookAuthorizer{gw, cache.NewLRUExpireCache(1024), authorizedTTL, unauthorizedTTL}, nil
return newWithBackoff(subjectAccessReview, authorizedTTL, unauthorizedTTL, retryBackoff)
}

// newWithBackoff allows tests to skip the sleep.
func newWithBackoff(subjectAccessReview authorizationclient.SubjectAccessReviewInterface, authorizedTTL, unauthorizedTTL, initialBackoff time.Duration) (*WebhookAuthorizer, error) {
return &WebhookAuthorizer{
subjectAccessReview: subjectAccessReview,
responseCache: cache.NewLRUExpireCache(1024),
authorizedTTL: authorizedTTL,
unauthorizedTTL: unauthorizedTTL,
initialBackoff: initialBackoff,
}, nil
}

// Authorize makes a REST request to the remote service describing the attempted action as a JSON
Expand Down Expand Up @@ -128,17 +140,17 @@ func newWithBackoff(kubeConfigFile string, authorizedTTL, unauthorizedTTL, initi
// }
//
func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (authorized bool, reason string, err error) {
r := &v1beta1.SubjectAccessReview{}
r := &authorization.SubjectAccessReview{}
if user := attr.GetUser(); user != nil {
r.Spec = v1beta1.SubjectAccessReviewSpec{
r.Spec = authorization.SubjectAccessReviewSpec{
User: user.GetName(),
Groups: user.GetGroups(),
Extra: convertToSARExtra(user.GetExtra()),
}
}

if attr.IsResourceRequest() {
r.Spec.ResourceAttributes = &v1beta1.ResourceAttributes{
r.Spec.ResourceAttributes = &authorization.ResourceAttributes{
Namespace: attr.GetNamespace(),
Verb: attr.GetVerb(),
Group: attr.GetAPIGroup(),
Expand All @@ -148,7 +160,7 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (authorized bo
Name: attr.GetName(),
}
} else {
r.Spec.NonResourceAttributes = &v1beta1.NonResourceAttributes{
r.Spec.NonResourceAttributes = &authorization.NonResourceAttributes{
Path: attr.GetPath(),
Verb: attr.GetVerb(),
}
Expand All @@ -158,26 +170,22 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (authorized bo
return false, "", err
}
if entry, ok := w.responseCache.Get(string(key)); ok {
r.Status = entry.(v1beta1.SubjectAccessReviewStatus)
r.Status = entry.(authorization.SubjectAccessReviewStatus)
} else {
result := w.WithExponentialBackoff(func() restclient.Result {
return w.RestClient.Post().Body(r).Do()
var (
result *authorization.SubjectAccessReview
err error
)
webhook.WithExponentialBackoff(w.initialBackoff, func() error {
result, err = w.subjectAccessReview.Create(r)
return err
})
if err := result.Error(); err != nil {
if err != nil {
// An error here indicates bad configuration or an outage. Log for debugging.
glog.Errorf("Failed to make webhook authorizer request: %v", err)
return false, "", err
}
var statusCode int
result.StatusCode(&statusCode)
switch {
case statusCode < 200,
statusCode >= 300:
return false, "", fmt.Errorf("Error contacting webhook: %d", statusCode)
}
if err := result.Into(r); err != nil {
return false, "", err
}
r.Status = result.Status
if r.Status.Allowed {
w.responseCache.Add(string(key), r.Status, w.authorizedTTL)
} else {
Expand All @@ -187,14 +195,35 @@ func (w *WebhookAuthorizer) Authorize(attr authorizer.Attributes) (authorized bo
return r.Status.Allowed, r.Status.Reason, nil
}

func convertToSARExtra(extra map[string][]string) map[string]v1beta1.ExtraValue {
func convertToSARExtra(extra map[string][]string) map[string]authorization.ExtraValue {
if extra == nil {
return nil
}
ret := map[string]v1beta1.ExtraValue{}
ret := map[string]authorization.ExtraValue{}
for k, v := range extra {
ret[k] = v1beta1.ExtraValue(v)
ret[k] = authorization.ExtraValue(v)
}

return ret
}

// subjectAccessReviewInterfaceFromKubeconfig builds a client from the specified kubeconfig file,
// and returns a SubjectAccessReviewInterface that uses that client. Note that the client submits SubjectAccessReview
// requests to the exact path specified in the kubeconfig file, so arbitrary non-API servers can be targeted.
func subjectAccessReviewInterfaceFromKubeconfig(kubeConfigFile string) (authorizationclient.SubjectAccessReviewInterface, error) {
gw, err := webhook.NewGenericWebhook(kubeConfigFile, groupVersions, 0)
if err != nil {
return nil, err
}
return &subjectAccessReviewClient{gw}, nil
}

type subjectAccessReviewClient struct {
w *webhook.GenericWebhook
}

func (t *subjectAccessReviewClient) Create(subjectAccessReview *authorization.SubjectAccessReview) (*authorization.SubjectAccessReview, error) {
result := &authorization.SubjectAccessReview{}
err := t.w.RestClient.Post().Body(subjectAccessReview).Do().Into(result)
return result, err
}
129 changes: 87 additions & 42 deletions plugin/pkg/auth/authorizer/webhook/webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"net/url"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -183,7 +184,11 @@ current-context: default
return fmt.Errorf("failed to execute test template: %v", err)
}
// Create a new authorizer
_, err = newWithBackoff(p, 0, 0, 0)
sarClient, err := subjectAccessReviewInterfaceFromKubeconfig(p)
if err != nil {
return fmt.Errorf("error building sar client: %v", err)
}
_, err = newWithBackoff(sarClient, 0, 0, 0)
return err
}()
if err != nil && !tt.wantErr {
Expand All @@ -203,6 +208,7 @@ type Service interface {

// NewTestServer wraps a Service as an httptest.Server.
func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error) {
const webhookPath = "/testserver"
var tlsConfig *tls.Config
if cert != nil {
cert, err := tls.X509KeyPair(cert, key)
Expand All @@ -223,26 +229,44 @@ func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error
}

serveHTTP := func(w http.ResponseWriter, r *http.Request) {
if r.Method != "POST" {
http.Error(w, fmt.Sprintf("unexpected method: %v", r.Method), http.StatusMethodNotAllowed)
return
}
if r.URL.Path != webhookPath {
http.Error(w, fmt.Sprintf("unexpected path: %v", r.URL.Path), http.StatusNotFound)
return
}

var review v1beta1.SubjectAccessReview
if err := json.NewDecoder(r.Body).Decode(&review); err != nil {
bodyData, _ := ioutil.ReadAll(r.Body)
if err := json.Unmarshal(bodyData, &review); err != nil {
http.Error(w, fmt.Sprintf("failed to decode body: %v", err), http.StatusBadRequest)
return
}

// ensure we received the serialized review as expected
if review.APIVersion != "authorization.k8s.io/v1beta1" {
http.Error(w, fmt.Sprintf("wrong api version: %s", string(bodyData)), http.StatusBadRequest)
return
}
// once we have a successful request, always call the review to record that we were called
s.Review(&review)
if s.HTTPStatusCode() < 200 || s.HTTPStatusCode() >= 300 {
http.Error(w, "HTTP Error", s.HTTPStatusCode())
return
}
s.Review(&review)
type status struct {
Allowed bool `json:"allowed"`
Reason string `json:"reason"`
Allowed bool `json:"allowed"`
Reason string `json:"reason"`
EvaluationError string `json:"evaluationError"`
}
resp := struct {
APIVersion string `json:"apiVersion"`
Status status `json:"status"`
}{
APIVersion: v1beta1.SchemeGroupVersion.String(),
Status: status{review.Status.Allowed, review.Status.Reason},
Status: status{review.Status.Allowed, review.Status.Reason, review.Status.EvaluationError},
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(resp)
Expand All @@ -251,16 +275,24 @@ func NewTestServer(s Service, cert, key, caCert []byte) (*httptest.Server, error
server := httptest.NewUnstartedServer(http.HandlerFunc(serveHTTP))
server.TLS = tlsConfig
server.StartTLS()

// Adjust the path to point to our custom path
serverURL, _ := url.Parse(server.URL)
serverURL.Path = webhookPath
server.URL = serverURL.String()

return server, nil
}

// A service that can be set to allow all or deny all authorization requests.
type mockService struct {
allow bool
statusCode int
called int
}

func (m *mockService) Review(r *v1beta1.SubjectAccessReview) {
m.called++
r.Status.Allowed = m.allow
}
func (m *mockService) Allow() { m.allow = true }
Expand Down Expand Up @@ -291,7 +323,11 @@ func newAuthorizer(callbackURL string, clientCert, clientKey, ca []byte, cacheTi
if err := json.NewEncoder(tempfile).Encode(config); err != nil {
return nil, err
}
return newWithBackoff(p, cacheTime, cacheTime, 0)
sarClient, err := subjectAccessReviewInterfaceFromKubeconfig(p)
if err != nil {
return nil, fmt.Errorf("error building sar client: %v", err)
}
return newWithBackoff(sarClient, cacheTime, cacheTime, 0)
}

func TestTLSConfig(t *testing.T) {
Expand Down Expand Up @@ -506,29 +542,36 @@ func TestWebhook(t *testing.T) {
}

type webhookCacheTestCase struct {
statusCode int
attr authorizer.AttributesRecord

allow bool
statusCode int

expectedErr bool
expectedAuthorized bool
expectedCached bool
expectedCalls int
}

func testWebhookCacheCases(t *testing.T, serv *mockService, wh *WebhookAuthorizer, attr authorizer.AttributesRecord, tests []webhookCacheTestCase) {
for _, test := range tests {
func testWebhookCacheCases(t *testing.T, serv *mockService, wh *WebhookAuthorizer, tests []webhookCacheTestCase) {
for i, test := range tests {
serv.called = 0
serv.allow = test.allow
serv.statusCode = test.statusCode
authorized, _, err := wh.Authorize(attr)
authorized, _, err := wh.Authorize(test.attr)
if test.expectedErr && err == nil {
t.Errorf("Expected error")
t.Errorf("%d: Expected error", i)
continue
} else if !test.expectedErr && err != nil {
t.Fatal(err)
t.Errorf("%d: unexpected error: %v", i, err)
continue
}
if test.expectedAuthorized && !authorized {
if test.expectedCached {
t.Errorf("Webhook should have successful response cached, but authorizer reported unauthorized.")
} else {
t.Errorf("Webhook returned HTTP %d, but authorizer reported unauthorized.", test.statusCode)
}
} else if !test.expectedAuthorized && authorized {
t.Errorf("Webhook returned HTTP %d, but authorizer reported success.", test.statusCode)

if test.expectedAuthorized != authorized {
t.Errorf("%d: expected authorized=%v, got %v", i, test.expectedAuthorized, authorized)
}

if test.expectedCalls != serv.called {
t.Errorf("%d: expected %d calls, got %d", i, test.expectedCalls, serv.called)
}
}
}
Expand All @@ -549,27 +592,29 @@ func TestWebhookCache(t *testing.T) {
t.Fatal(err)
}

tests := []webhookCacheTestCase{
{statusCode: 500, expectedErr: true, expectedAuthorized: false, expectedCached: false},
{statusCode: 404, expectedErr: true, expectedAuthorized: false, expectedCached: false},
{statusCode: 403, expectedErr: true, expectedAuthorized: false, expectedCached: false},
{statusCode: 401, expectedErr: true, expectedAuthorized: false, expectedCached: false},
{statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCached: false},
{statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCached: true},
}

attr := authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "alice"}}
serv.allow = true
aliceAttr := authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "alice"}}
bobAttr := authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "bob"}}

testWebhookCacheCases(t, serv, wh, attr, tests)

// For a different request, webhook should be called again.
tests = []webhookCacheTestCase{
{statusCode: 500, expectedErr: true, expectedAuthorized: false, expectedCached: false},
{statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCached: false},
{statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCached: true},
tests := []webhookCacheTestCase{
// server error and 429's retry
{attr: aliceAttr, allow: false, statusCode: 500, expectedErr: true, expectedAuthorized: false, expectedCalls: 5},
{attr: aliceAttr, allow: false, statusCode: 429, expectedErr: true, expectedAuthorized: false, expectedCalls: 5},
// regular errors return errors but do not retry
{attr: aliceAttr, allow: false, statusCode: 404, expectedErr: true, expectedAuthorized: false, expectedCalls: 1},
{attr: aliceAttr, allow: false, statusCode: 403, expectedErr: true, expectedAuthorized: false, expectedCalls: 1},
{attr: aliceAttr, allow: false, statusCode: 401, expectedErr: true, expectedAuthorized: false, expectedCalls: 1},
// successful responses are cached
{attr: aliceAttr, allow: true, statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCalls: 1},
// later requests within the cache window don't hit the backend
{attr: aliceAttr, allow: false, statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCalls: 0},

// a request with different attributes doesn't hit the cache
{attr: bobAttr, allow: false, statusCode: 500, expectedErr: true, expectedAuthorized: false, expectedCalls: 5},
// successful response for other attributes is cached
{attr: bobAttr, allow: true, statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCalls: 1},
// later requests within the cache window don't hit the backend
{attr: bobAttr, allow: false, statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCalls: 0},
}
attr = authorizer.AttributesRecord{User: &user.DefaultInfo{Name: "bob"}}

testWebhookCacheCases(t, serv, wh, attr, tests)
testWebhookCacheCases(t, serv, wh, tests)
}