Repository: bridgecrewio/checkov
Pull request: Ensure security group has stateless ingress security rules
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

resolved #1972

added check to ensure OCI Network Security Groups (NSG) has stateless security rules

Problem statement:
Add OCI TF check - OCI Network Security Groups (NSG) has stateful security rules
Equivalent RQL query

config from cloud.resource where cloud.type = 'oci' AND api.name = 'oci-networking-nsg' AND json.rule = securityRules[?any( direction equals INGRESS and (isStateless does not exist or isStateless is false) )] exists

Hints from issue discussion:
Note that RQL queries are spotting the error while “Checkov logic wish to find the Desired state” use the referance below in order to get more data for the check needed -

_This policy identifies the OCI Network Security Groups (NSG) security rules that have stateful ingress rules configured. It is recommended that Network Security Groups (NSG) security rules are configured with stateless ingress rules to slow the impact of a denial-of-service (DoS) attack._

https://github.com/PaloAltoNetworks/prisma-cloud-policies/blob/master/policies/OCI-Network-Security-Groups-NSG-has-stateful-security-rules.json

Commit message: Update test_SecurityGroupsIngressStatelessSecurityRules.py

Patch to review:
diff --git a/checkov/terraform/checks/resource/oci/SecurityGroupsIngressStatelessSecurityRules.py b/checkov/terraform/checks/resource/oci/SecurityGroupsIngressStatelessSecurityRules.py
new file mode 100644
index 0000000000..eac563f116
--- /dev/null
+++ b/checkov/terraform/checks/resource/oci/SecurityGroupsIngressStatelessSecurityRules.py
@@ -0,0 +1,23 @@
+from checkov.common.models.enums import CheckCategories, CheckResult
+from checkov.terraform.checks.resource.base_resource_check import BaseResourceCheck
+
+
+class SecurityGroupsIngressStatelessSecurityRules(BaseResourceCheck):
+    def __init__(self):
+        name = "Ensure security groups has stateless ingress security rules"
+        id = "CKV_OCI_21"
+        supported_resources = ['oci_core_network_security_group_security_rule']
+        categories = [CheckCategories.NETWORKING]
+        super().__init__(name=name, id=id, categories=categories, supported_resources=supported_resources)
+
+    def scan_resource_conf(self, conf):
+        stateless = conf.get('stateless')
+        direction = conf.get('direction')
+        if direction[0] == 'INGRESS':
+            if stateless is None or stateless[0] is False:
+                return CheckResult.FAILED
+            return CheckResult.PASSED
+        return CheckResult.SKIPPED
+
+
+check = SecurityGroupsIngressStatelessSecurityRules()
diff --git a/tests/terraform/checks/resource/oci/example_SecurityGroupsIngressStatelessSecurityRules/main.tf b/tests/terraform/checks/resource/oci/example_SecurityGroupsIngressStatelessSecurityRules/main.tf
new file mode 100644
index 0000000000..c5f303c99d
--- /dev/null
+++ b/tests/terraform/checks/resource/oci/example_SecurityGroupsIngressStatelessSecurityRules/main.tf
@@ -0,0 +1,34 @@
+
+resource "oci_core_network_security_group_security_rule" "pass" {
+  network_security_group_id = oci_core_network_security_group.test_network_security_group.id
+  direction                 = "INGRESS"
+  protocol                  = var.network_security_group_security_rule_protocol
+  stateless                 = true
+}
+
+resource "oci_core_network_security_group_security_rule" "fail" {
+  network_security_group_id = oci_core_network_security_group.test_network_security_group.id
+  direction                 = "INGRESS"
+  protocol                  = var.network_security_group_security_rule_protocol
+  stateless                 = false
+}
+
+resource "oci_core_network_security_group_security_rule" "fail1" {
+  network_security_group_id = oci_core_network_security_group.test_network_security_group.id
+  direction                 = "INGRESS"
+  protocol                  = var.network_security_group_security_rule_protocol
+}
+
+resource "oci_core_network_security_group_security_rule" "skip" {
+  network_security_group_id = oci_core_network_security_group.test_network_security_group.id
+  direction                 = "EGRESS"
+  protocol                  = var.network_security_group_security_rule_protocol
+  stateless                 = true
+}
+
+resource "oci_core_network_security_group_security_rule" "skip1" {
+  network_security_group_id = oci_core_network_security_group.test_network_security_group.id
+  direction                 = "EGRESS"
+  protocol                  = var.network_security_group_security_rule_protocol
+  stateless                 = false
+}
diff --git a/tests/terraform/checks/resource/oci/test_SecurityGroupsIngressStatelessSecurityRules.py b/tests/terraform/checks/resource/oci/test_SecurityGroupsIngressStatelessSecurityRules.py
new file mode 100644
index 0000000000..02cc87435b
--- /dev/null
+++ b/tests/terraform/checks/resource/oci/test_SecurityGroupsIngressStatelessSecurityRules.py
@@ -0,0 +1,47 @@
+import os
+import unittest
+
+from checkov.runner_filter import RunnerFilter
+from checkov.terraform.checks.resource.oci.SecurityGroupsIngressStatelessSecurityRules import check
+from checkov.terraform.runner import Runner
+
+
+class TestSecurityGroupsIngressStatelessSecurityRules(unittest.TestCase):
+    def test(self):
+        runner = Runner()
+        current_dir = os.path.dirname(os.path.realpath(__file__))
+
+        test_files_dir = current_dir + "/example_SecurityGroupsIngressStatelessSecurityRules"
+        report = runner.run(root_folder=test_files_dir, runner_filter=RunnerFilter(checks=[check.id]))
+        summary = report.get_summary()
+
+        passing_resources = {
+            "oci_core_network_security_group_security_rule.pass",
+        }
+
+        failing_resources = {
+            "oci_core_network_security_group_security_rule.fail",
+            "oci_core_network_security_group_security_rule.fail1",
+        }
+
+        skipped_resources = {
+            "oci_core_network_security_group_security_rule.skip",
+            "oci_core_network_security_group_security_rule.skip1",
+        }
+
+        passed_check_resources = set([c.resource for c in report.passed_checks])
+        failed_check_resources = set([c.resource for c in report.failed_checks])
+        skipped_check_resources = set([c.resource for c in report.skipped_checks])
+
+        self.assertEqual(summary["passed"], 1)
+        self.assertEqual(summary["failed"], 2)
+        self.assertEqual(summary["skipped"], 2)
+        self.assertEqual(summary["parsing_errors"], 0)
+
+        self.assertEqual(skipped_resources, skipped_check_resources)
+        self.assertEqual(passing_resources, passed_check_resources)
+        self.assertEqual(failing_resources, failed_check_resources)
+
+
+if __name__ == "__main__":
+    unittest.main()


Code review comments:
diff_hunk: @@ -0,0 +1,23 @@
+from checkov.common.models.enums import CheckCategories, CheckResult
+from checkov.terraform.checks.resource.base_resource_check import BaseResourceCheck
+
+
+class SecurityGroupsIngressStatelessSecurityRules(BaseResourceCheck):
+    def __init__(self):
+        name = "Ensure security groups has stateless ingress security rules"
path: checkov/terraform/checks/resource/oci/SecurityGroupsIngressStatelessSecurityRules.py
text: @user1:
```suggestion
        name = "Ensure security group has stateless ingress security rules"
```
or 
```suggestion
        name = "Ensure security groups have stateless ingress security rules"
```
your choice 🙂

diff_hunk: @@ -0,0 +1,23 @@
+from checkov.common.models.enums import CheckCategories, CheckResult
+from checkov.terraform.checks.resource.base_resource_check import BaseResourceCheck
+
+
+class SecurityGroupsIngressStatelessSecurityRules(BaseResourceCheck):
+    def __init__(self):
+        name = "Ensure security groups has stateless ingress security rules"
+        id = "CKV_OCI_21"
+        supported_resources = ['oci_core_network_security_group_security_rule']
+        categories = [CheckCategories.NETWORKING]
+        super().__init__(name=name, id=id, categories=categories, supported_resources=supported_resources)
+
+    def scan_resource_conf(self, conf):
+        stateless = conf.get('stateless')
+        direction = conf.get('direction')
+        if direction[0] == 'INGRESS':
+            if stateless is None or stateless[0] is False:
+                return CheckResult.FAILED
+            return CheckResult.PASSED
+        return CheckResult.SKIPPED
path: checkov/terraform/checks/resource/oci/SecurityGroupsIngressStatelessSecurityRules.py
text: @user1:
```suggestion
        return CheckResult.UNKNOWN
```
`SKIPPED` is used in a different context, user driven. we always use `UNKNOWN` if we can't tell is it passing or failing

@user1:
don't forget to remove the skipping resource check inside the test class, but don't remove the resources from the TF file