Bug fix: perform OR across types in UserInfo (#992)

* remove policy name cache entry on policy DELETE

* buugfix: perform OR in userInfo match

* add function description

go.mod

@@ -18,7 +18,7 @@ require (
 	github.com/julienschmidt/httprouter v1.3.0
 	github.com/minio/minio v0.0.0-20200114012931-30922148fbb5
 	github.com/ory/go-acc v0.2.1 // indirect
-	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pkg/errors v0.9.1
 	github.com/prometheus/common v0.4.1
 	github.com/rogpeppe/godef v1.1.2 // indirect
 	github.com/spf13/cobra v0.0.5

pkg/engine/utils.go

@@ -65,6 +65,21 @@ func checkSelector(labelSelector *metav1.LabelSelector, resourceLabels map[strin
 	return false, nil
 }
 
+// doesResourceMatchConditionBlock filters the resource with defined conditions
+// for a match / exclude block, it has the following attributes:
+// ResourceDescription:
+// 		Kinds      []string
+// 		Name       string
+// 		Namespaces []string
+// 		Selector
+// UserInfo:
+// 		Roles        []string
+// 		ClusterRoles []string
+// 		Subjects     []rbacv1.Subject
+// To filter out the targeted resources with ResourceDescription, the check
+// should be: AND across attibutes but an OR inside attributes that of type list
+// To filter out the targeted resources with UserInfo, the check
+// should be: OR (accross & inside) attributes
 func doesResourceMatchConditionBlock(conditionBlock kyverno.ResourceDescription, userInfo kyverno.UserInfo, admissionInfo kyverno.RequestInfo, resource unstructured.Unstructured) []error {
 	var errs []error
 	if len(conditionBlock.Kinds) > 0 {
@@ -94,26 +109,43 @@ func doesResourceMatchConditionBlock(conditionBlock kyverno.ResourceDescription,
 	}
 
 	keys := append(admissionInfo.AdmissionUserInfo.Groups, admissionInfo.AdmissionUserInfo.Username)
+	var userInfoErrors []error
+	var checkedItem int
+	if len(userInfo.Roles) > 0 && !utils.SliceContains(keys, ExcludeUserInfo...) {
+		checkedItem++
 
-	if len(userInfo.Roles) > 0 &&
-		!utils.SliceContains(keys, ExcludeUserInfo...) {
 		if !utils.SliceContains(userInfo.Roles, admissionInfo.Roles...) {
-			errs = append(errs, fmt.Errorf("user info does not match roles for the given conditionBlock"))
-		}
-	}
-	if len(userInfo.ClusterRoles) > 0 &&
-		!utils.SliceContains(keys, ExcludeUserInfo...) {
-		if !utils.SliceContains(userInfo.ClusterRoles, admissionInfo.ClusterRoles...) {
-			errs = append(errs, fmt.Errorf("user info does not match clustersRoles for the given conditionBlock"))
-		}
-	}
-	if len(userInfo.Subjects) > 0 {
-		if !matchSubjects(userInfo.Subjects, admissionInfo.AdmissionUserInfo) {
-			errs = append(errs, fmt.Errorf("user info does not match subject for the given conditionBlock"))
+			userInfoErrors = append(userInfoErrors, fmt.Errorf("user info does not match roles for the given conditionBlock"))
+		} else {
+			return errs
 		}
 	}
 
-	return errs
+	if len(userInfo.ClusterRoles) > 0 && !utils.SliceContains(keys, ExcludeUserInfo...) {
+		checkedItem++
+
+		if !utils.SliceContains(userInfo.ClusterRoles, admissionInfo.ClusterRoles...) {
+			userInfoErrors = append(userInfoErrors, fmt.Errorf("user info does not match clustersRoles for the given conditionBlock"))
+		} else {
+			return errs
+		}
+	}
+
+	if len(userInfo.Subjects) > 0 {
+		checkedItem++
+
+		if !matchSubjects(userInfo.Subjects, admissionInfo.AdmissionUserInfo) {
+			userInfoErrors = append(userInfoErrors, fmt.Errorf("user info does not match subject for the given conditionBlock"))
+		} else {
+			return errs
+		}
+	}
+
+	if checkedItem != len(userInfoErrors) {
+		return errs
+	}
+
+	return append(errs, userInfoErrors...)
 }
 
 // matchSubjects return true if one of ruleSubjects exist in userInfo

pkg/engine/validation.go

@@ -50,6 +50,7 @@ func Validate(policyContext PolicyContext) (resp response.EngineResponse) {
 		for i := range resp.PolicyResponse.Rules {
 			messageInterface, err := variables.SubstituteVars(logger, ctx, resp.PolicyResponse.Rules[i].Message)
 			if err != nil {
+				logger.V(4).Info("failed to substitue JMES value", "error", err.Error())
 				continue
 			}
 			resp.PolicyResponse.Rules[i].Message, _ = messageInterface.(string)

pkg/webhooks/common.go

@@ -30,11 +30,11 @@ func isResponseSuccesful(engineReponses []response.EngineResponse) bool {
 func toBlockResource(engineReponses []response.EngineResponse, log logr.Logger) bool {
 	for _, er := range engineReponses {
 		if !er.IsSuccessful() && er.PolicyResponse.ValidationFailureAction == common.Enforce {
-			log.Info("spec.ValidationFailureAction set to enforcel blocking resource request", "policy", er.PolicyResponse.Policy)
+			log.Info("spec.ValidationFailureAction set to enforce blocking resource request", "policy", er.PolicyResponse.Policy)
 			return true
 		}
 	}
-	log.V(4).Info("sepc.ValidationFailureAction set to auit for all applicable policies, won't block resource operation")
+	log.V(4).Info("sepc.ValidationFailureAction set to audit for all applicable policies, won't block resource operation")
 	return false
 }
 

pkg/webhooks/server.go

@@ -438,6 +438,8 @@ func (ws *WebhookServer) resourceValidation(request *v1beta1.AdmissionRequest) *
 				},
 			}
 		}
+		logger = logger.WithValues("username", request.UserInfo.Username,
+			"groups", request.UserInfo.Groups, "roles", roles, "clusterRoles", clusterRoles)
 	}
 
 	userRequestInfo := v1.RequestInfo{
@@ -562,7 +564,7 @@ func (ws *WebhookServer) excludeKyvernoResources(request *v1beta1.AdmissionReque
 	if request.Operation == v1beta1.Delete {
 		resource, err = enginutils.ConvertToUnstructured(request.OldObject.Raw)
 		isManagedResourceCheck = true
-	} else if request.Operation ==  v1beta1.Update  {
+	} else if request.Operation == v1beta1.Update {
 		resource, err = enginutils.ConvertToUnstructured(request.Object.Raw)
 		isManagedResourceCheck = true
 	}
@@ -577,7 +579,7 @@ func (ws *WebhookServer) excludeKyvernoResources(request *v1beta1.AdmissionReque
 			if labels["app.kubernetes.io/managed-by"] == "kyverno" && labels["app.kubernetes.io/synchronize"] == "enable" {
 				isAuthorized, err := userinfo.IsRoleAuthorize(ws.rbLister, ws.crbLister, ws.rLister, ws.crLister, request)
 				if err != nil {
-					return fmt.Errorf("failed to get RBAC infromation for request %v",err)
+					return fmt.Errorf("failed to get RBAC infromation for request %v", err)
 				}
 				if !isAuthorized {
 					// convert RAW to unstructured