Merge pull request #51119 from soltysh/failed_authn_audit

Automatic merge from submit-queue (batch tested with PRs 50832, 51119, 51636, 48921, 51712)

Allow audit to log authorization failures

**What this PR does / why we need it**:
This PR extends our current audit mechanism allowing to audit failed login attempts. 

**Release note**:

```release-note
Advanced audit allows logging failed login attempts
```

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) {

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",

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)
 }
 

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"
+}

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)
+	}
+}

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -482,7 +482,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)