From 85491f1578b9b97751a332d3b957d874cecf27b3 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Thu, 17 Aug 2017 11:50:25 -0700 Subject: [PATCH] Audit policy v1beta1 now supports matching subresources and resource names. policy: - level: Metadata resources: - group: "" resources ["pods/logs"] - level: None resources: - group: "" resources: ["configmaps"] resourceNames: ["controller-leader"] The top level resource no longer matches the subresource. For example "pods" no longer matches requests to the logs subresource on pods. ```release-note Audit policy supports matching subresources and resource names, but the top level resource no longer matches the subresouce. For example "pods" no longer matches requests to the logs subresource of pods. Use "pods/logs" to match subresources. ``` --- .../k8s.io/apiserver/pkg/apis/audit/types.go | 11 +++++-- .../pkg/apis/audit/v1alpha1/types.go | 11 +++++-- .../apiserver/pkg/apis/audit/v1beta1/types.go | 11 +++++-- .../pkg/apis/audit/validation/validation.go | 18 ++++++----- .../apis/audit/validation/validation_test.go | 6 ++++ .../apiserver/pkg/audit/policy/checker.go | 13 +++++++- .../pkg/audit/policy/checker_test.go | 32 +++++++++++++++++++ 7 files changed, 87 insertions(+), 15 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go index cfea0550661..d6d15adb583 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/types.go @@ -215,10 +215,17 @@ type GroupResources struct { // The empty string represents the core API group. // +optional Group string - // Resources is a list of resources within the API group. - // Any empty list implies every resource kind in the API group. + // Resources is a list of resources within the API group. Subresources are + // matched using a "/" to indicate the subresource. For example, "pods/logs" + // would match request to the logs subresource of pods. The top level resource + // does not match subresources, "pods" doesn't match "pods/logs". // +optional Resources []string + // ResourceNames is a list of resource instance names that the policy matches. + // Using this field requires Resources to be specified. + // An empty list implies that every instance of the resource is matched. + // +optional + ResourceNames []string } // ObjectReference contains enough information to let you inspect or modify the referred object. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go index e2022656828..9eaaa111136 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1/types.go @@ -216,10 +216,17 @@ type GroupResources struct { // The empty string represents the core API group. // +optional Group string `json:"group,omitempty" protobuf:"bytes,1,opt,name=group"` - // Resources is a list of resources within the API group. - // Any empty list implies every resource kind in the API group. + // Resources is a list of resources within the API group. Subresources are + // matched using a "/" to indicate the subresource. For example, "pods/logs" + // would match request to the logs subresource of pods. The top level resource + // does not match subresources, "pods" doesn't match "pods/logs". // +optional Resources []string `json:"resources,omitempty" protobuf:"bytes,2,rep,name=resources"` + // ResourceNames is a list of resource instance names that the policy matches. + // Using this field requires Resources to be specified. + // An empty list implies that every instance of the resource is matched. + // +optional + ResourceNames []string `json:"resourceNames,omitempty" protobuf:"bytes,3,rep,name=resourceNames"` } // ObjectReference contains enough information to let you inspect or modify the referred object. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go index 1ecac858854..b60b78ec49f 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1/types.go @@ -209,10 +209,17 @@ type GroupResources struct { // The empty string represents the core API group. // +optional Group string `json:"group,omitempty" protobuf:"bytes,1,opt,name=group"` - // Resources is a list of resources within the API group. - // Any empty list implies every resource kind in the API group. + // Resources is a list of resources within the API group. Subresources are + // matched using a "/" to indicate the subresource. For example, "pods/logs" + // would match request to the logs subresource of pods. The top level resource + // does not match subresources, "pods" doesn't match "pods/logs". // +optional Resources []string `json:"resources,omitempty" protobuf:"bytes,2,rep,name=resources"` + // ResourceNames is a list of resource instance names that the policy matches. + // Using this field requires Resources to be specified. + // An empty list implies that every instance of the resource is matched. + // +optional + ResourceNames []string `json:"resourceNames,omitempty" protobuf:"bytes,3,rep,name=resourceNames"` } // ObjectReference contains enough information to let you inspect or modify the referred object. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go index 80b73f851cb..0db2030433a 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation.go @@ -88,16 +88,18 @@ func validateResources(groupResources []audit.GroupResources, fldPath *field.Pat var allErrs field.ErrorList for _, groupResource := range groupResources { // The empty string represents the core API group. - if len(groupResource.Group) == 0 { - continue + if len(groupResource.Group) != 0 { + // Group names must be lower case and be valid DNS subdomains. + // reference: https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md + // an error is returned for group name like rbac.authorization.k8s.io/v1beta1 + // rbac.authorization.k8s.io is the valid one + if msgs := validation.NameIsDNSSubdomain(groupResource.Group, false); len(msgs) != 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), groupResource.Group, strings.Join(msgs, ","))) + } } - // Group names must be lower case and be valid DNS subdomains. - // reference: https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md - // an error is returned for group name like rbac.authorization.k8s.io/v1beta1 - // rbac.authorization.k8s.io is the valid one - if msgs := validation.NameIsDNSSubdomain(groupResource.Group, false); len(msgs) != 0 { - allErrs = append(allErrs, field.Invalid(fldPath.Child("group"), groupResource.Group, strings.Join(msgs, ","))) + if len(groupResource.ResourceNames) > 0 && len(groupResource.Resources) == 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("resourceNames"), groupResource.ResourceNames, "using resourceNames requires at least one resource")) } } return allErrs diff --git a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go index 99692157791..3acb9598bd0 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/audit/validation/validation_test.go @@ -102,6 +102,12 @@ func TestValidatePolicy(t *testing.T) { "/metrics", }, }, + { // ResourceNames without Resources + Level: audit.LevelMetadata, + Verbs: []string{"get"}, + Resources: []audit.GroupResources{{ResourceNames: []string{"leader"}}}, + Namespaces: []string{"kube-system"}, + }, } errorCases := []audit.Policy{} for _, rule := range invalidRules { 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..8ebe69ca136 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker.go @@ -143,6 +143,15 @@ func ruleMatchesResource(r *audit.PolicyRule, attrs authorizer.Attributes) bool apiGroup := attrs.GetAPIGroup() resource := attrs.GetResource() + // If subresource, the resource in the policy must match "(resource)/(subresource)" + // + // TODO: consider adding options like "pods/*" to match all subresources. + if sr := attrs.GetSubresource(); sr != "" { + resource = resource + "/" + sr + } + + name := attrs.GetName() + for _, gr := range r.Resources { if gr.Group == apiGroup { if len(gr.Resources) == 0 { @@ -150,7 +159,9 @@ func ruleMatchesResource(r *audit.PolicyRule, attrs authorizer.Attributes) bool } for _, res := range gr.Resources { if res == resource { - return true + if len(gr.ResourceNames) == 0 || hasString(gr.ResourceNames, name) { + return true + } } } } diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go index 29cf1a4689a..015e23beb7e 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/checker_test.go @@ -61,6 +61,18 @@ func TestChecker(t *testing.T) { ResourceRequest: false, Path: "/logs/kubelet.log", }, + "subresource": &authorizer.AttributesRecord{ + User: tim, + Verb: "get", + Namespace: "default", + APIGroup: "", // Core + APIVersion: "v1", + Resource: "pods", + Subresource: "log", + Name: "busybox", + ResourceRequest: true, + Path: "/api/v1/namespaces/default/pods/busybox", + }, } rules := map[string]audit.PolicyRule{ @@ -88,6 +100,11 @@ func TestChecker(t *testing.T) { Verbs: []string{"get"}, Resources: []audit.GroupResources{{Resources: []string{"pods"}}}, }, + "getPodLogs": { + Level: audit.LevelRequest, + Verbs: []string{"get"}, + Resources: []audit.GroupResources{{Resources: []string{"pods/log"}}}, + }, "getClusterRoles": { Level: audit.LevelRequestResponse, Verbs: []string{"get"}, @@ -111,6 +128,14 @@ func TestChecker(t *testing.T) { "/metrics", }, }, + "clusterRoleEdit": { + Level: audit.LevelRequest, + Resources: []audit.GroupResources{{ + Group: "rbac.authorization.k8s.io", + Resources: []string{"clusterroles"}, + ResourceNames: []string{"edit"}, + }}, + }, } test := func(req string, expected audit.Level, ruleNames ...string) { @@ -135,6 +160,7 @@ func TestChecker(t *testing.T) { test("namespaced", audit.LevelNone, "getMetrics") test("namespaced", audit.LevelMetadata, "getMetrics", "serviceAccounts", "default") test("namespaced", audit.LevelRequestResponse, "getMetrics", "getPods", "default") + test("namespaced", audit.LevelRequestResponse, "getPodLogs", "getPods") test("cluster", audit.LevelMetadata, "default") test("cluster", audit.LevelNone, "create") @@ -143,10 +169,12 @@ func TestChecker(t *testing.T) { test("cluster", audit.LevelNone, "serviceAccounts") test("cluster", audit.LevelNone, "getPods") test("cluster", audit.LevelRequestResponse, "getClusterRoles") + test("cluster", audit.LevelRequest, "clusterRoleEdit", "getClusterRoles") test("cluster", audit.LevelNone, "getLogs") test("cluster", audit.LevelNone, "getMetrics") test("cluster", audit.LevelMetadata, "getMetrics", "serviceAccounts", "default") test("cluster", audit.LevelRequestResponse, "getMetrics", "getClusterRoles", "default") + test("cluster", audit.LevelNone, "getPodLogs", "getPods") test("nonResource", audit.LevelMetadata, "default") test("nonResource", audit.LevelNone, "create") @@ -159,4 +187,8 @@ func TestChecker(t *testing.T) { test("nonResource", audit.LevelNone, "getMetrics") test("nonResource", audit.LevelMetadata, "getMetrics", "serviceAccounts", "default") test("nonResource", audit.LevelRequestResponse, "getLogs", "getClusterRoles", "default") + test("nonResource", audit.LevelNone, "getPodLogs", "getPods") + + test("subresource", audit.LevelRequest, "getPodLogs", "getPods") + test("subresource", audit.LevelRequest, "getPods", "getPodLogs") }