Skip to content

Commit

Permalink
feat: remove unused predicate
Browse files Browse the repository at this point in the history
  • Loading branch information
chen-keinan committed Nov 25, 2021
1 parent 7717565 commit 1b2b51b
Show file tree
Hide file tree
Showing 17 changed files with 39 additions and 81 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ Execute mesh-kridik with flags , execute test on demand
Usage: mesh-kridik [--version] [--help] <command> [<args>]

Available commands are:
-r , --report : run audit tests and generate remediation report
-r , --report : run security checks and generate remediation report
-i , --include: execute only specific security check, example -i=1.1
-e , --exclude: ignore specific security check, example -e=1.1,2.0
```

Execute tests and generate failure tests report and it remediation's
Expand Down
6 changes: 2 additions & 4 deletions internal/cli/commands/command-helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,9 @@ func buildPredicateChain(args []string) []filters.Predicate {
for _, n := range args {
switch {
case strings.HasPrefix(n, common.IncludeParam):
pc = append(pc, filters.IncludeAudit)
pc = append(pc, filters.IncludeCheck)
case strings.HasPrefix(n, common.ExcludeParam):
pc = append(pc, filters.ExcludeAudit)
case strings.HasPrefix(n, common.NodeParam):
pc = append(pc, filters.NodeAudit)
pc = append(pc, filters.ExcludeCheck)
case n == "a":
pc = append(pc, filters.Basic)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/cli/commands/command-helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func Test_LoadSecurityCheck(t *testing.T) {
//Test_FilterAuditTests test
func Test_FilterAuditTests(t *testing.T) {
at := &models.SubCategory{Checks: []*models.SecurityCheck{{Name: "1.2.1 aaa"}, {Name: "2.2.2"}}}
fab := FilterAuditTests([]filters.Predicate{filters.IncludeAudit}, []string{"1.2.1"}, at)
fab := FilterAuditTests([]filters.Predicate{filters.IncludeCheck}, []string{"1.2.1"}, at)
assert.Equal(t, fab.Checks[0].Name, "1.2.1 aaa")
assert.True(t, len(fab.Checks) == 1)
}
Expand All @@ -131,7 +131,7 @@ func Test_buildPredicateChainParams(t *testing.T) {

func Test_filteredAuditBenchTests(t *testing.T) {
asc := []*models.SubCategory{{Checks: []*models.SecurityCheck{{Name: "1.1.0 bbb"}}}}
fp := []filters.Predicate{filters.IncludeAudit, filters.ExcludeAudit}
fp := []filters.Predicate{filters.IncludeCheck, filters.ExcludeCheck}
st := []string{"i=1.1.0", "e=1.1.0"}
fr := filteredAuditBenchTests(asc, fp, st)
assert.True(t, len(fr) == 0)
Expand Down
7 changes: 3 additions & 4 deletions internal/cli/commands/help/synopsis
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
Usage: %s [--version] [--help] <command> [<args>]

Available commands are:
-r , --report : run audit tests and generate failure/warn report
-i , --include: execute only specific security check, example -i=1.2.3,1.4.5
-e , --exclude, ignore specific audit tests, example -e=1.2.3,1.4.5
-c , --classic: test report in classic view
-r , --report : run security checks and generate remediation report
-i , --include: execute only specific security check, example -i=1.1
-e , --exclude: ignore specific security check, example -e=1.1,2.0
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: Understand traffic capture limitations
security_checks:
-
name: 'Securing egress traffic'
name: '10.0 Securing egress traffic'
description: 'Istio has an installation option, meshConfig.outboundTrafficPolicy.mode,
that configures the sidecar handling of external services, that is,
those services that are not defined in Istio’s internal service registry.'
Expand Down
4 changes: 2 additions & 2 deletions internal/security/mesh/istio/1_istio_mutual_mtls.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: Istio Mutual mTLS
security_checks:
-
name: 'Make sure mTLS is not configured in permissive mode for each namespace'
name: '1.0 Make sure mTLS is not configured in permissive mode for each namespace'
description: Istio will automatically encrypt traffic using Mutual TLS whenever possible. However, proxies are configured in permissive mode by default, meaning they will accept both mutual TLS and plaintext traffic.
check_command:
- kubectl get namespaces -o=custom-columns=NAME:.metadata.name | awk '{if(NR>1)print}'
Expand All @@ -21,7 +21,7 @@ categories:
eval_message: 'check mTLS STRICT mode for namespace: $namespace'
references:
- https://istio.io/latest/docs/tasks/security/authentication/mtls-migration/
- name: 'Make sure mTLS is not configured in permissive mode for all cluster'
- name: '1.1 Make sure mTLS is not configured in permissive mode for all cluster'
description: Istio will automatically encrypt traffic using Mutual TLS whenever possible. However, proxies are configured in permissive mode by default, meaning they will accept both mutual TLS and plaintext traffic.
check_command:
- 'kubectl get peerauthentication -n istio-system -o json 2> /dev/null | jq ''. |= . + {"namespace": "istio-system"}'''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: Istio Safer Authorization Policy Patterns
security_checks:
-
name: 'Use ALLOW-with-positive-matching or DENY-with-negative-match patterns'
name: '2.0 Use ALLOW-with-positive-matching or DENY-with-negative-match patterns'
description: 'The ALLOW-with-positive-matching pattern is to use the ALLOW action only with positive matching fields (e.g. paths, values)
and do not use any of the negative matching fields (e.g. notPaths, notValues).
The DENY-with-negative-matching pattern is to use the DENY action only with negative matching
Expand All @@ -28,7 +28,7 @@ categories:
references:
- https://istio.io/latest/docs/ops/best-practices/security/#use-allow-with-positive-matching-and-deny-with-negative-match-patterns
-
name: 'path normalization in authorization policy'
name: '2.1 path normalization in authorization policy'
description: 'The enforcement point for authorization policies is the Envoy proxy instead of the usual resource access point in the backend application.
A policy mismatch happens when the Envoy proxy and the backend application interpret the request differently.'
check_command:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: TLS origination for egress traffic
security_checks:
-
name: 'Use of DestinationRule on service ServiceEntry for egress traffic'
name: '3.0 Use of DestinationRule on service ServiceEntry for egress traffic'
description: 'DestinationRule will perform TLS origination for HTTP requests on port 80 and the ServiceEntry will then redirect the requests on port 80 to target port 443.'
check_command:
- 'kubectl get ServiceEntry --all-namespaces -o=custom-columns=NAME:.metadata.name | awk ''{if(NR>1)print}'''
Expand All @@ -20,4 +20,4 @@ categories:
default_value: 'By default, DestinationRule for outside traffic is no configure'
eval_message: 'Use of DestinationRule on service ServiceEntry: $serviceEntry for egress traffic'
references:
- https://istio.io/latest/docs/tasks/traffic-management/egress/egress-control/
- https://istio.io/latest/docs/tasks/traffic-management/egress/egress-control/
4 changes: 2 additions & 2 deletions internal/security/mesh/istio/4_protocol_detection.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: Protocol detection
security_checks:
-
name: 'explicitly declare the service protocol'
name: '4.0 explicitly declare the service protocol'
description: 'Istio will automatically determine the protocol of traffic, to avoid accidental or intentional miss detection,it is recommended to explicitly declare the protocol where possible.'
check_command:
- 'kubectl get service --all-namespaces -o=custom-columns="NAME:.metadata.name,NAMESPACE:.metadata.namespace" | awk ''{if(NR>1)print}'' |awk '' { print $1 " -n " $NF } '''
Expand All @@ -19,4 +19,4 @@ categories:
default_value: 'By default, services are do not configure appProtocol field'
eval_message: 'Service $service on namespace $namespace has declared protocol explicitly '
references:
- https://istio.io/latest/docs/ops/configuration/traffic-management/protocol-selection/#explicit-protocol-selection
- https://istio.io/latest/docs/ops/configuration/traffic-management/protocol-selection/#explicit-protocol-selection
2 changes: 1 addition & 1 deletion internal/security/mesh/istio/5_cni.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: CNI
security_checks:
-
name: 'istio transparent traffic capture'
name: '5.0 istio transparent traffic capture'
description: 'In order to transparently capture all traffic, Istio relies on iptables rules configured by the istio-init initContainer. This adds a requirement for the NET_ADMIN and NET_RAW capabilities to be available to the pod'
check_command:
- 'kubectl get pods --all-namespaces -o=custom-columns="NAME:.metadata.name,NAMESPACE:.metadata.namespace" | awk ''{if(NR>1)print}'' |awk '' { print $1 " -n " $NF } '''
Expand Down
6 changes: 3 additions & 3 deletions internal/security/mesh/istio/6_gateway.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: Gateways
security_checks:
-
name: 'Restrict Gateway creation privileges'
name: '6.0 Restrict Gateway creation privileges'
description: 'restrict creation of Gateway resources to trusted cluster administrators. This can be achieved by Kubernetes RBAC policies or tools like Open Policy Agent.'
check_command:
- 'kubectl get ClusterRole -o=custom-columns="NAME:.metadata.name" | awk ''{if(NR>1)print}'''
Expand All @@ -22,7 +22,7 @@ categories:
- https://kubernetes.io/docs/reference/access-authn-authz/rbac/#auto-reconciliation
- https://www.openpolicyagent.org/
-
name: 'Avoid overly broad hosts configurations'
name: '6.1 Avoid overly broad hosts configurations'
description: 'When possible, avoid overly broad hosts settings in Gateway. may cause potential exposure of unexpected domains'
check_command:
- 'kubectl get gateways --all-namespaces -o=custom-columns="NAME:.metadata.name,NAMESPACE:.metadata.namespace" | awk ''{if(NR>1)print}'' |awk '' { print $1 " -n " $NF } '''
Expand All @@ -35,7 +35,7 @@ categories:
references:
- https://istio.io/latest/docs/ops/best-practices/security/#gateways
-
name: 'Isolate sensitive services'
name: '6.2 Isolate sensitive services'
description: 'It may be desired to enforce stricter physical isolation for sensitive services. For example,
you may want to run a dedicated gateway instance for a sensitive payments.example.com, while utilizing
a single shared gateway instance for less sensitive domains like blog.example.com and store.example.com.'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: Downstream Connections
security_checks:
-
name: 'make sure config map with downstream Connections created'
name: '7.0 make sure config map with downstream Connections created'
description: 'Update global_downstream_max_connections in the config map according to the number of concurrent connections needed by individual gateway instances in your deployment. Once the limit is reached, Envoy will start rejecting tcp connections.'
check_command:
- 'kubectl get configmap istio-custom-bootstrap-config -n istio-system -o json 2> /dev/null'
Expand All @@ -19,7 +19,7 @@ categories:
eval_message: 'config map istio-custom-bootstrap-config is exist on namespace istio-system'
references:
- https://istio.io/latest/docs/ops/best-practices/security/#configure-a-limit-on-downstream-connections
- name: 'make ingress gateway deployment is patched with downstream Connections limit config'
- name: '7.1 make ingress gateway deployment is patched with downstream Connections limit config'
description: 'Patch ingress gateway with downstream Connections limit config Once the limit is reached, Envoy will start rejecting tcp connections.'
check_command:
- 'kubectl get deployment istio-ingressgateway -n istio-system -o json 2> /dev/null'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: Third party tokens
security_checks:
-
name: 'Configure third party service account tokens'
name: '8.0 Configure third party service account tokens'
description: 'Because the properties of the first party token are less secure, Istio will default to using third party tokens. However, this feature is not enabled on all Kubernetes platforms.'
check_command:
- 'kubectl get --raw /api/v1 | jq ''.resources[] | select(.name | index("serviceaccounts/token"))'' 2> /dev/null'
Expand Down
6 changes: 3 additions & 3 deletions internal/security/mesh/istio/9_control_plane.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ categories:
name: Lock down ports
security_checks:
-
name: 'control-plane ensure Istiod do not exposes Port 8080 for debug as unauthenticated plaintext'
name: '9.0 control-plane ensure Istiod do not exposes Port 8080 for debug as unauthenticated plaintext'
description: 'Istiod exposes an unauthenticated plaintext port 8080 for convenience by default. If desired, these can be closed'
check_command:
- 'kubectl get deployment istiod -n istio-system -o json 2> /dev/null'
Expand All @@ -20,7 +20,7 @@ categories:
references:
- https://istio.io/latest/docs/ops/best-practices/security/#control-plane
-
name: 'control-plane ensure Istiod do not exposes Port 15010 XDS service over plaintext'
name: '9.1 control-plane ensure Istiod do not exposes Port 15010 XDS service over plaintext'
description: 'Istiod exposes an unauthenticated plaintext port 15010 for convenience by default. If desired, these can be closed'
check_command:
- 'kubectl get deployment istiod -n istio-system -o json 2> /dev/null'
Expand All @@ -33,7 +33,7 @@ categories:
references:
- https://istio.io/latest/docs/ops/best-practices/security/#control-plane
-
name: 'data plane - The proxy exposes a variety of ports'
name: '9.2 data plane - The proxy exposes a variety of ports'
description: 'The proxy exposes a variety of ports. Exposed externally are port 15090 (telemetry) and port 15021 (health check).
Ports 15020 and 15000 provide debugging endpoints'
check_command:
Expand Down
26 changes: 4 additions & 22 deletions pkg/filters/predicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
// Predicate filter audit tests cmd criteria
type Predicate func(tests *models.SubCategory, params string) *models.SubCategory

// IncludeAudit include audit tests , only included tests will be executed
var IncludeAudit Predicate = func(tests *models.SubCategory, params string) *models.SubCategory {
// IncludeCheck include audit tests , only included tests will be executed
var IncludeCheck Predicate = func(tests *models.SubCategory, params string) *models.SubCategory {
sat := make([]*models.SecurityCheck, 0)
spt := utils.GetAuditTestsList("i", params)
// check if param include category
Expand All @@ -33,8 +33,8 @@ var IncludeAudit Predicate = func(tests *models.SubCategory, params string) *mod
return &models.SubCategory{Name: tests.Name, Checks: sat}
}

// ExcludeAudit audit test from been executed
var ExcludeAudit Predicate = func(tests *models.SubCategory, params string) *models.SubCategory {
// ExcludeCheck audit test from been executed
var ExcludeCheck Predicate = func(tests *models.SubCategory, params string) *models.SubCategory {
sat := make([]*models.SecurityCheck, 0)
spt := utils.GetAuditTestsList("e", params)
// if exclude category
Expand All @@ -58,24 +58,6 @@ var ExcludeAudit Predicate = func(tests *models.SubCategory, params string) *mod
return &models.SubCategory{Name: tests.Name, Checks: sat}
}

// NodeAudit audit test from been executed
var NodeAudit Predicate = func(tests *models.SubCategory, params string) *models.SubCategory {
sat := make([]*models.SecurityCheck, 0)
spt := utils.GetAuditTestsList("n", params)
// check tests
for _, at := range tests.Checks {
for _, sp := range spt {
if strings.ToLower(at.ProfileApplicability) == sp {
sat = append(sat, at)
}
}
}
if len(sat) == 0 {
return &models.SubCategory{Name: tests.Name, Checks: make([]*models.SecurityCheck, 0)}
}
return &models.SubCategory{Name: tests.Name, Checks: sat}
}

// Basic filter by specific audit tests as set in command
var Basic Predicate = func(tests *models.SubCategory, params string) *models.SubCategory {
return tests
Expand Down
31 changes: 4 additions & 27 deletions pkg/filters/predicate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,30 @@ import (
//Test_IncludeTestPredict text
func Test_IncludeTestPredict(t *testing.T) {
ab := &models.SubCategory{Checks: []*models.SecurityCheck{{Name: "1.2.1 abc"}, {Name: "1.2.3 eft"}}}
abp := IncludeAudit(ab, "1.2.1")
abp := IncludeCheck(ab, "1.2.1")
assert.Equal(t, abp.Checks[0].Name, "1.2.1 abc")
assert.True(t, len(abp.Checks) == 1)
}

//Test_IncludeTestPredicateNoValidArg text
func Test_IncludeTestPredicateNoValidArg(t *testing.T) {
ab := &models.SubCategory{Checks: []*models.SecurityCheck{{Name: "1.2.1 abc"}, {Name: "1.2.3 eft"}}}
abp := IncludeAudit(ab, "1.2.5")
abp := IncludeCheck(ab, "1.2.5")
assert.True(t, len(abp.Checks) == 0)
}

//Test_ExcludeTestPredict text
func Test_ExcludeTestPredict(t *testing.T) {
ab := &models.SubCategory{Checks: []*models.SecurityCheck{{Name: "1.2.1 abc"}, {Name: "1.2.3 eft"}}}
abp := ExcludeAudit(ab, "1.2.1")
abp := ExcludeCheck(ab, "1.2.1")
assert.Equal(t, abp.Checks[0].Name, "1.2.3 eft")
assert.True(t, len(abp.Checks) == 1)
}

//Test_ExcludeTestPredicateNoValidArg text
func Test_ExcludeTestPredicateNoValidArg(t *testing.T) {
ab := &models.SubCategory{Checks: []*models.SecurityCheck{{Name: "1.2.1 abc"}, {Name: "1.2.3 eft"}}}
abp := ExcludeAudit(ab, "1.2.5")
abp := ExcludeCheck(ab, "1.2.5")
assert.Equal(t, abp.Checks[0].Name, "1.2.1 abc")
assert.Equal(t, abp.Checks[1].Name, "1.2.3 eft")
assert.True(t, len(abp.Checks) == 2)
Expand All @@ -46,26 +46,3 @@ func Test_BasicPredicate(t *testing.T) {
assert.Equal(t, abp.Checks[1].Name, "1.2.3 eft")
assert.True(t, len(abp.Checks) == 2)
}

//Test_NodePredicateMaster text
func Test_NodePredicateMaster(t *testing.T) {
ab := &models.SubCategory{Checks: []*models.SecurityCheck{{Name: "1.2.1 abc", ProfileApplicability: "Master"}, {Name: "1.2.3 eft", ProfileApplicability: "Worker"}}}
abp := NodeAudit(ab, "n=master")
assert.Equal(t, abp.Checks[0].Name, "1.2.1 abc")
assert.True(t, len(abp.Checks) == 1)
}

//Test_NodePredicateWorker text
func Test_NodePredicateWorker(t *testing.T) {
ab := &models.SubCategory{Checks: []*models.SecurityCheck{{Name: "1.2.1 abc", ProfileApplicability: "Master"}, {Name: "1.2.3 eft", ProfileApplicability: "Worker"}}}
abp := NodeAudit(ab, "n=worker")
assert.Equal(t, abp.Checks[0].Name, "1.2.3 eft")
assert.True(t, len(abp.Checks) == 1)
}

//Test_NodePredicateNone text
func Test_NodePredicateNone(t *testing.T) {
ab := &models.SubCategory{Checks: []*models.SecurityCheck{{Name: "1.2.1 abc", ProfileApplicability: "Master"}, {Name: "1.2.3 eft", ProfileApplicability: "Worker"}}}
abp := NodeAudit(ab, "n=abd")
assert.Equal(t, len(abp.Checks), 0)
}
4 changes: 2 additions & 2 deletions pkg/utils/fileutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (meshf bFolder) GetHomeFolder() (string, error) {
if err != nil {
return "", err
}
// User can set a custom KUBE_KNARK_HOME from environment variable
// user can set a custom KUBE_KNARK_HOME from environment variable
usrHome := GetEnv(common.MeshKridikHomeEnvVar, usr.HomeDir)
return path.Join(usrHome, ".mesh-kridik"), nil
}
Expand Down Expand Up @@ -98,7 +98,7 @@ func GetHomeFolder() string {
if err != nil {
panic("Failed to fetch user home folder")
}
// User can set a custom MESH_KRIDIK_HOME from environment variable
// user can set a custom MESH_KRIDIK_HOME from environment variable
usrHome := GetEnv(common.MeshKridikHomeEnvVar, usr.HomeDir)
return path.Join(usrHome, ".mesh-kridik")
}
Expand Down

0 comments on commit 1b2b51b

Please sign in to comment.