From fa40bc8f18f7c153910d048bbafefc430fe9bd11 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Thu, 19 Oct 2017 17:27:29 -0700 Subject: [PATCH 1/2] audit policy: reject audit policy files without apiVersion and kind --- .../k8s.io/apiserver/pkg/audit/policy/BUILD | 2 +- .../apiserver/pkg/audit/policy/reader.go | 27 ++++++++++++++++--- .../apiserver/pkg/audit/policy/reader_test.go | 27 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/BUILD b/staging/src/k8s.io/apiserver/pkg/audit/policy/BUILD index 8efedfb44a..17d5881b06 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/BUILD @@ -34,7 +34,7 @@ go_library( importpath = "k8s.io/apiserver/pkg/audit/policy", deps = [ "//vendor/github.com/golang/glog:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apiserver/pkg/apis/audit:go_default_library", "//vendor/k8s.io/apiserver/pkg/apis/audit/v1alpha1:go_default_library", "//vendor/k8s.io/apiserver/pkg/apis/audit/v1beta1:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/reader.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/reader.go index b748e8649c..1d02e1a3fb 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/reader.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/reader.go @@ -20,7 +20,7 @@ import ( "fmt" "io/ioutil" - "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" auditinternal "k8s.io/apiserver/pkg/apis/audit" auditv1alpha1 "k8s.io/apiserver/pkg/apis/audit/v1alpha1" auditv1beta1 "k8s.io/apiserver/pkg/apis/audit/v1beta1" @@ -30,6 +30,20 @@ import ( "github.com/golang/glog" ) +var ( + apiGroupVersions = []schema.GroupVersion{ + auditv1beta1.SchemeGroupVersion, + auditv1alpha1.SchemeGroupVersion, + } + apiGroupVersionSet = map[schema.GroupVersion]bool{} +) + +func init() { + for _, gv := range apiGroupVersions { + apiGroupVersionSet[gv] = true + } +} + func LoadPolicyFromFile(filePath string) (*auditinternal.Policy, error) { if filePath == "" { return nil, fmt.Errorf("file path not specified") @@ -40,11 +54,18 @@ func LoadPolicyFromFile(filePath string) (*auditinternal.Policy, error) { } policy := &auditinternal.Policy{} - decoder := audit.Codecs.UniversalDecoder(auditv1beta1.SchemeGroupVersion, auditv1alpha1.SchemeGroupVersion) - if err := runtime.DecodeInto(decoder, policyDef, policy); err != nil { + decoder := audit.Codecs.UniversalDecoder(apiGroupVersions...) + + _, gvk, err := decoder.Decode(policyDef, nil, policy) + if err != nil { return nil, fmt.Errorf("failed decoding file %q: %v", filePath, err) } + // Ensure the policy file contained an apiVersion and kind. + if !apiGroupVersionSet[schema.GroupVersion{Group: gvk.Group, Version: gvk.Version}] { + return nil, fmt.Errorf("unknown group version field %v in policy file %s", gvk, filePath) + } + if err := validation.ValidatePolicy(policy); err != nil { return nil, err.ToAggregate() } diff --git a/staging/src/k8s.io/apiserver/pkg/audit/policy/reader_test.go b/staging/src/k8s.io/apiserver/pkg/audit/policy/reader_test.go index f9bda28463..b05297a983 100644 --- a/staging/src/k8s.io/apiserver/pkg/audit/policy/reader_test.go +++ b/staging/src/k8s.io/apiserver/pkg/audit/policy/reader_test.go @@ -71,6 +71,24 @@ rules: - level: Metadata ` +const policyWithNoVersionOrKind = ` +rules: + - level: None + nonResourceURLs: + - /healthz* + - /version + - level: RequestResponse + users: ["tim"] + userGroups: ["testers", "developers"] + verbs: ["patch", "delete", "create"] + resources: + - group: "" + - group: "rbac.authorization.k8s.io" + resources: ["clusterroles", "clusterrolebindings"] + namespaces: ["default", "kube-system"] + - level: Metadata +` + var expectedPolicy = &audit.Policy{ Rules: []audit.PolicyRule{{ Level: audit.LevelNone, @@ -104,6 +122,15 @@ func TestParserV1alpha1(t *testing.T) { } } +func TestParsePolicyWithNoVersionOrKind(t *testing.T) { + f, err := writePolicy(t, policyWithNoVersionOrKind) + require.NoError(t, err) + defer os.Remove(f) + + _, err = LoadPolicyFromFile(f) + assert.Contains(t, err.Error(), "unknown group version field") +} + func TestParserV1beta1(t *testing.T) { f, err := writePolicy(t, policyDefV1beta1) require.NoError(t, err) From 393ac3cc555f033308436c82f25594737b6a98c1 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Thu, 19 Oct 2017 17:36:24 -0700 Subject: [PATCH 2/2] CHANGELOG: loosen language around audit policy file kind and apiVersion --- CHANGELOG-1.8.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG-1.8.md b/CHANGELOG-1.8.md index 89adc9f61a..876ef49c30 100644 --- a/CHANGELOG-1.8.md +++ b/CHANGELOG-1.8.md @@ -490,7 +490,7 @@ Consider the following changes, limitations, and guidelines before you upgrade: * The `--audit-policy-file` option is required if the `AdvancedAudit` feature is not explicitly turned off (`--feature-gates=AdvancedAudit=false`) on the API server. * The audit log file defaults to JSON encoding when using the advanced auditing feature gate. - * The `--audit-policy-file` option requires `kind` and `apiVersion` fields specifying what format version the `Policy` is using. + * An audit policy file without either an `apiVersion` or a `kind` field may be treated as invalid. * The webhook and log file now output the `v1beta1` event format. For more details, see [Advanced audit](https://kubernetes.io/docs/tasks/debug-application-cluster/audit/#advanced-audit).