Merge pull request #59398 from CaoShuFeng/audit_user_nil

Automatic merge from submit-queue. 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>. fix invalid match rules for advanced audit policy When users or groups are set in a rule, this rule should not match attribute with unauthorized request where user and group are nil. **Release note**: ```release-note Unauthorized requests will not match audit policy rules where users or groups are set. ``` Fixes: https://github.com/kubernetes/kubernetes/issues/59331

Merge pull request #59398 from CaoShuFeng/audit_user_nil
89ac18de · Kubernetes Submit Queue · GitHub · 8d9d0317 · 9a7acaae · 89ac18de
Unverified Commit 89ac18de authored Feb 19, 2018 by Kubernetes Submit Queue Committed by GitHub Feb 19, 2018
Hide whitespace changes
Inline Side-by-side

Showing with 22 additions and 4 deletions

checker.go staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go +8 -4

checker_test.go ...ing/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go +14 -0

No files found.
--- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go
+++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go
@@ -76,14 +76,18 @@ func (p *policyChecker) LevelAndStages(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 && attrs.GetUser() != nil {
-		if !hasString(r.Users, attrs.GetUser().GetName()) {
+	user := attrs.GetUser()
+	if len(r.Users) > 0 {
+		if user == nil || !hasString(r.Users, user.GetName()) {
 			return false
 		}
 	}
-	if len(r.UserGroups) > 0 && attrs.GetUser() != nil {
+	if len(r.UserGroups) > 0 {
+		if user == nil {
+			return false
+		}
 		matched := false
-		for _, group := range attrs.GetUser().GetGroups() {
+		for _, group := range user.GetGroups() {
 			if hasString(r.UserGroups, group) {
 				matched = true
 				break

--- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go
+++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go
@@ -73,6 +73,16 @@ var (
 			ResourceRequest: true,
 			Path:            "/api/v1/namespaces/default/pods/busybox",
 		},
+		"Unauthorized": &authorizer.AttributesRecord{
+			Verb:            "get",
+			Namespace:       "default",
+			APIGroup:        "", // Core
+			APIVersion:      "v1",
+			Resource:        "pods",
+			Name:            "busybox",
+			ResourceRequest: true,
+			Path:            "/api/v1/namespaces/default/pods/busybox",
+		},
 	}

 	rules = map[string]audit.PolicyRule{
@@ -227,6 +237,10 @@ func testAuditLevel(t *testing.T, stages []audit.Stage) {
 	test(t, "subresource", audit.LevelRequest, stages, stages, "getPodResourceWildcardMatching")
 	test(t, "subresource", audit.LevelRequest, stages, stages, "getPodSubResourceWildcardMatching")

+	test(t, "Unauthorized", audit.LevelNone, stages, stages, "tims")
+	test(t, "Unauthorized", audit.LevelMetadata, stages, stages, "tims", "default")
+	test(t, "Unauthorized", audit.LevelNone, stages, stages, "humans")
+	test(t, "Unauthorized", audit.LevelMetadata, stages, stages, "humans", "default")
 }

 func TestChecker(t *testing.T) {