Make Inheritlabels apply to scans and not only pods #683

Activity

1. RixTmobilender

   added

   bugBugsBugs

   on Sep 29, 2021
2. EndPositive commented on Sep 29, 2021

   

   EndPositive

   on Sep 29, 2021

   Contributor

   More actions

   Investigating 👀

   👍React with 👍3Reacted by Jannik Hollenbach, Yannik Fuhrmeister and Robert Felber
3. EndPositive commented on Sep 29, 2021

   

   EndPositive

   on Sep 29, 2021

   Last edited by EndPositive

   Contributor

   More actions

   Hi @RixTmobilender, thanks for trying out secureCodeBox!

   Unfortunately I wasn't able to reproduce your issue. There's actually a unit test for this specific scenario.

   secureCodeBox/hooks/cascading-scans/hook/hook.test.js

   Lines 386 to 418 in 79a279a

   	test("should not copy labels if inheritLabels is set to false", () => {
   	parentScan.metadata.labels = {
   	organization: "OWASP",
   	location: "barcelona",
   	vlan: "lan"
   	};
   	parentScan.spec.cascades.inheritLabels = false;
   	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
   	);
   	for (const cascadedScan of cascadedScans) {
   	expect(Object.entries(parentScan.metadata.labels).every(([label, value]) =>
   	cascadedScan.metadata.labels[label] === value
   	)).toBe(false)
   	}
   	});

   I have a feeling this behaviour is caused by tanka.dev 's garbage collection. As said in tanka's docs, it automatically adds this label for each created resource.

   @RixTmobilender could you check whether you can reproduce the behavior with labels other than tanka.dev/environment?

   Regarding the deletion of cascading scans: The cascading scans hook automatically sets the ownerReferences on each cascading scans, as visible in your example:

   apiVersion: execution.securecodebox.io/v1
   kind: Scan
   metadata:
     [...]
     labels:
       tanka.dev/environment: 6a2b816247aad89622120ae34db00790e8b0e49713341819
     name: securecodebox-scanners-network-securecodebox-scanners-zap-6dmpw
     ownerReferences:
     - apiVersion: execution.securecodebox.io/v1
       blockOwnerDeletion: true
       controller: true
       kind: Scan
       name: securecodebox-scanners-network
       uid: 6069b242-e596-48a4-8e61-b52c38f8a02c
     [...]

   With the ownerReference set, if the parent scan is deleted, the Kubernetes garbage collector will automatically delete all cascaded scans.

   So either disable Tanka's garbage collection or we have to look into if we can run the cascading scans hook without setting ownerReferences on cascading scans. I don't think this reference is used anywhere else but for garbage collection, so it should be no problem to implement.

   👍React with 👍1Reacted by Robert Felber
4. RixTmobilender commented on Sep 29, 2021

   

   RixTmobilender

   on Sep 29, 2021

   Last edited by RixTmobilender

   Author

   More actions

   I have redeployed my project from scratch with the following changes:

   • Completely disabled tanka's garbage collection (both for the operator and the actual scans)
   • Added a custom label test_label: hello on the parent nmap scan
   • I updated operator image tag to latest (I actually tried it with v3.1.1 and got the same results)

   I am running an nmap scan that cascades into sslyze and zap-advanced, I dont think it's relevant but I have disabled the default cascading rule for zap-advanced and created my own in order to selectively drop http targets and run https-only scans.

   • K8s manifests used for the operator stack deployment: operator.yaml.txt
   • K8s manifests used for the scan stack deployment: scans.yaml.txt
   • Output of kubectl get scans -n mynamespace -o yaml: get_scans.yaml.txt

   I can confirm that both cascading scans have inherited their parent's test_label despite inheritLabels: false being present in the parent nmap.

   If there's anything else I can supply please let me know 😀.
   Thanks a lot and congrats on an amazing project.

   🎉React with 🎉1Reacted by Robert Felber

   👀React with 👀1Reacted by Jop Zitman
5. rfelber

   added

   questionFurther information is requestedFurther information is requested

   cascading-scans

   and removed

   bugBugsBugs

   on Sep 30, 2021
6. rfelber

   assigned

   EndPositive

   on Sep 30, 2021
7. EndPositive commented on Sep 30, 2021

   

   EndPositive

   on Sep 30, 2021

   Last edited by EndPositive

   Contributor

   More actions

   @RixTmobilender Interestingly, there was a difference between the applied Scan object and the actual Scan in the cluster.

   Original:

   apiVersion: execution.securecodebox.io/v1
   kind: Scan
   metadata:
     labels:
       test_label: hello
     name: securecodebox-scanners-network
     namespace: myscannamespace
   spec:
     cascades:
       inheritLabels: false
       matchExpressions: [...]
     parameters:
     - scan.me
     scanType: nmap

   Actual:

   apiVersion: execution.securecodebox.io/v1
   kind: Scan
   metadata:
     annotations:
       kubectl.kubernetes.io/last-applied-configuration: |
         {"apiVersion":"execution.securecodebox.io/v1","kind":"Scan","metadata":{"annotations":{},"labels":{"test_label":"hello"},"name":"securecodebox-scanners-network","namespace":"myscannamespace"},"spec":{"cascades":{"inheritLabels":false,"matchExpressions":[{"key":"securecodebox.io/intensive","operator":"In","values":["light","medium","high"]},{"key":"securecodebox.io/invasive","operator":"In","values":["non-invasive","invasive"]}]},"parameters":["scan.me"],"scanType":"nmap"}}
     creationTimestamp: "2021-09-29T22:49:18Z"
     finalizers:
     - s3.storage.securecodebox.io
     generation: 2
     labels:
       test_label: hello
     name: securecodebox-scanners-network
     namespace: myscannamespace
     resourceVersion: "1020544"
     uid: fd56d06b-6d54-4bfa-8fc4-a637d844dd5e
   spec:
     cascades:
       matchExpressions: [..]
     parameters:
     - scan.me
     scanType: nmap
   status: [...]

   After some more investigating, looks like the operator deletes the inheritLabels field if it is set to false. Then the cascading scan hook receives an undefined value and assumes the default true. Making it impossible to set inheritLabels to false while creating a resource. I've created #687 !

   Thanks for the extensive logs and detailed explanations!

   😄React with 😄1Reacted by RixTmobilender
8. J12934

   closed this as completedin #687on Oct 1, 2021