Merge pull request #58807 from CaoShuFeng/audit_annotation_rbac

Automatic merge from submit-queue (batch tested with PRs 61183, 58807). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add RBAC information to audit logs

Depends on: https://github.com/kubernetes/kubernetes/pull/58806
**Release note**:
```release-note
RBAC information is included in audit logs via audit.Event annotations:
authorization.k8s.io/decision = {allow, forbid}
authorization.k8s.io/reason = human-readable reason for the decision
```

plugin/pkg/auth/authorizer/rbac/rbac.go

@@ -62,7 +62,7 @@ type authorizingVisitor struct {
 func (v *authorizingVisitor) visit(source fmt.Stringer, rule *rbac.PolicyRule, err error) bool {
 	if rule != nil && RuleAllows(v.requestAttributes, rule) {
 		v.allowed = true
-		v.reason = fmt.Sprintf("allowed by %s", source.String())
+		v.reason = fmt.Sprintf("RBAC: allowed by %s", source.String())
 		return false
 	}
 	if err != nil {
@@ -120,7 +120,7 @@ func (r *RBACAuthorizer) Authorize(requestAttributes authorizer.Attributes) (aut
 
 	reason := ""
 	if len(ruleCheckingVisitor.errors) > 0 {
-		reason = fmt.Sprintf("%v", utilerrors.NewAggregate(ruleCheckingVisitor.errors))
+		reason = fmt.Sprintf("RBAC: %v", utilerrors.NewAggregate(ruleCheckingVisitor.errors))
 	}
 	return authorizer.DecisionNoOpinion, reason, nil
 }

staging/src/k8s.io/apiserver/pkg/endpoints/filters/BUILD

@@ -20,6 +20,7 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//vendor/github.com/pborman/uuid:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/k8s.io/api/authentication/v1:go_default_library",
         "//vendor/k8s.io/api/batch/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

staging/src/k8s.io/apiserver/pkg/endpoints/filters/audit_test.go

@@ -736,7 +736,8 @@ func TestAudit(t *testing.T) {
 }
 
 type fakeRequestContextMapper struct {
-	user *user.DefaultInfo
+	user  *user.DefaultInfo
+	audit *auditinternal.Event
 }
 
 func (m *fakeRequestContextMapper) Get(req *http.Request) (request.Context, bool) {
@@ -744,6 +745,9 @@ func (m *fakeRequestContextMapper) Get(req *http.Request) (request.Context, bool
 	if m.user != nil {
 		ctx = request.WithUser(ctx, m.user)
 	}
+	if m.audit != nil {
+		ctx = request.WithAuditEvent(ctx, m.audit)
+	}
 
 	resolver := newTestRequestInfoResolver()
 	info, err := resolver.NewRequestInfo(req)

staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization.go

@@ -23,11 +23,23 @@ import (
 	"github.com/golang/glog"
 
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/audit"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
 	"k8s.io/apiserver/pkg/endpoints/request"
 )
 
+const (
+	// Annotation key names set in advanced audit
+	decisionAnnotationKey = "authorization.k8s.io/decision"
+	reasonAnnotationKey   = "authorization.k8s.io/reason"
+
+	// Annotation values set in advanced audit
+	decisionAllow  = "allow"
+	decisionForbid = "forbid"
+	reasonError    = "internal error"
+)
+
 // WithAuthorizationCheck passes all authorized requests on to handler, and returns a forbidden error otherwise.
 func WithAuthorization(handler http.Handler, requestContextMapper request.RequestContextMapper, a authorizer.Authorizer, s runtime.NegotiatedSerializer) http.Handler {
 	if a == nil {
@@ -40,6 +52,7 @@ func WithAuthorization(handler http.Handler, requestContextMapper request.Reques
 			responsewriters.InternalError(w, req, errors.New("no context found for request"))
 			return
 		}
+		ae := request.AuditEventFrom(ctx)
 
 		attributes, err := GetAuthorizerAttributes(ctx)
 		if err != nil {
@@ -49,15 +62,20 @@ func WithAuthorization(handler http.Handler, requestContextMapper request.Reques
 		authorized, reason, err := a.Authorize(attributes)
 		// an authorizer like RBAC could encounter evaluation errors and still allow the request, so authorizer decision is checked before error here.
 		if authorized == authorizer.DecisionAllow {
+			audit.LogAnnotation(ae, decisionAnnotationKey, decisionAllow)
+			audit.LogAnnotation(ae, reasonAnnotationKey, reason)
 			handler.ServeHTTP(w, req)
 			return
 		}
 		if err != nil {
+			audit.LogAnnotation(ae, reasonAnnotationKey, reasonError)
 			responsewriters.InternalError(w, req, err)
 			return
 		}
 
 		glog.V(4).Infof("Forbidden: %#v, Reason: %q", req.RequestURI, reason)
+		audit.LogAnnotation(ae, decisionAnnotationKey, decisionForbid)
+		audit.LogAnnotation(ae, reasonAnnotationKey, reason)
 		responsewriters.Forbidden(ctx, attributes, w, req, reason, s)
 	})
 }

staging/src/k8s.io/apiserver/pkg/endpoints/filters/authorization_test.go

@@ -23,7 +23,11 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	batch "k8s.io/api/batch/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
+	auditinternal "k8s.io/apiserver/pkg/apis/audit"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
 	"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
 	"k8s.io/apiserver/pkg/endpoints/request"
@@ -127,3 +131,65 @@ func TestGetAuthorizerAttributes(t *testing.T) {
 		}
 	}
 }
+
+type fakeAuthorizer struct {
+	decision authorizer.Decision
+	reason   string
+	err      error
+}
+
+func (f fakeAuthorizer) Authorize(a authorizer.Attributes) (authorizer.Decision, string, error) {
+	return f.decision, f.reason, f.err
+}
+
+func TestAuditAnnotation(t *testing.T) {
+	testcases := map[string]struct {
+		authorizer         fakeAuthorizer
+		decisionAnnotation string
+		reasonAnnotation   string
+	}{
+		"decision allow": {
+			fakeAuthorizer{
+				authorizer.DecisionAllow,
+				"RBAC: allowed to patch pod",
+				nil,
+			},
+			"allow",
+			"RBAC: allowed to patch pod",
+		},
+		"decision forbid": {
+			fakeAuthorizer{
+				authorizer.DecisionDeny,
+				"RBAC: not allowed to patch pod",
+				nil,
+			},
+			"forbid",
+			"RBAC: not allowed to patch pod",
+		},
+		"error": {
+			fakeAuthorizer{
+				authorizer.DecisionNoOpinion,
+				"",
+				errors.New("can't parse user info"),
+			},
+			"",
+			reasonError,
+		},
+	}
+
+	scheme := runtime.NewScheme()
+	negotiatedSerializer := serializer.DirectCodecFactory{CodecFactory: serializer.NewCodecFactory(scheme)}
+	for k, tc := range testcases {
+		audit := &auditinternal.Event{Level: auditinternal.LevelMetadata}
+		handler := WithAuthorization(&fakeHTTPHandler{}, &fakeRequestContextMapper{
+			audit: audit,
+		}, tc.authorizer, negotiatedSerializer)
+
+		req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil)
+		req.RemoteAddr = "127.0.0.1"
+		handler.ServeHTTP(httptest.NewRecorder(), req)
+		assert.Equal(t, tc.decisionAnnotation, audit.Annotations[decisionAnnotationKey], k+": unexpected decision annotation")
+		assert.Equal(t, tc.reasonAnnotation, audit.Annotations[reasonAnnotationKey], k+": unexpected reason annotation")
+	}
+
+}

test/e2e/auth/audit.go

@@ -26,12 +26,14 @@ import (
 	apiv1 "k8s.io/api/core/v1"
 	extensions "k8s.io/api/extensions/v1beta1"
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
-	"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
+	apiextensionclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	"k8s.io/apiextensions-apiserver/test/integration/testserver"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/apis/audit/v1beta1"
+	clientset "k8s.io/client-go/kubernetes"
+	restclient "k8s.io/client-go/rest"
 	"k8s.io/kubernetes/test/e2e/framework"
 	imageutils "k8s.io/kubernetes/test/utils/image"
 
@@ -63,9 +65,19 @@ var _ = SIGDescribe("Advanced Audit", func() {
 
 		config, err := framework.LoadConfig()
 		framework.ExpectNoError(err, "failed to load config")
-		apiExtensionClient, err := clientset.NewForConfig(config)
+		apiExtensionClient, err := apiextensionclientset.NewForConfig(config)
 		framework.ExpectNoError(err, "failed to initialize apiExtensionClient")
 
+		By("Creating a kubernetes client that impersonates an unauthorized anonymous user")
+		config, err = framework.LoadConfig()
+		framework.ExpectNoError(err)
+		config.Impersonate = restclient.ImpersonationConfig{
+			UserName: "system:anonymous",
+			Groups:   []string{"system:unauthenticated"},
+		}
+		anonymousClient, err := clientset.NewForConfig(config)
+		framework.ExpectNoError(err)
+
 		testCases := []struct {
 			action func()
 			events []auditEvent
@@ -118,6 +130,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						true,
 						true,
+						"allow",
 					}, {
 						v1beta1.LevelRequest,
 						v1beta1.StageResponseComplete,
@@ -129,6 +142,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelRequest,
 						v1beta1.StageResponseComplete,
@@ -140,6 +154,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelRequest,
 						v1beta1.StageResponseStarted,
@@ -151,6 +166,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelRequest,
 						v1beta1.StageResponseComplete,
@@ -162,6 +178,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelRequestResponse,
 						v1beta1.StageResponseComplete,
@@ -173,6 +190,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						true,
 						true,
+						"allow",
 					}, {
 						v1beta1.LevelRequestResponse,
 						v1beta1.StageResponseComplete,
@@ -184,6 +202,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						true,
 						true,
+						"allow",
 					}, {
 						v1beta1.LevelRequestResponse,
 						v1beta1.StageResponseComplete,
@@ -195,6 +214,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						true,
 						true,
+						"allow",
 					},
 				},
 			},
@@ -239,6 +259,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						true,
 						true,
+						"allow",
 					}, {
 						v1beta1.LevelRequest,
 						v1beta1.StageResponseComplete,
@@ -250,6 +271,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelRequest,
 						v1beta1.StageResponseComplete,
@@ -261,6 +283,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelRequest,
 						v1beta1.StageResponseStarted,
@@ -272,6 +295,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelRequest,
 						v1beta1.StageResponseComplete,
@@ -283,6 +307,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelRequestResponse,
 						v1beta1.StageResponseComplete,
@@ -294,6 +319,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						true,
 						true,
+						"allow",
 					}, {
 						v1beta1.LevelRequestResponse,
 						v1beta1.StageResponseComplete,
@@ -305,6 +331,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						true,
 						true,
+						"allow",
 					}, {
 						v1beta1.LevelRequestResponse,
 						v1beta1.StageResponseComplete,
@@ -316,6 +343,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						true,
 						true,
+						"allow",
 					},
 				},
 			},
@@ -366,6 +394,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -377,6 +406,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -388,6 +418,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseStarted,
@@ -399,6 +430,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -410,6 +442,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -421,6 +454,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -432,6 +466,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -443,6 +478,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					},
 				},
 			},
@@ -492,6 +528,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -503,6 +540,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -514,6 +552,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseStarted,
@@ -525,6 +564,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -536,6 +576,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -547,6 +588,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -558,6 +600,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					}, {
 						v1beta1.LevelMetadata,
 						v1beta1.StageResponseComplete,
@@ -569,6 +612,7 @@ var _ = SIGDescribe("Advanced Audit", func() {
 						namespace,
 						false,
 						false,
+						"allow",
 					},
 				},
 			},
@@ -581,50 +625,87 @@ var _ = SIGDescribe("Advanced Audit", func() {
 				},
 				[]auditEvent{
 					{
-						level:          v1beta1.LevelRequestResponse,
+						level:             v1beta1.LevelRequestResponse,
-						stage:          v1beta1.StageResponseComplete,
+						stage:             v1beta1.StageResponseComplete,
-						requestURI:     "/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions",
+						requestURI:        "/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions",
-						verb:           "create",
+						verb:              "create",
-						code:           201,
+						code:              201,
-						user:           auditTestUser,
+						user:              auditTestUser,
-						resource:       "customresourcedefinitions",
+						resource:          "customresourcedefinitions",
-						requestObject:  true,
+						requestObject:     true,
-						responseObject: true,
+						responseObject:    true,
+						authorizeDecision: "allow",
 					}, {
-						level:          v1beta1.LevelMetadata,
+						level:             v1beta1.LevelMetadata,
-						stage:          v1beta1.StageResponseComplete,
+						stage:             v1beta1.StageResponseComplete,
-						requestURI:     fmt.Sprintf("/apis/%s/v1beta1/%s", crdNamespace, crdName),
+						requestURI:        fmt.Sprintf("/apis/%s/v1beta1/%s", crdNamespace, crdName),
-						verb:           "create",
+						verb:              "create",
-						code:           201,
+						code:              201,
-						user:           auditTestUser,
+						user:              auditTestUser,
-						resource:       crdName,
+						resource:          crdName,
-						requestObject:  false,
+						requestObject:     false,
-						responseObject: false,
+						responseObject:    false,
+						authorizeDecision: "allow",
 					}, {
-						level:          v1beta1.LevelRequestResponse,
+						level:             v1beta1.LevelRequestResponse,
-						stage:          v1beta1.StageResponseComplete,
+						stage:             v1beta1.StageResponseComplete,
-						requestURI:     fmt.Sprintf("/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/%s", crd.Name),
+						requestURI:        fmt.Sprintf("/apis/apiextensions.k8s.io/v1beta1/customresourcedefinitions/%s", crd.Name),
-						verb:           "delete",
+						verb:              "delete",
-						code:           200,
+						code:              200,
-						user:           auditTestUser,
+						user:              auditTestUser,
-						resource:       "customresourcedefinitions",
+						resource:          "customresourcedefinitions",
-						requestObject:  false,
+						requestObject:     false,
-						responseObject: true,
+						responseObject:    true,
+						authorizeDecision: "allow",
 					}, {
-						level:          v1beta1.LevelMetadata,
+						level:             v1beta1.LevelMetadata,
-						stage:          v1beta1.StageResponseComplete,
+						stage:             v1beta1.StageResponseComplete,
-						requestURI:     fmt.Sprintf("/apis/%s/v1beta1/%s/setup-instance", crdNamespace, crdName),
+						requestURI:        fmt.Sprintf("/apis/%s/v1beta1/%s/setup-instance", crdNamespace, crdName),
-						verb:           "delete",
+						verb:              "delete",
-						code:           200,
+						code:              200,
-						user:           auditTestUser,
+						user:              auditTestUser,
-						resource:       crdName,
+						resource:          crdName,
-						requestObject:  false,
+						requestObject:     false,
-						responseObject: false,
+						responseObject:    false,
+						authorizeDecision: "allow",
 					},
 				},
 			},
 		}
 
+		// test authorizer annotations, RBAC is required.
+		annotationTestCases := []struct {
+			action func()
+			events []auditEvent
+		}{
+
+			// get a pod with unauthorized user
+			{
+				func() {
+					_, err := anonymousClient.CoreV1().Pods(namespace).Get("another-audit-pod", metav1.GetOptions{})
+					expectForbidden(err)
+				},
+				[]auditEvent{
+					{
+						level:             v1beta1.LevelRequest,
+						stage:             v1beta1.StageResponseComplete,
+						requestURI:        fmt.Sprintf("/api/v1/namespaces/%s/pods/another-audit-pod", namespace),
+						verb:              "get",
+						code:              403,
+						user:              auditTestUser,
+						resource:          "pods",
+						namespace:         namespace,
+						requestObject:     false,
+						responseObject:    false,
+						authorizeDecision: "forbid",
+					},
+				},
+			},
+		}
+
+		if framework.IsRBACEnabled(f) {
+			testCases = append(testCases, annotationTestCases...)
+		}
 		expectedEvents := []auditEvent{}
 		for _, t := range testCases {
 			t.action()
@@ -647,16 +728,17 @@ var _ = SIGDescribe("Advanced Audit", func() {
 })
 
 type auditEvent struct {
-	level          v1beta1.Level
+	level             v1beta1.Level
-	stage          v1beta1.Stage
+	stage             v1beta1.Stage
-	requestURI     string
+	requestURI        string
-	verb           string
+	verb              string
-	code           int32
+	code              int32
-	user           string
+	user              string
-	resource       string
+	resource          string
-	namespace      string
+	namespace         string
-	requestObject  bool
+	requestObject     bool
-	responseObject bool
+	responseObject    bool
+	authorizeDecision string
 }
 
 // Search the audit log for the expected audit lines.
@@ -725,5 +807,6 @@ func parseAuditLine(line string) (auditEvent, error) {
 	if e.RequestObject != nil {
 		event.requestObject = true
 	}
+	event.authorizeDecision = e.Annotations["authorization.k8s.io/decision"]
 	return event, nil
 }