Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Our objective is to align the titles and logic with the commercial version. #1382

Merged
merged 3 commits into from
Jul 27, 2023
Merged

Our objective is to align the titles and logic with the commercial version. #1382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f4d882
Our objective for this task is to align the titles and logic with the…
ManojShastha Jul 10, 2023
5347284
Our objective for this task is to align the titles and logic with the…
ManojShastha Jul 10, 2023
7d5f6ff
Merge branch 'updating_rego_titles' of github.com:mjshastha/defsec in…
mjshastha Jul 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions pkg/scanners/helm/test/scanner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func Test_helm_scanner_with_archive(t *testing.T) {
require.NotNil(t, results)

failed := results.GetFailed()
assert.Equal(t, 12, len(failed))
assert.Equal(t, 13, len(failed))

visited := make(map[string]bool)
var errorCodes []string
Expand All @@ -58,7 +58,7 @@ func Test_helm_scanner_with_archive(t *testing.T) {
errorCodes = append(errorCodes, id)
}
}
assert.Len(t, errorCodes, 12)
assert.Len(t, errorCodes, 13)

sort.Strings(errorCodes)

Expand All @@ -67,7 +67,7 @@ func Test_helm_scanner_with_archive(t *testing.T) {
"AVD-KSV-0011", "AVD-KSV-0012", "AVD-KSV-0014",
"AVD-KSV-0015", "AVD-KSV-0016", "AVD-KSV-0018",
"AVD-KSV-0020", "AVD-KSV-0021", "AVD-KSV-0030",
"AVD-KSV-0106",
"AVD-KSV-0104", "AVD-KSV-0106",
}, errorCodes)
}
}
Expand Down Expand Up @@ -127,7 +127,7 @@ func Test_helm_scanner_with_dir(t *testing.T) {
require.NotNil(t, results)

failed := results.GetFailed()
assert.Equal(t, 12, len(failed))
assert.Equal(t, 13, len(failed))

visited := make(map[string]bool)
var errorCodes []string
Expand All @@ -146,7 +146,7 @@ func Test_helm_scanner_with_dir(t *testing.T) {
"AVD-KSV-0011", "AVD-KSV-0012", "AVD-KSV-0014",
"AVD-KSV-0015", "AVD-KSV-0016", "AVD-KSV-0018",
"AVD-KSV-0020", "AVD-KSV-0021", "AVD-KSV-0030",
"AVD-KSV-0106",
"AVD-KSV-0104", "AVD-KSV-0106",
}, errorCodes)
}
}
Expand Down Expand Up @@ -213,7 +213,7 @@ deny[res] {
require.NotNil(t, results)

failed := results.GetFailed()
assert.Equal(t, 14, len(failed))
assert.Equal(t, 15, len(failed))

visited := make(map[string]bool)
var errorCodes []string
Expand All @@ -224,7 +224,7 @@ deny[res] {
errorCodes = append(errorCodes, id)
}
}
assert.Len(t, errorCodes, 13)
assert.Len(t, errorCodes, 14)

sort.Strings(errorCodes)

Expand All @@ -233,7 +233,7 @@ deny[res] {
"AVD-KSV-0011", "AVD-KSV-0012", "AVD-KSV-0014",
"AVD-KSV-0015", "AVD-KSV-0016", "AVD-KSV-0018",
"AVD-KSV-0020", "AVD-KSV-0021", "AVD-KSV-0030",
"AVD-KSV-0106", "AVD-USR-ID001",
"AVD-KSV-0104", "AVD-KSV-0106", "AVD-USR-ID001",
}, errorCodes)
})
}
Expand Down
7 changes: 5 additions & 2 deletions rules/kubernetes/policies/advanced/default_namespace_should_not_be_used.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# METADATA
# title: "The default namespace should not be used"
# title: "Workloads in the default namespace"
# description: "ensure that default namespace should not be used"
# scope: package
# schemas:
Expand All @@ -21,12 +21,15 @@ import data.lib.kubernetes

default defaultNamespaceInUse = false

allowedKinds := ["pod", "replicaset", "replicationcontroller", "deployment", "statefulset", "daemonset", "cronjob", "job"]

defaultNamespaceInUse {
kubernetes.namespace == "default"
lower(kubernetes.kind) == allowedKinds[_]
}

deny[res] {
defaultNamespaceInUse
msg := sprintf("%s '%s' should not be set with 'default' namespace", [kubernetes.kind, kubernetes.name])
msg := kubernetes.format(sprintf("%s %s in %s namespace should set metadata.namespace to a non-default namespace", [lower(kubernetes.kind), kubernetes.name, kubernetes.namespace]))
res := result.new(msg, input.metadata.namespace)
}
2 changes: 1 addition & 1 deletion rules/kubernetes/policies/advanced/optional/capabilities_no_drop_at_least_one.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# METADATA
# title: "Unused capabilities should be dropped (drop any)"
# title: "Default capabilities: some containers do not drop any"
# description: "Security best practices require containers to run with minimal required capabilities."
# scope: package
# schemas:
Expand Down
2 changes: 1 addition & 1 deletion rules/kubernetes/policies/advanced/optional/manages_etc_hosts.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# METADATA
# title: "hostAliases is set"
# title: "Manages /etc/hosts"
# description: "Managing /etc/hosts aliases can prevent the container engine from modifying the file after a pod’s containers have already been started."
# scope: package
# schemas:
Expand Down
8 changes: 4 additions & 4 deletions rules/kubernetes/policies/general/allowing_to_update_a_malicious_pod.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# METADATA
# title: "Do not allow update/create of a malicious pod"
# title: "Manage Kubernetes workloads and pods"
# description: "Check whether role permits update/create of a malicious pod"
# scope: package
# schemas:
Expand All @@ -20,9 +20,9 @@ package builtin.kubernetes.KSV048
import data.lib.kubernetes
import data.lib.utils

workloads := ["deployments", "daemonsets", "statefulsets", "replicationcontrollers", "replicasets", "jobs", "cronjobs"]
workloads := ["pods", "deployments", "jobs", "cronjobs", "statefulsets", "daemonsets", "replicasets", "replicationcontrollers"]

changeVerbs := ["update", "create", "*"]
changeVerbs := ["create", "update", "patch", "delete", "deletecollection", "impersonate", "*"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering why we are using asterisk (*) here - wildcard character that matches any string. We have added more verbs now. Do we still need * ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly! We have expanded the list of verbs and included the wildcard "*" value in order to align the oss with the commercial.

Copy link
Contributor

@dheerajkadri dheerajkadri Jul 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using the wildcard (*), then there is no need to define verbs explicitly, since the asterisk( * ) will match any verb. Using the wildcard character could result in the policy generating false positives. Additionally, using the wildcard character could make the policy less efficient, as it will have to check for rules that allow a wider range of verbs.

Can you check if we really need this wildcard character ? this is used in many checks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I concur that including a wildcard () in the verbs list negates the need to define verbs explicitly, as the asterisk () will match any verb. The asterisk (*) was already present in the verbs list, and I introduced additional verbs to ensure alignment between the oss and commercial.


readKinds := ["Role", "ClusterRole"]

Expand All @@ -35,6 +35,6 @@ update_malicious_pod[input.rules[ru]] {

deny[res] {
badRule := update_malicious_pod[_]
msg := "Role permits create/update of a malicious pod"
msg := kubernetes.format(sprintf("%s '%s' should not have access to resources %s for verbs %s", [kubernetes.kind, kubernetes.name, workloads, changeVerbs]))
res := result.new(msg, badRule)
}
72 changes: 72 additions & 0 deletions rules/kubernetes/policies/general/allowing_to_update_a_malicious_pod_test.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,3 +251,75 @@ test_update_malicious_pod_cronjobs {

count(r) > 0
}

test_update_malicious_pod_deletecollection {
r := deny with input as {
"apiVersion": "rbac.authorization.k8s.io/v1",
"kind": "Role",
"metadata": {
"namespace": "default",
"name": "pod-reader",
},
"rules": [{
"apiGroups": ["*"],
"resources": ["cronjobs"],
"verbs": ["deletecollection"],
}],
}

count(r) > 0
}

test_update_malicious_pod_delete {
r := deny with input as {
"apiVersion": "rbac.authorization.k8s.io/v1",
"kind": "Role",
"metadata": {
"namespace": "default",
"name": "pod-reader",
},
"rules": [{
"apiGroups": ["*"],
"resources": ["job"],
"verbs": ["delete"],
}],
}

count(r) == 0
}

test_update_malicious_pod_patch {
r := deny with input as {
"apiVersion": "rbac.authorization.k8s.io/v1",
"kind": "Role",
"metadata": {
"namespace": "default",
"name": "pod-reader",
},
"rules": [{
"apiGroups": ["*"],
"resources": ["job"],
"verbs": ["patch"],
}],
}

count(r) == 0
}

test_update_malicious_pod_impersonate {
r := deny with input as {
"apiVersion": "rbac.authorization.k8s.io/v1",
"kind": "Role",
"metadata": {
"namespace": "default",
"name": "pod-reader",
},
"rules": [{
"apiGroups": ["*"],
"resources": ["job"],
"verbs": ["impersonate"],
}],
}

count(r) == 0
}
2 changes: 1 addition & 1 deletion rules/kubernetes/policies/general/capabilities_no_drop_all.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# METADATA
# title: "Default capabilities not dropped"
# title: "Default capabilities: some containers do not drop all"
# description: "The container should drop all default capabilities and add only those that are needed for its execution."
# scope: package
# schemas:
Expand Down
2 changes: 1 addition & 1 deletion rules/kubernetes/policies/general/delete_pod_logs.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# METADATA
# title: "Do not allow deletion of pod logs"
# title: "Delete pod logs"
# description: "Used to cover attacker’s tracks, but most clusters ship logs quickly off-cluster."
# scope: package
# schemas:
Expand Down
25 changes: 11 additions & 14 deletions rules/kubernetes/policies/general/get_shell_on_pod.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# METADATA
# title: "Do not allow getting shell on pods"
# title: "Exec into Pods"
# description: "Check whether role permits getting shell on pods"
# scope: package
# schemas:
Expand All @@ -20,24 +20,21 @@ package builtin.kubernetes.KSV053
import data.lib.kubernetes
import data.lib.utils

workloads := ["pods/exec"]

changeVerbs := ["create", "update", "patch", "delete", "deletecollection", "impersonate", "*"]

readKinds := ["Role", "ClusterRole"]

get_shell_on_pod[ruleA] {
execPodsRestricted[input.rules[ru]] {
some ru, r, v
input.kind == readKinds[_]
some i, j
ruleA := input.rules[i]
ruleB := input.rules[j]
i < j
ruleA.apiGroups[_] == "*"
ruleA.resources[_] == "pods/exec"
ruleA.verbs[_] == "create"
ruleB.apiGroups[_] == "*"
ruleB.resources[_] == "pods"
ruleB.verbs[_] == "get"
input.rules[ru].resources[r] == workloads[_]
input.rules[ru].verbs[v] == changeVerbs[_]
}

deny[res] {
badRule := get_shell_on_pod[_]
msg := "Role permits getting shell on pods"
badRule := execPodsRestricted[_]
msg := kubernetes.format(sprintf("%s '%s' should not have access to resource '%s' for verbs %s", [kubernetes.kind, kubernetes.name, workloads, changeVerbs]))
res := result.new(msg, badRule)
}
85 changes: 25 additions & 60 deletions rules/kubernetes/policies/general/get_shell_on_pod_test.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,11 @@ test_getting_shell_on_pods {
"namespace": "default",
"name": "pod-reader",
},
"rules": [
{
"apiGroups": ["*"],
"resources": ["pods/exec"],
"verbs": ["create"],
},
{
"apiGroups": ["*"],
"resources": ["pods"],
"verbs": ["get"],
},
],
"rules": [{
"apiGroups": ["*"],
"resources": ["pods/exec"],
"verbs": ["create"],
}],
}

count(r) == 1
Expand All @@ -33,18 +26,11 @@ test_getting_shell_on_pods_no_pod_exec {
"namespace": "default",
"name": "pod-reader",
},
"rules": [
{
"apiGroups": ["*"],
"resources": ["pods/exec1"],
"verbs": ["create"],
},
{
"apiGroups": ["*"],
"resources": ["pods"],
"verbs": ["get"],
},
],
"rules": [{
"apiGroups": ["*"],
"resources": ["pods/exec1"],
"verbs": ["create"],
}],
}

count(r) == 0
Expand All @@ -58,18 +44,11 @@ test_getting_shell_on_pods_no_verb_create {
"namespace": "default",
"name": "pod-reader",
},
"rules": [
{
"apiGroups": ["*"],
"resources": ["pods/exec"],
"verbs": ["create1"],
},
{
"apiGroups": ["*"],
"resources": ["pods"],
"verbs": ["get"],
},
],
"rules": [{
"apiGroups": ["*"],
"resources": ["pods/exec"],
"verbs": ["create1"],
}],
}

count(r) == 0
Expand All @@ -83,18 +62,11 @@ test_getting_shell_on_pods_no_resource_pod {
"namespace": "default",
"name": "pod-reader",
},
"rules": [
{
"apiGroups": ["*"],
"resources": ["pods/exec"],
"verbs": ["create1"],
},
{
"apiGroups": ["*"],
"resources": ["pods1"],
"verbs": ["get"],
},
],
"rules": [{
"apiGroups": ["*"],
"resources": ["pods/exec"],
"verbs": ["create1"],
}],
}

count(r) == 0
Expand All @@ -108,18 +80,11 @@ test_getting_shell_on_pods_no_verb_get {
"namespace": "default",
"name": "pod-reader",
},
"rules": [
{
"apiGroups": ["*"],
"resources": ["pods/exec"],
"verbs": ["create1"],
},
{
"apiGroups": ["*"],
"resources": ["pods"],
"verbs": ["get1"],
},
],
"rules": [{
"apiGroups": ["*"],
"resources": ["pods/exec"],
"verbs": ["create1"],
}],
}

count(r) == 0
Expand Down
2 changes: 1 addition & 1 deletion rules/kubernetes/policies/general/manage_configmaps.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# METADATA
# title: "Do not allow management of configmaps"
# title: "Manage configmaps"
# description: "Some workloads leverage configmaps to store sensitive data or configuration parameters that affect runtime behavior that can be modified by an attacker or combined with another issue to potentially lead to compromise."
# scope: package
# schemas:
Expand Down
Loading