Skip to content

Commit

Permalink
checking for compatible policy rules before pulling k8s resources; fa…
Browse files Browse the repository at this point in the history
…iling to pull some k8s resource should not fail the entire scan (kubescape#1578)

Signed-off-by: Amir Malka <[email protected]>
  • Loading branch information
amirmalka authored Jan 9, 2024
1 parent 4e4a642 commit 4b8786b
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 38 deletions.
34 changes: 31 additions & 3 deletions core/cautils/datastructuresmethods.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/kubescape/opa-utils/reporthandling"
"github.com/kubescape/opa-utils/reporthandling/apis"
reporthandlingv2 "github.com/kubescape/opa-utils/reporthandling/v2"
)

func NewPolicies() *Policies {
Expand All @@ -14,7 +15,7 @@ func NewPolicies() *Policies {
}
}

func (policies *Policies) Set(frameworks []reporthandling.Framework, version string, excludedRules map[string]bool, scanningScope reporthandling.ScanningScopeType) {
func (policies *Policies) Set(frameworks []reporthandling.Framework, excludedRules map[string]bool, scanningScope reporthandling.ScanningScopeType) {
for i := range frameworks {
if !isFrameworkFitToScanScope(frameworks[i], scanningScope) {
continue
Expand All @@ -32,9 +33,12 @@ func (policies *Policies) Set(frameworks []reporthandling.Framework, version str
}
}

if isRuleKubescapeVersionCompatible(frameworks[i].Controls[j].Rules[r].Attributes, version) && isControlFitToScanScope(frameworks[i].Controls[j], scanningScope) {
compatibleRules = append(compatibleRules, frameworks[i].Controls[j].Rules[r])
if ShouldSkipRule(frameworks[i].Controls[j], frameworks[i].Controls[j].Rules[r], scanningScope) {
continue
}
// if isRuleKubescapeVersionCompatible(frameworks[i].Controls[j].Rules[r].Attributes, version) && isControlFitToScanScope(frameworks[i].Controls[j], scanningScope) {
compatibleRules = append(compatibleRules, frameworks[i].Controls[j].Rules[r])
// }
}
if len(compatibleRules) > 0 {
frameworks[i].Controls[j].Rules = compatibleRules
Expand All @@ -54,6 +58,20 @@ func (policies *Policies) Set(frameworks []reporthandling.Framework, version str
}
}

// ShouldSkipRule checks if the rule should be skipped
// It checks the following:
// 1. Rule is compatible with the current kubescape version
// 2. Rule fits the current scanning scope
func ShouldSkipRule(control reporthandling.Control, rule reporthandling.PolicyRule, scanningScope reporthandling.ScanningScopeType) bool {
if !isRuleKubescapeVersionCompatible(rule.Attributes, BuildNumber) {
return true
}
if !isControlFitToScanScope(control, scanningScope) {
return true
}
return false
}

// Checks that kubescape version is in range of use for this rule
// In local build (BuildNumber = ""):
// returns true only if rule doesn't have the "until" attribute
Expand Down Expand Up @@ -135,3 +153,13 @@ func isFrameworkFitToScanScope(framework reporthandling.Framework, scanScopeMatc
}
return false
}

func GetScanningScope(ContextMetadata reporthandlingv2.ContextMetadata) reporthandling.ScanningScopeType {
if ContextMetadata.ClusterContextMetadata != nil {
if ContextMetadata.ClusterContextMetadata.CloudMetadata != nil && ContextMetadata.ClusterContextMetadata.CloudMetadata.CloudProvider != "" {
return reporthandling.ScanningScopeType(ContextMetadata.ClusterContextMetadata.CloudMetadata.CloudProvider)
}
return reporthandling.ScopeCluster
}
return reporthandling.ScopeFile
}
4 changes: 2 additions & 2 deletions core/pkg/opaprocessor/processorhandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func NewOPAProcessor(sessionObj *cautils.OPASessionObj, regoDependenciesData *re
}

func (opap *OPAProcessor) ProcessRulesListener(ctx context.Context, progressListener IJobProgressNotificationClient) error {
scanningScope := getScanningScope(opap.Metadata.ContextMetadata)
opap.OPASessionObj.AllPolicies = convertFrameworksToPolicies(opap.Policies, cautils.BuildNumber, opap.ExcludedRules, scanningScope)
scanningScope := cautils.GetScanningScope(opap.Metadata.ContextMetadata)
opap.OPASessionObj.AllPolicies = convertFrameworksToPolicies(opap.Policies, opap.ExcludedRules, scanningScope)

ConvertFrameworksToSummaryDetails(&opap.Report.SummaryDetails, opap.Policies, opap.OPASessionObj.AllPolicies)

Expand Down
12 changes: 5 additions & 7 deletions core/pkg/opaprocessor/processorhandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func TestProcessResourcesResult(t *testing.T) {
opaSessionObj.Policies = frameworks

scanningScope := reporthandling.ScopeCluster
policies := convertFrameworksToPolicies(opaSessionObj.Policies, "", nil, scanningScope)
policies := convertFrameworksToPolicies(opaSessionObj.Policies, nil, scanningScope)
ConvertFrameworksToSummaryDetails(&opaSessionObj.Report.SummaryDetails, opaSessionObj.Policies, policies)

opaSessionObj.K8SResources = k8sResources
Expand Down Expand Up @@ -270,9 +270,8 @@ func TestProcessRule(t *testing.T) {
name: "TestRelatedResourcesIDs",
rule: reporthandling.PolicyRule{
PortalBase: armotypes.PortalBase{
Name: "exposure-to-internet",
Attributes: map[string]interface{}{
},
Name: "exposure-to-internet",
Attributes: map[string]interface{}{},
},
Rule: "package armo_builtins\n\n# Checks if NodePort or LoadBalancer is connected to a workload to expose something\ndeny[msga] {\n service := input[_]\n service.kind == \"Service\"\n is_exposed_service(service)\n \n wl := input[_]\n spec_template_spec_patterns := {\"Deployment\", \"ReplicaSet\", \"DaemonSet\", \"StatefulSet\", \"Pod\", \"Job\", \"CronJob\"}\n spec_template_spec_patterns[wl.kind]\n wl_connected_to_service(wl, service)\n failPath := [\"spec.type\"]\n msga := {\n \"alertMessage\": sprintf(\"workload '%v' is exposed through service '%v'\", [wl.metadata.name, service.metadata.name]),\n \"packagename\": \"armo_builtins\",\n \"alertScore\": 7,\n \"fixPaths\": [],\n \"failedPaths\": [],\n \"alertObject\": {\n \"k8sApiObjects\": [wl]\n },\n \"relatedObjects\": [{\n \"object\": service,\n \"failedPaths\": failPath,\n \"reviewPaths\": failPath,\n }]\n }\n}\n\n# Checks if Ingress is connected to a service and a workload to expose something\ndeny[msga] {\n ingress := input[_]\n ingress.kind == \"Ingress\"\n \n svc := input[_]\n svc.kind == \"Service\"\n # avoid duplicate alerts\n # if service is already exposed through NodePort or LoadBalancer workload will fail on that\n not is_exposed_service(svc)\n\n wl := input[_]\n spec_template_spec_patterns := {\"Deployment\", \"ReplicaSet\", \"DaemonSet\", \"StatefulSet\", \"Pod\", \"Job\", \"CronJob\"}\n spec_template_spec_patterns[wl.kind]\n wl_connected_to_service(wl, svc)\n\n result := svc_connected_to_ingress(svc, ingress)\n \n msga := {\n \"alertMessage\": sprintf(\"workload '%v' is exposed through ingress '%v'\", [wl.metadata.name, ingress.metadata.name]),\n \"packagename\": \"armo_builtins\",\n \"failedPaths\": [],\n \"fixPaths\": [],\n \"alertScore\": 7,\n \"alertObject\": {\n \"k8sApiObjects\": [wl]\n },\n \"relatedObjects\": [{\n \"object\": ingress,\n \"failedPaths\": result,\n \"reviewPaths\": result,\n }]\n }\n} \n\n# ====================================================================================\n\nis_exposed_service(svc) {\n svc.spec.type == \"NodePort\"\n}\n\nis_exposed_service(svc) {\n svc.spec.type == \"LoadBalancer\"\n}\n\nwl_connected_to_service(wl, svc) {\n count({x | svc.spec.selector[x] == wl.metadata.labels[x]}) == count(svc.spec.selector)\n}\n\nwl_connected_to_service(wl, svc) {\n wl.spec.selector.matchLabels == svc.spec.selector\n}\n\n# check if service is connected to ingress\nsvc_connected_to_ingress(svc, ingress) = result {\n rule := ingress.spec.rules[i]\n paths := rule.http.paths[j]\n svc.metadata.name == paths.backend.service.name\n result := [sprintf(\"ingress.spec.rules[%d].http.paths[%d].backend.service.name\", [i,j])]\n}\n\n",
Match: []reporthandling.RuleMatchObjects{
Expand Down Expand Up @@ -334,9 +333,8 @@ func TestProcessRule(t *testing.T) {
name: "TestMultipleRelatedResources",
rule: reporthandling.PolicyRule{
PortalBase: armotypes.PortalBase{
Name: "exposure-to-internet",
Attributes: map[string]interface{}{
},
Name: "exposure-to-internet",
Attributes: map[string]interface{}{},
},
Rule: "\npackage armo_builtins\n\n# Checks if NodePort or LoadBalancer is connected to a workload to expose something\ndeny[msga] {\n service := input[_]\n service.kind == \"Service\"\n is_exposed_service(service)\n \n wl := input[_]\n spec_template_spec_patterns := {\"Deployment\", \"ReplicaSet\", \"DaemonSet\", \"StatefulSet\", \"Pod\", \"Job\", \"CronJob\"}\n spec_template_spec_patterns[wl.kind]\n wl_connected_to_service(wl, service)\n failPath := [\"spec.type\"]\n msga := {\n \"alertMessage\": sprintf(\"workload '%v' is exposed through service '%v'\", [wl.metadata.name, service.metadata.name]),\n \"packagename\": \"armo_builtins\",\n \"alertScore\": 7,\n \"fixPaths\": [],\n \"failedPaths\": [],\n \"alertObject\": {\n \"k8sApiObjects\": [wl]\n },\n \"relatedObjects\": [{\n \"object\": service,\n\t\t \"reviewPaths\": failPath,\n \"failedPaths\": failPath,\n }]\n }\n}\n\n# Checks if Ingress is connected to a service and a workload to expose something\ndeny[msga] {\n ingress := input[_]\n ingress.kind == \"Ingress\"\n \n svc := input[_]\n svc.kind == \"Service\"\n\n # Make sure that they belong to the same namespace\n svc.metadata.namespace == ingress.metadata.namespace\n\n # avoid duplicate alerts\n # if service is already exposed through NodePort or LoadBalancer workload will fail on that\n not is_exposed_service(svc)\n\n wl := input[_]\n spec_template_spec_patterns := {\"Deployment\", \"ReplicaSet\", \"DaemonSet\", \"StatefulSet\", \"Pod\", \"Job\", \"CronJob\"}\n spec_template_spec_patterns[wl.kind]\n wl_connected_to_service(wl, svc)\n\n result := svc_connected_to_ingress(svc, ingress)\n \n msga := {\n \"alertMessage\": sprintf(\"workload '%v' is exposed through ingress '%v'\", [wl.metadata.name, ingress.metadata.name]),\n \"packagename\": \"armo_builtins\",\n \"failedPaths\": [],\n \"fixPaths\": [],\n \"alertScore\": 7,\n \"alertObject\": {\n \"k8sApiObjects\": [wl]\n },\n \"relatedObjects\": [\n\t\t{\n\t \"object\": ingress,\n\t\t \"reviewPaths\": result,\n\t \"failedPaths\": result,\n\t },\n\t\t{\n\t \"object\": svc,\n\t\t}\n ]\n }\n} \n\n# ====================================================================================\n\nis_exposed_service(svc) {\n svc.spec.type == \"NodePort\"\n}\n\nis_exposed_service(svc) {\n svc.spec.type == \"LoadBalancer\"\n}\n\nwl_connected_to_service(wl, svc) {\n count({x | svc.spec.selector[x] == wl.metadata.labels[x]}) == count(svc.spec.selector)\n}\n\nwl_connected_to_service(wl, svc) {\n wl.spec.selector.matchLabels == svc.spec.selector\n}\n\n# check if service is connected to ingress\nsvc_connected_to_ingress(svc, ingress) = result {\n rule := ingress.spec.rules[i]\n paths := rule.http.paths[j]\n svc.metadata.name == paths.backend.service.name\n result := [sprintf(\"spec.rules[%d].http.paths[%d].backend.service.name\", [i,j])]\n}\n\n",
Match: []reporthandling.RuleMatchObjects{
Expand Down
16 changes: 2 additions & 14 deletions core/pkg/opaprocessor/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package opaprocessor
import (
"fmt"

reporthandlingv2 "github.com/kubescape/opa-utils/reporthandling/v2"

logger "github.com/kubescape/go-logger"
"github.com/kubescape/go-logger/helpers"
"github.com/kubescape/kubescape/v3/core/cautils"
Expand All @@ -19,9 +17,9 @@ import (
)

// convertFrameworksToPolicies convert list of frameworks to list of policies
func convertFrameworksToPolicies(frameworks []reporthandling.Framework, version string, excludedRules map[string]bool, scanningScope reporthandling.ScanningScopeType) *cautils.Policies {
func convertFrameworksToPolicies(frameworks []reporthandling.Framework, excludedRules map[string]bool, scanningScope reporthandling.ScanningScopeType) *cautils.Policies {
policies := cautils.NewPolicies()
policies.Set(frameworks, version, excludedRules, scanningScope)
policies.Set(frameworks, excludedRules, scanningScope)
return policies
}

Expand Down Expand Up @@ -111,13 +109,3 @@ var imageNameNormalizeDefinition = func(bctx rego.BuiltinContext, a *ast.Term) (
normalizedName, err := cautils.NormalizeImageName(string(aStr))
return ast.StringTerm(normalizedName), err
}

func getScanningScope(ContextMetadata reporthandlingv2.ContextMetadata) reporthandling.ScanningScopeType {
if ContextMetadata.ClusterContextMetadata != nil {
if ContextMetadata.ClusterContextMetadata.CloudMetadata != nil && ContextMetadata.ClusterContextMetadata.CloudMetadata.CloudProvider != "" {
return reporthandling.ScanningScopeType(ContextMetadata.ClusterContextMetadata.CloudMetadata.CloudProvider)
}
return reporthandling.ScopeCluster
}
return reporthandling.ScopeFile
}
13 changes: 7 additions & 6 deletions core/pkg/opaprocessor/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,18 @@ import (

"github.com/stretchr/testify/assert"

"github.com/kubescape/kubescape/v3/core/cautils"
"github.com/kubescape/kubescape/v3/core/mocks"
"github.com/kubescape/opa-utils/reporthandling"
"github.com/kubescape/opa-utils/reporthandling/results/v1/reportsummary"
v2 "github.com/kubescape/opa-utils/reporthandling/v2"
)

func TestconvertFrameworksToPolicies(t *testing.T) {
func TestConvertFrameworksToPolicies(t *testing.T) {
fw0 := mocks.MockFramework_0006_0013()
fw1 := mocks.MockFramework_0044()
scanningScope := getScanningScope(v2.ContextMetadata{ClusterContextMetadata: &v2.ClusterMetadata{}})
policies := convertFrameworksToPolicies([]reporthandling.Framework{*fw0, *fw1}, "", nil, scanningScope)
scanningScope := cautils.GetScanningScope(v2.ContextMetadata{ClusterContextMetadata: &v2.ClusterMetadata{}})
policies := convertFrameworksToPolicies([]reporthandling.Framework{*fw0, *fw1}, nil, scanningScope)
assert.Equal(t, 2, len(policies.Frameworks))
assert.Equal(t, 3, len(policies.Controls))

Expand All @@ -25,19 +26,19 @@ func TestconvertFrameworksToPolicies(t *testing.T) {
}
fw0 = mocks.MockFramework_0006_0013()
fw1 = mocks.MockFramework_0044()
policies = convertFrameworksToPolicies([]reporthandling.Framework{*fw0, *fw1}, "", excludedRulesMap, scanningScope)
policies = convertFrameworksToPolicies([]reporthandling.Framework{*fw0, *fw1}, excludedRulesMap, scanningScope)
assert.Equal(t, 2, len(policies.Frameworks))
assert.Equal(t, 2, len(policies.Controls))

}
func TestInitializeSummaryDetails(t *testing.T) {
fw0 := mocks.MockFramework_0006_0013()
fw1 := mocks.MockFramework_0044()
scanningScope := getScanningScope(v2.ContextMetadata{ClusterContextMetadata: &v2.ClusterMetadata{}})
scanningScope := cautils.GetScanningScope(v2.ContextMetadata{ClusterContextMetadata: &v2.ClusterMetadata{}})

summaryDetails := reportsummary.SummaryDetails{}
frameworks := []reporthandling.Framework{*fw0, *fw1}
policies := convertFrameworksToPolicies([]reporthandling.Framework{*fw0, *fw1}, "", nil, scanningScope)
policies := convertFrameworksToPolicies([]reporthandling.Framework{*fw0, *fw1}, nil, scanningScope)
ConvertFrameworksToSummaryDetails(&summaryDetails, frameworks, policies)
assert.Equal(t, 2, len(summaryDetails.Frameworks))
// assert.Equal(t, 3, len(summaryDetails.Controls))
Expand Down
4 changes: 3 additions & 1 deletion core/pkg/resourcehandler/filesloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,11 @@ func (fileHandler *FileResourceHandler) GetResources(ctx context.Context, sessio
return nil, nil, nil, nil, fmt.Errorf("resource %s has a parent and cannot be scanned", sessionObj.SingleResourceScan.GetID())
}

scanningScope := cautils.GetScanningScope(sessionObj.Metadata.ContextMetadata)

// build a resources map, based on the policies
// map resources based on framework required resources: map["/group/version/kind"][]<k8s workloads ids>
resourceToQuery, excludedRulesMap := getQueryableResourceMapFromPolicies(sessionObj.Policies, sessionObj.SingleResourceScan)
resourceToQuery, excludedRulesMap := getQueryableResourceMapFromPolicies(sessionObj.Policies, sessionObj.SingleResourceScan, scanningScope)
k8sResources := resourceToQuery.ToK8sResourceMap()

// save only relevant resources
Expand Down
12 changes: 11 additions & 1 deletion core/pkg/resourcehandler/k8sresources.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,12 @@ func (k8sHandler *K8sResourceHandler) GetResources(ctx context.Context, sessionO
return nil, nil, nil, nil, err
}

scanningScope := cautils.GetScanningScope(sessionObj.Metadata.ContextMetadata)

resourceToControl := make(map[string][]string)
// build resources map
// map resources based on framework required resources: map["/group/version/kind"][]<k8s workloads ids>
queryableResources, excludedRulesMap := getQueryableResourceMapFromPolicies(sessionObj.Policies, sessionObj.SingleResourceScan)
queryableResources, excludedRulesMap := getQueryableResourceMapFromPolicies(sessionObj.Policies, sessionObj.SingleResourceScan, scanningScope)
ksResourceMap := setKSResourceMap(sessionObj.Policies, resourceToControl)

// map of Kubescape resources to control_ids
Expand Down Expand Up @@ -320,6 +322,7 @@ func (k8sHandler *K8sResourceHandler) pullResources(queryableResources Queryable
result, err := k8sHandler.pullSingleResource(&gvr, nil, queryableResources[i].FieldSelectors, globalFieldSelectors)
if err != nil {
if !strings.Contains(err.Error(), "the server could not find the requested resource") {
logger.L().Error("failed to pull resource", helpers.String("resource", queryableResources[i].GroupVersionResourceTriplet), helpers.Error(err))
// handle error
if errs == nil {
errs = err
Expand All @@ -342,6 +345,13 @@ func (k8sHandler *K8sResourceHandler) pullResources(queryableResources Queryable
k8sResources[key] = append(k8sResources[key], workloadinterface.ListMetaIDs(metaObjs)...)
}
}

// we don't want to fail the scan if we failed to pull only some resources
// in that case, we return nil error (and errors are logged in the loop above)
if errs != nil && len(allResources) > 0 {
errs = nil
}

return k8sResources, allResources, errs
}

Expand Down
11 changes: 8 additions & 3 deletions core/pkg/resourcehandler/resourcehandlerutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,19 @@ func addSingleResourceToResourceMaps(k8sResources cautils.K8SResources, allResou
k8sResources[resourceGroup] = append(k8sResources[resourceGroup], wl.GetID())
}

func getQueryableResourceMapFromPolicies(frameworks []reporthandling.Framework, resource workloadinterface.IWorkload) (QueryableResources, map[string]bool) {
func getQueryableResourceMapFromPolicies(frameworks []reporthandling.Framework, resource workloadinterface.IWorkload, scanningScope reporthandling.ScanningScopeType) (QueryableResources, map[string]bool) {
queryableResources := make(QueryableResources)
excludedRulesMap := make(map[string]bool)
namespace := getScannedResourceNamespace(resource)

for _, framework := range frameworks {
for _, control := range framework.Controls {
for _, rule := range control.Rules {
// check if the rule should be skipped according to the scanning scope and the rule attributes
if cautils.ShouldSkipRule(control, rule, scanningScope) {
continue
}

var resourcesFilterMap map[string]bool = nil
// for single resource scan, we need to filter the rules and which resources to query according to the given resource
if resource != nil {
Expand All @@ -39,8 +44,8 @@ func getQueryableResourceMapFromPolicies(frameworks []reporthandling.Framework,
continue
}
}
for _, match := range rule.Match {
updateQueryableResourcesMapFromRuleMatchObject(&match, resourcesFilterMap, queryableResources, namespace)
for i := range rule.Match {
updateQueryableResourcesMapFromRuleMatchObject(&rule.Match[i], resourcesFilterMap, queryableResources, namespace)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/pkg/resourcehandler/resourcehandlerutils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func TestGetQueryableResourceMapFromPolicies(t *testing.T) {
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
resourceGroups, excludedRulesMap := getQueryableResourceMapFromPolicies([]reporthandling.Framework{*mockFramework("test", testCase.controls)}, testCase.workload) // TODO check second param
resourceGroups, excludedRulesMap := getQueryableResourceMapFromPolicies([]reporthandling.Framework{*mockFramework("test", testCase.controls)}, testCase.workload, reporthandling.ScopeCluster) // TODO check second param
assert.Equalf(t, len(testCase.expectedExcludedRules), len(excludedRulesMap), "excludedRulesMap length is not as expected")
for _, expectedExcludedRuleName := range testCase.expectedExcludedRules {
assert.Contains(t, excludedRulesMap, expectedExcludedRuleName, "excludedRulesMap does not contain expected rule name")
Expand Down

0 comments on commit 4b8786b

Please sign in to comment.