Skip to content

Commit

Permalink
fix invalid match rules for advanced audit policy
Browse files Browse the repository at this point in the history
When users or groups are set in a rule, this rule should not match
attribute with unauthorized request where user and group are nil.
  • Loading branch information
CaoShuFeng committed Feb 6, 2018
1 parent 69f8b15 commit 9a7acaa
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
12 changes: 8 additions & 4 deletions staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -209,6 +219,10 @@ func testAuditLevel(t *testing.T, stages []audit.Stage) {

test(t, "subresource", audit.LevelRequest, stages, stages, "getPodLogs", "getPods")

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

0 comments on commit 9a7acaa

Please sign in to comment.