From 9fef244d4ccce0ea8daf37ab86a7af4892d000cf Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Tue, 22 Aug 2017 16:52:40 +0200 Subject: [PATCH] Allow audit to log authorization failures --- .../apiserver/pkg/audit/policy/checker.go | 4 +- .../apiserver/pkg/endpoints/filters/BUILD | 2 + .../apiserver/pkg/endpoints/filters/audit.go | 81 +++++++----- .../pkg/endpoints/filters/authn_audit.go | 87 +++++++++++++ .../pkg/endpoints/filters/authn_audit_test.go | 122 ++++++++++++++++++ .../src/k8s.io/apiserver/pkg/server/config.go | 6 +- 6 files changed, 264 insertions(+), 38 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit.go create mode 100644 staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit_test.go diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go index 526710c2c6f..78082d216b4 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go @@ -59,12 +59,12 @@ func (p *policyChecker) Level(attrs authorizer.Attributes) audit.Level { // Check whether the rule matches the request attrs. func ruleMatches(r *audit.PolicyRule, attrs authorizer.Attributes) bool { - if len(r.Users) > 0 { + if len(r.Users) > 0 && attrs.GetUser() != nil { if !hasString(r.Users, attrs.GetUser().GetName()) { return false } } - if len(r.UserGroups) > 0 { + if len(r.UserGroups) > 0 && attrs.GetUser() != nil { matched := false for _, group := range attrs.GetUser().GetGroups() { if hasString(r.UserGroups, group) { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/BUILD b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/BUILD index a4c43bf3f25..a1c34ea660d 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/BUILD @@ -11,6 +11,7 @@ go_test( srcs = [ "audit_test.go", "authentication_test.go", + "authn_audit_test.go", "authorization_test.go", "impersonation_test.go", "legacy_audit_test.go", @@ -46,6 +47,7 @@ go_library( srcs = [ "audit.go", "authentication.go", + "authn_audit.go", "authorization.go", "doc.go", "impersonation.go", diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go index ecad0c25933..39813ea98cd 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit.go @@ -42,43 +42,19 @@ func WithAudit(handler http.Handler, requestContextMapper request.RequestContext return handler } return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - ctx, ok := requestContextMapper.Get(req) - if !ok { - responsewriters.InternalError(w, req, errors.New("no context found for request")) - return - } - - attribs, err := GetAuthorizerAttributes(ctx) + ctx, ev, err := createAuditEventAndAttachToContext(requestContextMapper, req, policy) if err != nil { - utilruntime.HandleError(fmt.Errorf("failed to GetAuthorizerAttributes: %v", err)) - responsewriters.InternalError(w, req, errors.New("failed to parse request")) + utilruntime.HandleError(fmt.Errorf("failed to create audit event: %v", err)) + responsewriters.InternalError(w, req, errors.New("failed to create audit event")) return } - - level := policy.Level(attribs) - audit.ObservePolicyLevel(level) - if level == auditinternal.LevelNone { - // Don't audit. + if ev == nil || ctx == nil { handler.ServeHTTP(w, req) return } - ev, err := audit.NewEventFromRequest(req, level, attribs) - if err != nil { - utilruntime.HandleError(fmt.Errorf("failed to complete audit event from request: %v", err)) - responsewriters.InternalError(w, req, errors.New("failed to update context")) - return - } - - ctx = request.WithAuditEvent(ctx, ev) - if err := requestContextMapper.Update(req, ctx); err != nil { - utilruntime.HandleError(fmt.Errorf("failed to attach audit event to the context: %v", err)) - responsewriters.InternalError(w, req, errors.New("failed to update context")) - return - } - ev.Stage = auditinternal.StageRequestReceived - processEvent(sink, ev) + processAuditEvent(sink, ev) // intercept the status code var longRunningSink audit.Sink @@ -102,7 +78,7 @@ func WithAudit(handler http.Handler, requestContextMapper request.RequestContext Reason: metav1.StatusReasonInternalError, Message: fmt.Sprintf("APIServer panic'd: %v", r), } - processEvent(sink, ev) + processAuditEvent(sink, ev) return } @@ -116,20 +92,56 @@ func WithAudit(handler http.Handler, requestContextMapper request.RequestContext if ev.ResponseStatus == nil && longRunningSink != nil { ev.ResponseStatus = fakedSuccessStatus ev.Stage = auditinternal.StageResponseStarted - processEvent(longRunningSink, ev) + processAuditEvent(longRunningSink, ev) } ev.Stage = auditinternal.StageResponseComplete if ev.ResponseStatus == nil { ev.ResponseStatus = fakedSuccessStatus } - processEvent(sink, ev) + processAuditEvent(sink, ev) }() handler.ServeHTTP(respWriter, req) }) } -func processEvent(sink audit.Sink, ev *auditinternal.Event) { +// createAuditEventAndAttachToContext is responsible for creating the audit event +// and attaching it to the appropriate request context. It returns: +// - context with audit event attached to it +// - created audit event +// - error if anything bad happened +func createAuditEventAndAttachToContext(requestContextMapper request.RequestContextMapper, req *http.Request, policy policy.Checker) (request.Context, *auditinternal.Event, error) { + ctx, ok := requestContextMapper.Get(req) + if !ok { + return nil, nil, fmt.Errorf("no context found for request") + } + + attribs, err := GetAuthorizerAttributes(ctx) + if err != nil { + return nil, nil, fmt.Errorf("failed to GetAuthorizerAttributes: %v", err) + } + + level := policy.Level(attribs) + audit.ObservePolicyLevel(level) + if level == auditinternal.LevelNone { + // Don't audit. + return nil, nil, nil + } + + ev, err := audit.NewEventFromRequest(req, level, attribs) + if err != nil { + return nil, nil, fmt.Errorf("failed to complete audit event from request: %v", err) + } + + ctx = request.WithAuditEvent(ctx, ev) + if err := requestContextMapper.Update(req, ctx); err != nil { + return nil, nil, fmt.Errorf("failed to attach audit event to context: %v", err) + } + + return ctx, ev, nil +} + +func processAuditEvent(sink audit.Sink, ev *auditinternal.Event) { audit.ObserveEvent() sink.ProcessEvents(ev) } @@ -176,7 +188,7 @@ func (a *auditResponseWriter) processCode(code int) { a.event.Stage = auditinternal.StageResponseStarted if a.sink != nil { - processEvent(a.sink, a.event) + processAuditEvent(a.sink, a.event) } }) } @@ -185,7 +197,6 @@ func (a *auditResponseWriter) Write(bs []byte) (int, error) { // the Go library calls WriteHeader internally if no code was written yet. But this will go unnoticed for us a.processCode(http.StatusOK) a.setHttpHeader() - return a.ResponseWriter.Write(bs) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit.go new file mode 100644 index 00000000000..a3c192f79fa --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit.go @@ -0,0 +1,87 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package filters + +import ( + "errors" + "fmt" + "net/http" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + auditinternal "k8s.io/apiserver/pkg/apis/audit" + "k8s.io/apiserver/pkg/audit" + "k8s.io/apiserver/pkg/audit/policy" + "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" + "k8s.io/apiserver/pkg/endpoints/request" +) + +// WithFailedAuthenticationAudit decorates a failed http.Handler used in WithAuthentication handler. +// It is meant to log only failed authentication requests. +func WithFailedAuthenticationAudit(failedHandler http.Handler, requestContextMapper request.RequestContextMapper, sink audit.Sink, policy policy.Checker) http.Handler { + if sink == nil || policy == nil { + return failedHandler + } + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + _, ev, err := createAuditEventAndAttachToContext(requestContextMapper, req, policy) + if err != nil { + utilruntime.HandleError(fmt.Errorf("failed to create audit event: %v", err)) + responsewriters.InternalError(w, req, errors.New("failed to create audit event")) + return + } + if ev == nil { + failedHandler.ServeHTTP(w, req) + return + } + + ev.ResponseStatus = &metav1.Status{} + ev.ResponseStatus.Message = getAuthMethods(req) + ev.Stage = auditinternal.StageResponseStarted + + rw := decorateResponseWriter(w, ev, sink) + failedHandler.ServeHTTP(rw, req) + }) +} + +func getAuthMethods(req *http.Request) string { + authMethods := []string{} + + if _, _, ok := req.BasicAuth(); ok { + authMethods = append(authMethods, "basic") + } + + auth := strings.TrimSpace(req.Header.Get("Authorization")) + parts := strings.Split(auth, " ") + if len(parts) > 1 && strings.ToLower(parts[0]) == "bearer" { + authMethods = append(authMethods, "bearer") + } + + token := strings.TrimSpace(req.URL.Query().Get("access_token")) + if len(token) > 0 { + authMethods = append(authMethods, "access_token") + } + + if req.TLS != nil && len(req.TLS.PeerCertificates) > 0 { + authMethods = append(authMethods, "x509") + } + + if len(authMethods) > 0 { + return fmt.Sprintf("Authentication failed, attempted: %s", strings.Join(authMethods, ", ")) + } + return "Authentication failed, no credentials provided" +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit_test.go new file mode 100644 index 00000000000..fb9eeebf6b3 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authn_audit_test.go @@ -0,0 +1,122 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package filters + +import ( + "crypto/tls" + "crypto/x509" + "net/http" + "net/http/httptest" + "strings" + "testing" + + auditinternal "k8s.io/apiserver/pkg/apis/audit" + "k8s.io/apiserver/pkg/audit/policy" +) + +func TestFailedAuthnAudit(t *testing.T) { + sink := &fakeAuditSink{} + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse) + handler := WithFailedAuthenticationAudit( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }), + &fakeRequestContextMapper{}, sink, policyChecker) + req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil) + req.RemoteAddr = "127.0.0.1" + req.SetBasicAuth("username", "password") + handler.ServeHTTP(httptest.NewRecorder(), req) + + if len(sink.events) != 1 { + t.Fatalf("Unexpected number of audit events generated, expected 1, got: %d", len(sink.events)) + } + ev := sink.events[0] + if ev.ResponseStatus.Code != http.StatusUnauthorized { + t.Errorf("Unexpected response code, expected unauthorized, got %d", ev.ResponseStatus.Code) + } + if !strings.Contains(ev.ResponseStatus.Message, "basic") { + t.Errorf("Expected response status message to contain basic auth method, got %s", ev.ResponseStatus.Message) + } + if ev.Verb != "list" { + t.Errorf("Unexpected verb, expected list, got %s", ev.Verb) + } + if ev.RequestURI != "/api/v1/namespaces/default/pods" { + t.Errorf("Unexpected user, expected /api/v1/namespaces/default/pods, got %s", ev.RequestURI) + } +} + +func TestFailedMultipleAuthnAudit(t *testing.T) { + sink := &fakeAuditSink{} + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse) + handler := WithFailedAuthenticationAudit( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }), + &fakeRequestContextMapper{}, sink, policyChecker) + req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil) + req.RemoteAddr = "127.0.0.1" + req.SetBasicAuth("username", "password") + req.TLS = &tls.ConnectionState{PeerCertificates: []*x509.Certificate{{}}} + handler.ServeHTTP(httptest.NewRecorder(), req) + + if len(sink.events) != 1 { + t.Fatalf("Unexpected number of audit events generated, expected 1, got: %d", len(sink.events)) + } + ev := sink.events[0] + if ev.ResponseStatus.Code != http.StatusUnauthorized { + t.Errorf("Unexpected response code, expected unauthorized, got %d", ev.ResponseStatus.Code) + } + if !strings.Contains(ev.ResponseStatus.Message, "basic") || !strings.Contains(ev.ResponseStatus.Message, "x509") { + t.Errorf("Expected response status message to contain basic and x509 auth method, got %s", ev.ResponseStatus.Message) + } + if ev.Verb != "list" { + t.Errorf("Unexpected verb, expected list, got %s", ev.Verb) + } + if ev.RequestURI != "/api/v1/namespaces/default/pods" { + t.Errorf("Unexpected user, expected /api/v1/namespaces/default/pods, got %s", ev.RequestURI) + } +} + +func TestFailedAuthnAuditWithoutAuthorization(t *testing.T) { + sink := &fakeAuditSink{} + policyChecker := policy.FakeChecker(auditinternal.LevelRequestResponse) + handler := WithFailedAuthenticationAudit( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }), + &fakeRequestContextMapper{}, sink, policyChecker) + req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil) + req.RemoteAddr = "127.0.0.1" + handler.ServeHTTP(httptest.NewRecorder(), req) + + if len(sink.events) != 1 { + t.Fatalf("Unexpected number of audit events generated, expected 1, got: %d", len(sink.events)) + } + ev := sink.events[0] + if ev.ResponseStatus.Code != http.StatusUnauthorized { + t.Errorf("Unexpected response code, expected unauthorized, got %d", ev.ResponseStatus.Code) + } + if !strings.Contains(ev.ResponseStatus.Message, "no credentials provided") { + t.Errorf("Expected response status message to contain no credentials provided, got %s", ev.ResponseStatus.Message) + } + if ev.Verb != "list" { + t.Errorf("Unexpected verb, expected list, got %s", ev.Verb) + } + if ev.RequestURI != "/api/v1/namespaces/default/pods" { + t.Errorf("Unexpected user, expected /api/v1/namespaces/default/pods, got %s", ev.RequestURI) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index 4d67c70528d..93fab4e720c 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -479,7 +479,11 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler { } else { handler = genericapifilters.WithLegacyAudit(handler, c.RequestContextMapper, c.LegacyAuditWriter) } - handler = genericapifilters.WithAuthentication(handler, c.RequestContextMapper, c.Authenticator, genericapifilters.Unauthorized(c.RequestContextMapper, c.Serializer, c.SupportsBasicAuth)) + failedHandler := genericapifilters.Unauthorized(c.RequestContextMapper, c.Serializer, c.SupportsBasicAuth) + if utilfeature.DefaultFeatureGate.Enabled(features.AdvancedAuditing) { + failedHandler = genericapifilters.WithFailedAuthenticationAudit(failedHandler, c.RequestContextMapper, c.AuditBackend, c.AuditPolicyChecker) + } + handler = genericapifilters.WithAuthentication(handler, c.RequestContextMapper, c.Authenticator, failedHandler) handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true") handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.RequestContextMapper, c.LongRunningFunc, c.RequestTimeout) handler = genericapifilters.WithRequestInfo(handler, NewRequestInfoResolver(c), c.RequestContextMapper)