#33 Add loop prevention to stop endless scan cycles

Author: J12934

hooks/declarative-subsequent-scans/hook.test.js

@@ -1,53 +1,55 @@
 const { getCascadingScans } = require("./hook");
 
 let parentScan = undefined;
+let sslyzeCascadingRules = undefined;
 
 beforeEach(() => {
   parentScan = {
     apiVersion: "execution.experimental.securecodebox.io/v1",
     kind: "Scan",
     metadata: {
       name: "nmap-foobar.com",
+      annotations: {}
     },
     spec: {
       scanType: "nmap",
-      parameters: "foobar.com",
-    },
+      parameters: "foobar.com"
+    }
   };
-});
 
-const sslyzeCascadingRules = [
-  {
-    apiVersion: "cascading.experimental.securecodebox.io/v1",
-    kind: "CascadingRule",
-    metadata: {
-      name: "tls-scans",
-    },
-    spec: {
-      matches: {
-        anyOf: [
-          {
-            category: "Open Port",
-            attributes: {
-              port: 443,
-              service: "https",
-            },
-          },
-          {
-            category: "Open Port",
-            attributes: {
-              service: "https",
-            },
-          },
-        ],
-      },
-      scanSpec: {
-        scanType: "sslyze",
-        parameters: ["--regular", "{{attributes.hostname}}"],
+  sslyzeCascadingRules = [
+    {
+      apiVersion: "cascading.experimental.securecodebox.io/v1",
+      kind: "CascadingRule",
+      metadata: {
+        name: "tls-scans"
       },
-    },
-  },
-];
+      spec: {
+        matches: {
+          anyOf: [
+            {
+              category: "Open Port",
+              attributes: {
+                port: 443,
+                service: "https"
+              }
+            },
+            {
+              category: "Open Port",
+              attributes: {
+                service: "https"
+              }
+            }
+          ]
+        },
+        scanSpec: {
+          scanType: "sslyze",
+          parameters: ["--regular", "{{attributes.hostname}}"]
+        }
+      }
+    }
+  ];
+});
 
 test("should create subsequent scans for open HTTPS ports (NMAP findings)", () => {
   const findings = [
@@ -58,9 +60,9 @@ test("should create subsequent scans for open HTTPS ports (NMAP findings)", () =
         state: "open",
         hostname: "foobar.com",
         port: 443,
-        service: "https",
-      },
-    },
+        service: "https"
+      }
+    }
   ];
 
   const cascadedScans = getCascadingScans(
@@ -72,6 +74,7 @@ test("should create subsequent scans for open HTTPS ports (NMAP findings)", () =
   expect(cascadedScans).toMatchInlineSnapshot(`
     Array [
       Object {
+        "generatedBy": "tls-scans",
         "name": "sslyze-foobar.com-tls-scans",
         "parameters": Array [
           "--regular",
@@ -92,9 +95,9 @@ test("Should create no subsequent scans if there are no rules", () => {
         state: "open",
         hostname: "foobar.com",
         port: 443,
-        service: "https",
-      },
-    },
+        service: "https"
+      }
+    }
   ];
 
   const cascadingRules = [];
@@ -115,9 +118,9 @@ test("should not try to do magic to the scan name if its something random", () =
         state: "open",
         hostname: "foobar.com",
         port: 443,
-        service: "https",
-      },
-    },
+        service: "https"
+      }
+    }
   ];
 
   const cascadedScans = getCascadingScans(
@@ -129,6 +132,7 @@ test("should not try to do magic to the scan name if its something random", () =
   expect(cascadedScans).toMatchInlineSnapshot(`
     Array [
       Object {
+        "generatedBy": "tls-scans",
         "name": "foobar.com-tls-scans",
         "parameters": Array [
           "--regular",
@@ -139,3 +143,29 @@ test("should not try to do magic to the scan name if its something random", () =
     ]
   `);
 });
+
+test("should not start scan when the cascadingrule for it is already in the chain", () => {
+  parentScan.metadata.annotations["cascading.securecodebox.io/chain"] =
+    sslyzeCascadingRules[0].metadata.name;
+
+  const findings = [
+    {
+      name: "Port 443 is open",
+      category: "Open Port",
+      attributes: {
+        state: "open",
+        hostname: "foobar.com",
+        port: 443,
+        service: "https"
+      }
+    }
+  ];
+
+  const cascadedScans = getCascadingScans(
+    parentScan,
+    findings,
+    sslyzeCascadingRules
+  );
+
+  expect(cascadedScans).toMatchInlineSnapshot(`Array []`);
+});

hooks/declarative-subsequent-scans/hook.ts

@@ -87,7 +87,29 @@ export function getCascadingScans(
 ): Array<ExtendedScanSpec> {
   const cascadingScans: Array<ExtendedScanSpec> = [];
 
+  const cascadingRuleChain = new Set<string>();
+
+  // Get the current Scan Chain (meaning which CascadingRules were used to start this scan and its parents) and convert it to a set, which makes it easier to query.
+  if (parentScan.metadata.annotations["cascading.securecodebox.io/chain"]) {
+    const chainElements = parentScan.metadata.annotations[
+      "cascading.securecodebox.io/chain"
+    ].split(",");
+
+    for (const element of chainElements) {
+      cascadingRuleChain.add(element);
+    }
+  }
+
   for (const cascadingRule of cascadingRules) {
+    // Check if the Same CascadingRule was already applied in the Cascading Chain
+    // If it has already been used skip this rule as it could potentially lead to loops
+    if (cascadingRuleChain.has(cascadingRule.metadata.name)) {
+      console.log(
+        `Skipping Rule "${cascadingRule.metadata.name}" as it was already applied in this chain.`
+      );
+      continue;
+    }
+
     for (const finding of findings) {
       // Check if one (ore more) of the CascadingRule matchers apply to the finding
       const matches = cascadingRule.spec.matches.anyOf.some((matchesRule) =>

hooks/declarative-subsequent-scans/scan-helpers.ts

@@ -6,13 +6,21 @@ kc.loadFromDefault();
 
 const k8sApiCRD = kc.makeApiClient(k8s.CustomObjectsApi);
 
-async function startSubsequentSecureCodeBoxScan({
+export async function startSubsequentSecureCodeBoxScan({
   name,
   parentScan,
   scanType,
   parameters,
   generatedBy,
 }) {
+  let cascadingChain: Array<string> = [];
+
+  if (parentScan.metadata.annotations["cascading.securecodebox.io/chain"]) {
+    cascadingChain = parentScan.metadata.annotations[
+      "cascading.securecodebox.io/chain"
+    ].split(",");
+  }
+
   const scanDefinition = {
     apiVersion: "execution.experimental.securecodebox.io/v1",
     kind: "Scan",
@@ -23,8 +31,11 @@ async function startSubsequentSecureCodeBoxScan({
       },
       annotations: {
         "securecodebox.io/hook": "declarative-subsequent-scans",
-        "securecodebox.io/parent-scan": parentScan.metadata.name,
-        "cascading.securecodebox.io/generated-by": generatedBy,
+        "cascading.securecodebox.io/parent-scan": parentScan.metadata.name,
+        "cascading.securecodebox.io/chain": [
+          ...cascadingChain,
+          generatedBy,
+        ].join(","),
       },
       ownerReferences: [
         {
@@ -43,6 +54,8 @@ async function startSubsequentSecureCodeBoxScan({
     },
   };
 
+  console.log(`Starting Scan ${name}`);
+
   try {
     // Starting another subsequent sslyze scan based on the nmap results
     // found at: https://github.com/kubernetes-client/javascript/blob/79736b9a608c18d818de61a6b44503a08ea3a78f/src/gen/api/customObjectsApi.ts#L209
@@ -60,50 +73,21 @@ async function startSubsequentSecureCodeBoxScan({
   }
 }
 
-module.exports.startSubsequentSecureCodeBoxScan = startSubsequentSecureCodeBoxScan;
-
-async function getCascadingRulesFromCluster() {
+export async function getCascadingRulesFromCluster() {
   try {
     const namespace = process.env["NAMESPACE"];
-    const { body } = await k8sApiCRD.listNamespacedCustomObject(
+    const response: any = await k8sApiCRD.listNamespacedCustomObject(
       "cascading.experimental.securecodebox.io",
       "v1",
       namespace,
       "cascadingrules"
     );
-    console.log("got CascadingRules");
-    console.log(body);
-    console.log(JSON.stringify(body));
-    return body.items;
+
+    console.log(`Fetched ${response.body.items.length} CascadingRules`);
+    return response.body.items;
   } catch (err) {
     console.error("Failed to get CascadingRules from the kubernetes api");
     console.error(err);
     process.exit(1);
   }
 }
-module.exports.getCascadingRulesFromCluster = getCascadingRulesFromCluster;
-
-enum LabelSelectorRequirementOperator {
-  In,
-  NotIn,
-  Exists,
-  DoesNotExist,
-}
-
-// See: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#labelselectorrequirement-v1-meta
-// Re created in TS because the included types suck 😕
-interface LabelSelectorRequirement {
-  key: string;
-  values: string;
-
-  operator: LabelSelectorRequirementOperator;
-}
-
-function generateLabelSelectorString(
-  matchExpression: Array<LabelSelectorRequirement>,
-  matchLabels: Map<string, string>
-): string {
-  // Convert matchLabels to matchExpression syntax
-  matchExpression;
-  return "";
-}