Merge pull request #54267 from ericchiang/audit-policy-file-without-kind-or-version

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>.

audit policy: reject audit policy files without apiVersion and kind

Closes https://github.com/kubernetes/kubernetes/issues/54254

/cc @sttts @CaoShuFeng @crassirostris @tallclair

/sig auth
/kind cleanup

```release-note
Audit policy files without apiVersion and kind are treated as invalid.
```

CHANGELOG-1.8.md

@@ -582,7 +582,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).

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",

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)
+	decoder := audit.Codecs.UniversalDecoder(apiGroupVersions...)
-	if err := runtime.DecodeInto(decoder, policyDef, policy); err != nil {
+
+	_, 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()
 	}

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)