From 41e9989f6dbd03eb17d94eee4b679d51f8ac0428 Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Tue, 22 Jun 2021 11:16:07 +0200 Subject: [PATCH 1/3] Cascading Scans: update getCascadingScanDefinition argument list to be a bit more workable Signed-off-by: Jop Zitman --- hooks/cascading-scans/hook.test.js | 118 +++++--------------------- hooks/cascading-scans/hook.ts | 15 +--- hooks/cascading-scans/scan-helpers.ts | 6 +- 3 files changed, 29 insertions(+), 110 deletions(-) diff --git a/hooks/cascading-scans/hook.test.js b/hooks/cascading-scans/hook.test.js index 8321521a1b..9e14091249 100644 --- a/hooks/cascading-scans/hook.test.js +++ b/hooks/cascading-scans/hook.test.js @@ -80,7 +80,7 @@ test("Should create subsequent scans for open HTTPS ports (NMAP findings)", () = expect(cascadedScans).toMatchInlineSnapshot(` Array [ Object { - "cascades": null, + "cascades": Object {}, "env": undefined, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", @@ -143,7 +143,7 @@ test("Should not try to do magic to the scan name if its something random", () = expect(cascadedScans).toMatchInlineSnapshot(` Array [ Object { - "cascades": null, + "cascades": Object {}, "env": undefined, "generatedBy": "tls-scans", "name": "foobar.com-tls-scans", @@ -210,7 +210,7 @@ test("Should not crash when the annotations are not set", () => { expect(cascadedScans).toMatchInlineSnapshot(` Array [ Object { - "cascades": null, + "cascades": Object {}, "env": undefined, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", @@ -256,7 +256,7 @@ test("Should copy ENV fields from cascadingRule to created scan", () => { expect(cascadedScans).toMatchInlineSnapshot(` Array [ Object { - "cascades": null, + "cascades": Object {}, "env": Array [ Object { "name": "FOOBAR", @@ -332,7 +332,7 @@ test("Should allow wildcards in cascadingRules", () => { expect(cascadedScans).toMatchInlineSnapshot(` Array [ Object { - "cascades": null, + "cascades": Object {}, "env": undefined, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", @@ -375,17 +375,8 @@ test("should not copy labels if inheritLabels is set to false", () => { sslyzeCascadingRules ); - for (const { name, scanType, parameters, generatedBy, env, scanLabels, scanAnnotations } of cascadedScans) { - const cascadingScanDefinition = getCascadingScanDefinition({ - name, - parentScan: parentScan, - generatedBy, - scanType, - parameters, - env, - scanLabels, - scanAnnotations - }); + for (const cascadedScan of cascadedScans) { + const cascadingScanDefinition = getCascadingScanDefinition(cascadedScan, parentScan); expect(Object.entries(parentScan.metadata.labels).every(([label, value]) => cascadingScanDefinition.metadata.labels[label] === value @@ -419,17 +410,8 @@ test("should copy labels if inheritLabels is not set", () => { sslyzeCascadingRules ); - for (const { name, scanType, parameters, generatedBy, env, scanLabels, scanAnnotations } of cascadedScans) { - const cascadingScanDefinition = getCascadingScanDefinition({ - name, - parentScan: parentScan, - generatedBy, - scanType, - parameters, - env, - scanLabels, - scanAnnotations - }); + for (const cascadedScan of cascadedScans) { + const cascadingScanDefinition = getCascadingScanDefinition(cascadedScan, parentScan); expect(Object.entries(parentScan.metadata.labels).every(([label, value]) => cascadingScanDefinition.metadata.labels[label] === value @@ -465,17 +447,8 @@ test("should copy labels if inheritLabels is set to true", () => { sslyzeCascadingRules ); - for (const { name, scanType, parameters, generatedBy, env, scanLabels, scanAnnotations } of cascadedScans) { - const cascadingScanDefinition = getCascadingScanDefinition({ - name, - parentScan: parentScan, - generatedBy, - scanType, - parameters, - env, - scanLabels, - scanAnnotations - }); + for (const cascadedScan of cascadedScans) { + const cascadingScanDefinition = getCascadingScanDefinition(cascadedScan, parentScan); expect(Object.entries(parentScan.metadata.labels).every(([label, value]) => cascadingScanDefinition.metadata.labels[label] === value @@ -509,17 +482,8 @@ test("should not copy annotations if inheritAnnotations is set to false", () => sslyzeCascadingRules ); - for (const { name, scanType, parameters, generatedBy, env, scanLabels, scanAnnotations } of cascadedScans) { - const cascadingScanDefinition = getCascadingScanDefinition({ - name, - parentScan: parentScan, - generatedBy, - scanType, - parameters, - env, - scanLabels, - scanAnnotations - }); + for (const cascadedScan of cascadedScans) { + const cascadingScanDefinition = getCascadingScanDefinition(cascadedScan, parentScan); expect(Object.entries(parentScan.metadata.annotations).every(([label, value]) => cascadingScanDefinition.metadata.annotations[label] === value @@ -552,17 +516,8 @@ test("should copy annotations if inheritAnnotations is not set", () => { sslyzeCascadingRules ); - for (const { name, scanType, parameters, generatedBy, env, scanLabels, scanAnnotations } of cascadedScans) { - const cascadingScanDefinition = getCascadingScanDefinition({ - name, - parentScan: parentScan, - generatedBy, - scanType, - parameters, - env, - scanLabels, - scanAnnotations - }); + for (const cascadedScan of cascadedScans) { + const cascadingScanDefinition = getCascadingScanDefinition(cascadedScan, parentScan); expect(Object.entries(parentScan.metadata.annotations).every(([label, value]) => cascadingScanDefinition.metadata.annotations[label] === value @@ -596,17 +551,8 @@ test("should copy annotations if inheritAnnotations is set to true", () => { sslyzeCascadingRules ); - for (const { name, scanType, parameters, generatedBy, env, scanLabels, scanAnnotations } of cascadedScans) { - const cascadingScanDefinition = getCascadingScanDefinition({ - name, - parentScan: parentScan, - generatedBy, - scanType, - parameters, - env, - scanLabels, - scanAnnotations - }); + for (const cascadedScan of cascadedScans) { + const cascadingScanDefinition = getCascadingScanDefinition(cascadedScan, parentScan); expect(Object.entries(parentScan.metadata.annotations).every(([label, value]) => cascadingScanDefinition.metadata.annotations[label] === value @@ -639,12 +585,12 @@ test("should copy scanLabels from CascadingRule to cascading scan", () => { sslyzeCascadingRules ); - const { name, scanType, parameters, generatedBy, env, scanLabels, scanAnnotations } = cascadedScans[0] + const cascadedScan = cascadedScans[0] expect(cascadedScans).toMatchInlineSnapshot(` Array [ Object { - "cascades": null, + "cascades": Object {}, "env": undefined, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", @@ -662,16 +608,7 @@ test("should copy scanLabels from CascadingRule to cascading scan", () => { ] `); - const cascadingScanDefinition = getCascadingScanDefinition({ - name, - parentScan: parentScan, - generatedBy, - scanType, - parameters, - env, - scanLabels, - scanAnnotations - }); + const cascadingScanDefinition = getCascadingScanDefinition(cascadedScan, parentScan); expect(Object.entries(sslyzeCascadingRules[0].spec.scanLabels).every(([label, value]) => cascadingScanDefinition.metadata.labels[label] === value @@ -703,12 +640,12 @@ test("should copy scanAnnotations from CascadingRule to cascading scan", () => { sslyzeCascadingRules ); - const { name, scanType, parameters, generatedBy, env, scanLabels, scanAnnotations } = cascadedScans[0] + const cascadedScan = cascadedScans[0] expect(cascadedScans).toMatchInlineSnapshot(` Array [ Object { - "cascades": null, + "cascades": Object {}, "env": undefined, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", @@ -726,16 +663,7 @@ test("should copy scanAnnotations from CascadingRule to cascading scan", () => { ] `); - const cascadingScanDefinition = getCascadingScanDefinition({ - name, - parentScan: parentScan, - generatedBy, - scanType, - parameters, - env, - scanLabels, - scanAnnotations - }); + const cascadingScanDefinition = getCascadingScanDefinition(cascadedScan, parentScan); expect(Object.entries(sslyzeCascadingRules[0].spec.scanAnnotations).every(([label, value]) => cascadingScanDefinition.metadata.annotations[label] === value diff --git a/hooks/cascading-scans/hook.ts b/hooks/cascading-scans/hook.ts index 573e073aaa..7f8c17afdb 100644 --- a/hooks/cascading-scans/hook.ts +++ b/hooks/cascading-scans/hook.ts @@ -28,17 +28,8 @@ export async function handle({ scan, getFindings }: HandleArgs) { const cascadingScans = getCascadingScans(scan, findings, cascadingRules); - for (const { name, scanType, parameters, generatedBy, env, scanLabels, scanAnnotations } of cascadingScans) { - const cascadingScanDefinition = getCascadingScanDefinition({ - name, - parentScan: scan, - generatedBy, - scanType, - parameters, - env, - scanLabels, - scanAnnotations - }); + for (const cascadingScan of cascadingScans) { + const cascadingScanDefinition = getCascadingScanDefinition(cascadingScan, scan); await startSubsequentSecureCodeBoxScan(cascadingScanDefinition); } } @@ -127,7 +118,7 @@ function getCascadingScan( parameters: parameters.map(parameter => Mustache.render(parameter, templateArgs) ), - cascades: null, + cascades: parentScan.spec.cascades, generatedBy: cascadingRule.metadata.name, env, scanLabels: cascadingRule.spec.scanLabels === undefined ? {} : diff --git a/hooks/cascading-scans/scan-helpers.ts b/hooks/cascading-scans/scan-helpers.ts index 9ba0f19b14..0be88f9f47 100644 --- a/hooks/cascading-scans/scan-helpers.ts +++ b/hooks/cascading-scans/scan-helpers.ts @@ -84,14 +84,14 @@ export interface ExtendedScanSpec extends ScanSpec { export function getCascadingScanDefinition({ name, - parentScan, scanType, parameters, generatedBy, env, + cascades, scanLabels, scanAnnotations - }) { + }: ExtendedScanSpec, parentScan: Scan) { function mergeInherited(parentProps, ruleProps, inherit: boolean = true) { if (!inherit) { parentProps = {}; @@ -146,7 +146,7 @@ export function getCascadingScanDefinition({ spec: { scanType, parameters, - cascades: parentScan.spec.cascades, + cascades, env, } }; From 7aac0aa076a3c15dbb9dea8a776812e66ec3270c Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Tue, 22 Jun 2021 11:28:41 +0200 Subject: [PATCH 2/3] Cascading Scans: add finding ID to cascadedScan Signed-off-by: Jop Zitman --- hooks/cascading-scans/hook.test.js | 71 +++++++++++++++++++++++++++ hooks/cascading-scans/hook.ts | 1 + hooks/cascading-scans/scan-helpers.ts | 8 ++- 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/hooks/cascading-scans/hook.test.js b/hooks/cascading-scans/hook.test.js index 9e14091249..2d7c511365 100644 --- a/hooks/cascading-scans/hook.test.js +++ b/hooks/cascading-scans/hook.test.js @@ -82,6 +82,16 @@ test("Should create subsequent scans for open HTTPS ports (NMAP findings)", () = Object { "cascades": Object {}, "env": undefined, + "finding": Object { + "attributes": Object { + "hostname": "foobar.com", + "port": 443, + "service": "https", + "state": "open", + }, + "category": "Open Port", + "name": "Port 443 is open", + }, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", "parameters": Array [ @@ -145,6 +155,17 @@ test("Should not try to do magic to the scan name if its something random", () = Object { "cascades": Object {}, "env": undefined, + "finding": Object { + "attributes": Object { + "hostname": undefined, + "ip_address": "10.42.42.42", + "port": 443, + "service": "https", + "state": "open", + }, + "category": "Open Port", + "name": "Port 443 is open", + }, "generatedBy": "tls-scans", "name": "foobar.com-tls-scans", "parameters": Array [ @@ -212,6 +233,16 @@ test("Should not crash when the annotations are not set", () => { Object { "cascades": Object {}, "env": undefined, + "finding": Object { + "attributes": Object { + "hostname": "foobar.com", + "port": 443, + "service": "https", + "state": "open", + }, + "category": "Open Port", + "name": "Port 443 is open", + }, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", "parameters": Array [ @@ -268,6 +299,16 @@ test("Should copy ENV fields from cascadingRule to created scan", () => { }, }, ], + "finding": Object { + "attributes": Object { + "hostname": "foobar.com", + "port": 443, + "service": "https", + "state": "open", + }, + "category": "Open Port", + "name": "Port 443 is open", + }, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", "parameters": Array [ @@ -334,6 +375,16 @@ test("Should allow wildcards in cascadingRules", () => { Object { "cascades": Object {}, "env": undefined, + "finding": Object { + "attributes": Object { + "hostname": "foobar.com", + "port": 8443, + "service": "https-alt", + "state": "open", + }, + "category": "Open Port", + "name": "Port 8443 is open", + }, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", "parameters": Array [ @@ -592,6 +643,16 @@ test("should copy scanLabels from CascadingRule to cascading scan", () => { Object { "cascades": Object {}, "env": undefined, + "finding": Object { + "attributes": Object { + "hostname": "foobar.com", + "port": 443, + "service": "https", + "state": "open", + }, + "category": "Open Port", + "name": "Port 443 is open", + }, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", "parameters": Array [ @@ -647,6 +708,16 @@ test("should copy scanAnnotations from CascadingRule to cascading scan", () => { Object { "cascades": Object {}, "env": undefined, + "finding": Object { + "attributes": Object { + "hostname": "foobar.com", + "port": 443, + "service": "https", + "state": "open", + }, + "category": "Open Port", + "name": "Port 443 is open", + }, "generatedBy": "tls-scans", "name": "sslyze-foobar.com-tls-scans", "parameters": Array [ diff --git a/hooks/cascading-scans/hook.ts b/hooks/cascading-scans/hook.ts index 7f8c17afdb..260826b680 100644 --- a/hooks/cascading-scans/hook.ts +++ b/hooks/cascading-scans/hook.ts @@ -125,6 +125,7 @@ function getCascadingScan( mapValues(cascadingRule.spec.scanLabels, value => Mustache.render(value, templateArgs)), scanAnnotations: cascadingRule.spec.scanAnnotations === undefined ? {} : mapValues(cascadingRule.spec.scanAnnotations, value => Mustache.render(value, templateArgs)), + finding }; } diff --git a/hooks/cascading-scans/scan-helpers.ts b/hooks/cascading-scans/scan-helpers.ts index 0be88f9f47..34f3b280af 100644 --- a/hooks/cascading-scans/scan-helpers.ts +++ b/hooks/cascading-scans/scan-helpers.ts @@ -24,6 +24,7 @@ export interface Finding { severity: string; osi_layer: string; attributes: Map; + id: string; } export interface CascadingRule { @@ -80,6 +81,9 @@ export interface ExtendedScanSpec extends ScanSpec { scanAnnotations: { [key: string]: string; }; + + // Finding that triggered the scan + finding: Finding } export function getCascadingScanDefinition({ @@ -90,7 +94,8 @@ export function getCascadingScanDefinition({ env, cascades, scanLabels, - scanAnnotations + scanAnnotations, + finding }: ExtendedScanSpec, parentScan: Scan) { function mergeInherited(parentProps, ruleProps, inherit: boolean = true) { if (!inherit) { @@ -126,6 +131,7 @@ export function getCascadingScanDefinition({ annotations: { "securecodebox.io/hook": "cascading-scans", "cascading.securecodebox.io/parent-scan": parentScan.metadata.name, + "cascading.securecodebox.io/matched-finding": finding.id, "cascading.securecodebox.io/chain": [ ...cascadingChain, generatedBy From b1a2f6d7b9ad0a5bcb85f2bb816eb0b20341d47c Mon Sep 17 00:00:00 2001 From: Jop Zitman Date: Tue, 22 Jun 2021 12:02:55 +0200 Subject: [PATCH 3/3] Cascading Scans: add finding ID test Signed-off-by: Jop Zitman --- hooks/cascading-scans/hook.test.js | 72 ++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/hooks/cascading-scans/hook.test.js b/hooks/cascading-scans/hook.test.js index 2d7c511365..4477164309 100644 --- a/hooks/cascading-scans/hook.test.js +++ b/hooks/cascading-scans/hook.test.js @@ -787,3 +787,75 @@ test("should properly parse template values in scanLabels and scanAnnotations", expect(scanAnnotations).toEqual(result) }) +test("should copy proper finding ID into annotations", () => { + const findings = [ + { + name: "Port 12345 is open", + category: "Open Port", + attributes: { + state: "open", + hostname: "foobar.com", + port: 12345, + service: "unknown" + }, + id: "random-id" + }, + { + name: "Port 443 is open", + category: "Open Port", + attributes: { + state: "open", + hostname: "foobar.com", + port: 443, + service: "https" + }, + id: "f0c718bd-9987-42c8-2259-73794e61dd5a" + } + ]; + + const cascadedScans = getCascadingScans( + parentScan, + findings, + sslyzeCascadingRules + ); + + const cascadedScan = cascadedScans[0] + + expect(cascadedScans).toMatchInlineSnapshot(` + Array [ + Object { + "cascades": Object {}, + "env": undefined, + "finding": Object { + "attributes": Object { + "hostname": "foobar.com", + "port": 443, + "service": "https", + "state": "open", + }, + "category": "Open Port", + "id": "f0c718bd-9987-42c8-2259-73794e61dd5a", + "name": "Port 443 is open", + }, + "generatedBy": "tls-scans", + "name": "sslyze-foobar.com-tls-scans", + "parameters": Array [ + "--regular", + "foobar.com:443", + ], + "scanAnnotations": Object {}, + "scanLabels": Object {}, + "scanType": "sslyze", + }, + ] + `); + + const cascadingScanDefinition = getCascadingScanDefinition(cascadedScan, parentScan); + + expect(Object.entries(cascadingScanDefinition.metadata.annotations).every(([label, value]) => { + if (label === "cascading.securecodebox.io/matched-finding") { + return value === "f0c718bd-9987-42c8-2259-73794e61dd5a"; + } else return true; + } + )).toBe(true) +});