update e2e tests, address comments

Signed-off-by: everettraven <[email protected]>
carvel-dev · May 8, 2024 · 0505c5f · 0505c5f
1 parent 4002225
commit 0505c5f
Show file tree

Hide file tree

Showing 5 changed files with 461 additions and 9 deletions.
diff --git a/pkg/kapp/permissions/preflight.go b/pkg/kapp/permissions/preflight.go
@@ -27,19 +27,19 @@ type Preflight struct {
 
 const (
 	PermissionValidatorTypeSelfSubjectAccessReview = "SelfSubjectAccessReview"
-	PermissionValidatorTypeSelfSubjectRulesReview  = "SelfSubjectRulesReviews"
+	PermissionValidatorTypeSelfSubjectRulesReview  = "SelfSubjectRulesReview"
 )
 
 type PreflightConfig struct {
-	PermissionValidatorType string `json:"permissionValidatorResource"`
+	PermissionValidatorResource string `json:"permissionValidatorResource"`
 }
 
 func NewPreflight(depsFactory cmdcore.DepsFactory, enabled bool) preflight.Check {
 	return &Preflight{
 		depsFactory: depsFactory,
 		enabled:     enabled,
 		config: &PreflightConfig{
-			PermissionValidatorType: PermissionValidatorTypeSelfSubjectAccessReview,
+			PermissionValidatorResource: PermissionValidatorTypeSelfSubjectAccessReview,
 		},
 	}
 }
@@ -64,14 +64,14 @@ func (p *Preflight) SetConfig(cfg preflight.CheckConfig) error {
 		return fmt.Errorf("parsing permissions preflight config: %w", err)
 	}
 
-	switch pCfg.PermissionValidatorType {
-	case PermissionValidatorTypeSelfSubjectAccessReview:
-	case PermissionValidatorTypeSelfSubjectRulesReview:
+	switch pCfg.PermissionValidatorResource {
+	// Valid, do nothing
+	case PermissionValidatorTypeSelfSubjectAccessReview, PermissionValidatorTypeSelfSubjectRulesReview:
 	// Default to using SelfSubjectAccessReview
 	case "":
-		pCfg.PermissionValidatorType = PermissionValidatorTypeSelfSubjectAccessReview
+		pCfg.PermissionValidatorResource = PermissionValidatorTypeSelfSubjectAccessReview
 	default:
-		return fmt.Errorf("unknown permissionValidatorType %q", pCfg.PermissionValidatorType)
+		return fmt.Errorf("unknown permissionValidatorType %q", pCfg.PermissionValidatorResource)
 	}
 	return nil
 }
@@ -88,7 +88,7 @@ func (p *Preflight) Run(ctx context.Context, changeGraph *ctldgraph.ChangeGraph)
 	}
 
 	var permissionValidator PermissionValidator
-	switch p.config.PermissionValidatorType {
+	switch p.config.PermissionValidatorResource {
 	case PermissionValidatorTypeSelfSubjectAccessReview:
 		permissionValidator = NewSelfSubjectAccessReviewValidator(client.AuthorizationV1().SelfSubjectAccessReviews())
 	case PermissionValidatorTypeSelfSubjectRulesReview:

diff --git a/test/e2e/preflight_permission_validation_escalation_test.go b/test/e2e/preflight_permission_validation_escalation_test.go
@@ -135,3 +135,142 @@ roleRef:
 		NewPresentClusterResource("rolebinding", testName, testName, kubectl)
 	})
 }
+
+func TestPreflightPermissionValidationEscalationSSRR(t *testing.T) {
+	env := BuildEnv(t)
+	logger := Logger{}
+	kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger}
+	kubectl := Kubectl{t, env.Namespace, logger}
+
+	testName := "preflight-permission-validation-escalation-ssrr"
+
+	base := `
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: __test-name__
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: scoped-sa
+  namespace: __ns__
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: scoped-sa
+  namespace: __ns__
+  annotations:
+    kubernetes.io/service-account.name: scoped-sa
+type: kubernetes.io/service-account-token
+---
+kind: ClusterRole
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: __test-name__
+rules:
+- apiGroups: [""]
+  resources: ["configmaps"]
+  verbs: ["*"]
+- apiGroups: [""]
+  resources: ["namespaces"]
+  verbs: ["list"]
+- apiGroups: [""]
+  resources: ["pods"]
+  verbs: ["get", "list"]
+- apiGroups: ["rbac.authorization.k8s.io"]
+  resources: ["roles", "rolebindings", "clusterroles", "clusterrolebindings"]
+  verbs: ["get", "list", "create", "update", "delete", "escalate", "bind"]
+---
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: __test-name__
+subjects:
+- kind: ServiceAccount
+  name: scoped-sa
+  namespace: __ns__
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: __test-name__
+`
+
+	base = strings.ReplaceAll(base, "__test-name__", testName)
+	base = strings.ReplaceAll(base, "__ns__", env.Namespace)
+	baseName := "preflight-permission-validation-base-app"
+	appName := "preflight-permission-validation-app"
+	scopedContext := "scoped-context"
+	scopedUser := "scoped-user"
+
+	cleanUp := func() {
+		kapp.Run([]string{"delete", "-a", baseName})
+		kapp.Run([]string{"delete", "-a", appName})
+		RemoveClusterResource(t, "ns", testName, "", kubectl)
+	}
+	cleanUp()
+	defer cleanUp()
+
+	kapp.RunWithOpts([]string{"deploy", "-a", baseName, "-f", "-"}, RunOpts{StdinReader: strings.NewReader(base)})
+	cleanUpContext := ScopedContext(t, kubectl, testName, scopedContext, scopedUser)
+	defer cleanUpContext()
+
+	roleResource := `
+apiVersion: kapp.k14s.io/v1alpha1
+kind: Config
+preflightRules:
+- name: PermissionValidation
+  config:
+    permissionValidatorResource: SelfSubjectRulesReview
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  namespace: __test-name__
+  name: __test-name__
+rules:
+- apiGroups: [""]
+  resources: ["secrets"]
+  verbs: ["*"]
+`
+
+	roleResource = strings.ReplaceAll(roleResource, "__test-name__", testName)
+	logger.Section("deploy app with privilege escalation Role", func() {
+		kapp.RunWithOpts([]string{"deploy", "--preflight=PermissionValidation", "-a", appName, "-f", "-", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+			RunOpts{StdinReader: strings.NewReader(roleResource)})
+
+		NewPresentClusterResource("role", testName, testName, kubectl)
+	})
+
+	bindingResource := `
+apiVersion: kapp.k14s.io/v1alpha1
+kind: Config
+preflightRules:
+- name: PermissionValidation
+  config:
+    permissionValidatorResource: SelfSubjectRulesReview
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  namespace: __test-name__
+  name: __test-name__
+subjects:
+  - kind: ServiceAccount
+    namespace: __test-name__
+    name: default
+roleRef:
+  kind: ClusterRole
+  name: admin
+  apiGroup: rbac.authorization.k8s.io
+`
+	bindingResource = strings.ReplaceAll(bindingResource, "__test-name__", testName)
+	logger.Section("deploy app with privilege escalation RoleBinding", func() {
+		kapp.RunWithOpts([]string{"deploy", "--preflight=PermissionValidation", "-a", appName, "-f", "-", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+			RunOpts{StdinReader: strings.NewReader(bindingResource)})
+
+		NewPresentClusterResource("rolebinding", testName, testName, kubectl)
+	})
+}
diff --git a/test/e2e/preflight_permission_validation_failed_escalation_test.go b/test/e2e/preflight_permission_validation_failed_escalation_test.go
@@ -114,6 +114,34 @@ rules:
 		NewMissingClusterResource(t, "role", testName, testName, kubectl)
 	})
 
+	roleResourceSSRR := `
+apiVersion: kapp.k14s.io/v1alpha1
+kind: Config
+preflightRules:
+- name: PermissionValidation
+  config:
+    permissionValidatorResource: SelfSubjectRulesReview
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  namespace: __test-name__
+  name: __test-name__
+rules:
+- apiGroups: [""]
+  resources: ["secrets"]
+  verbs: ["*"]
+`
+
+	roleResourceSSRR = strings.ReplaceAll(roleResourceSSRR, "__test-name__", testName)
+	logger.Section("attempt to deploy app with privilege escalation Role without privilege escalation permissions using SSRR validation", func() {
+		_, err := kapp.RunWithOpts([]string{"deploy", "--preflight=PermissionValidation", "-a", appName, "-f", "-", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+			RunOpts{StdinReader: strings.NewReader(roleResourceSSRR), AllowError: true})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "running preflight check \"PermissionValidation\": potential privilege escalation, not permitted to \"create\" rbac.authorization.k8s.io/v1, Kind=Role")
+		NewMissingClusterResource(t, "role", testName, testName, kubectl)
+	})
+
 	bindingResource := `
 ---
 apiVersion: rbac.authorization.k8s.io/v1
@@ -138,4 +166,35 @@ roleRef:
 		require.Contains(t, err.Error(), "running preflight check \"PermissionValidation\": potential privilege escalation, not permitted to \"create\" rbac.authorization.k8s.io/v1, Kind=RoleBinding")
 		NewMissingClusterResource(t, "rolebinding", testName, testName, kubectl)
 	})
+
+	bindingResourceSSRR := `
+apiVersion: kapp.k14s.io/v1alpha1
+kind: Config
+preflightRules:
+- name: PermissionValidation
+  config:
+    permissionValidatorResource: SelfSubjectRulesReview
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  namespace: __test-name__
+  name: __test-name__
+subjects:
+  - kind: ServiceAccount
+    namespace: __test-name__
+    name: default
+roleRef:
+  kind: ClusterRole
+  name: admin
+  apiGroup: rbac.authorization.k8s.io
+`
+	bindingResourceSSRR = strings.ReplaceAll(bindingResourceSSRR, "__test-name__", testName)
+	logger.Section("attempt deploy app with privilege escalation RoleBinding without privilege escalation permissions using SSRR validation", func() {
+		_, err := kapp.RunWithOpts([]string{"deploy", "--preflight=PermissionValidation", "-a", appName, "-f", "-", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+			RunOpts{StdinReader: strings.NewReader(bindingResourceSSRR), AllowError: true})
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "running preflight check \"PermissionValidation\": potential privilege escalation, not permitted to \"create\" rbac.authorization.k8s.io/v1, Kind=RoleBinding")
+		NewMissingClusterResource(t, "rolebinding", testName, testName, kubectl)
+	})
 }
diff --git a/test/e2e/preflight_permission_validation_missing_test.go b/test/e2e/preflight_permission_validation_missing_test.go
@@ -117,6 +117,37 @@ spec:
 		NewMissingClusterResource(t, "pod", testName, testName, kubectl)
 	})
 
+	basicResourceSSRR := `
+apiVersion: kapp.k14s.io/v1alpha1
+kind: Config
+preflightRules:
+- name: PermissionValidation
+  config:
+    permissionValidatorResource: SelfSubjectRulesReview
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: __test-name__
+  namespace: __test-name__
+spec:
+  containers:
+  - name: simple-app
+    image: docker.io/dkalinin/k8s-simple-app@sha256:4c8b96d4fffdfae29258d94a22ae4ad1fe36139d47288b8960d9958d1e63a9d0
+    env:
+    - name: HELLO_MSG
+      value: stranger
+`
+	basicResourceSSRR = strings.ReplaceAll(basicResourceSSRR, "__test-name__", testName)
+	logger.Section("attempt to deploy app with a Pod and missing permissions to create Pods using SSRR for validation", func() {
+		_, err := kapp.RunWithOpts([]string{"deploy", "--preflight=PermissionValidation", "-a", appName, "-f", "-", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+			RunOpts{StdinReader: strings.NewReader(basicResourceSSRR), AllowError: true})
+
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "running preflight check \"PermissionValidation\": not permitted to \"create\" /v1, Resource=pods")
+		NewMissingClusterResource(t, "pod", testName, testName, kubectl)
+	})
+
 	roleResource := `
 ---
 apiVersion: rbac.authorization.k8s.io/v1
@@ -140,6 +171,35 @@ rules:
 		NewMissingClusterResource(t, "role", testName, testName, kubectl)
 	})
 
+	roleResourceSSRR := `
+apiVersion: kapp.k14s.io/v1alpha1
+kind: Config
+preflightRules:
+- name: PermissionValidation
+  config:
+    permissionValidatorResource: SelfSubjectRulesReview
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  namespace: __test-name__
+  name: __test-name__
+rules:
+- apiGroups: [""]
+  resources: ["pods"]
+  verbs: ["create", "update", "delete"]
+`
+
+	roleResourceSSRR = strings.ReplaceAll(roleResourceSSRR, "__test-name__", testName)
+	logger.Section("attempt to deploy app with a Role and missing permissions to create Roles using SSRR for validation", func() {
+		_, err := kapp.RunWithOpts([]string{"deploy", "--preflight=PermissionValidation", "-a", appName, "-f", "-", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+			RunOpts{StdinReader: strings.NewReader(roleResourceSSRR), AllowError: true})
+
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "running preflight check \"PermissionValidation\": not permitted to \"create\" rbac.authorization.k8s.io/v1, Resource=roles")
+		NewMissingClusterResource(t, "role", testName, testName, kubectl)
+	})
+
 	bindingResource := `
 ---
 apiVersion: rbac.authorization.k8s.io/v1
@@ -165,4 +225,36 @@ roleRef:
 		require.Contains(t, err.Error(), "running preflight check \"PermissionValidation\": not permitted to \"create\" rbac.authorization.k8s.io/v1, Resource=rolebindings")
 		NewMissingClusterResource(t, "rolebinding", testName, testName, kubectl)
 	})
+
+	bindingResourceSSRR := `
+apiVersion: kapp.k14s.io/v1alpha1
+kind: Config
+preflightRules:
+- name: PermissionValidation
+  config:
+    permissionValidatorResource: SelfSubjectRulesReview
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  namespace: __test-name__
+  name: __test-name__
+subjects:
+  - kind: ServiceAccount
+    namespace: __test-name__
+    name: default
+roleRef:
+  kind: ClusterRole
+  name: admin
+  apiGroup: rbac.authorization.k8s.io
+`
+	bindingResourceSSRR = strings.ReplaceAll(bindingResourceSSRR, "__test-name__", testName)
+	logger.Section("attempt to deploy app with a RoleBinding and missing permissions to create RoleBindings using SSRR validation", func() {
+		_, err := kapp.RunWithOpts([]string{"deploy", "--preflight=PermissionValidation", "-a", appName, "-f", "-", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+			RunOpts{StdinReader: strings.NewReader(bindingResourceSSRR), AllowError: true})
+
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "running preflight check \"PermissionValidation\": not permitted to \"create\" rbac.authorization.k8s.io/v1, Resource=rolebindings")
+		NewMissingClusterResource(t, "rolebinding", testName, testName, kubectl)
+	})
 }