fix(query): Extend containers_running_as_root k8s rule to work if no securityContext is defined

[Churro]

Problem

• The condition: pod.securityContext.runAsNonRoot == false and pod.securityContext.runAsUser undefined, is missing. This leads to false positives.
• The current rule implementation requires that a securityContext is defined at pod level so that checkRootParent is evaluated. The same applies to checkUserContainer. Patching the rule such that securityContext may or may not be specified is tricky and would potentially break the rule logic.

Proposed Changes

• Rewrite / simplify the current rule implementation by inverting the logic. Instead of looking for spots where runAsUser > 0 or runAsNonRoot == true (and performing a no-op recursion to checkRootContainer), the opposite is now looked up

I submit this contribution under the Apache-2.0 license.

[kicsbot]

Scan submitted to Checkmarx

[cx-joao-reigota]

Hi @Churro, everything looks good to me, the only thing I would like to request here is the use of the searchLine

Thank you for your great contributions

[Churro]

Hi @joaoReigota1, are you sure searchLine is needed in this case? securityContext is the only attribute that is "optional" in searchKey and in case it doesn't exist, the vulnerability line detector successfully targets the container's name. At least it does so for all the cases I've tested...

My understanding is that searchLine particularly compensates for cases in which multiple keys, separated by dots, may be undefined, e.g., resources.requests, or capabilities.drop. Is there a case I'm missing?

[cx-joao-reigota]

Hi @Churro,
Thank you, for your comment
I would suggest using the searchLine even if securityContext is the only optional field in the searchKey,
take this for example

1: apiVersion: v1
2: kind: Pod
3: metadata:
4:   name: security-context-demo-2
5: spec:
6:   containers:
7:   - name: sec-ctx-demo-1
8:     image: gcr.io/google-samples/node-hello:1.0
9:   - name: sec-ctx-demo-2
10:   image: gcr.io/google-samples/node-hello:1.0
11:   securityContext:
12:     runAsUser: 0
13:     allowPrivilegeEscalation: false
14:     runAsNonRoot: false

Having this searchKey:

metadata.name={{security-context-demo-2}}.spec.containers.name={{sec-ctx-demo-1}}.securityContext 

The issue is in the first container, following our searchKey until the name we get to line 7 where we have a .securityContext, which means, look at the lines under for a securityContext that is present in line 11. So the searchKey will now point to line 11 in the second container.
Unfortunately, our serchKey has some limitations and arrays is one of them.
All put together I would say the searchLine is more accurate than the searchKey, but since Docker still does not support it, the searchKey is the only one required.

[kicsbot]

Checkmarx SAST - Scan Summary & Details

Cx-SAST Summary

Total of 5 vulnerabilities
 0 High
 0 Medium
 5 Low
 0 Info

Violation Summary

No policy violation found

[Churro]

Hi @joaoReigota1, thank you for coming up with a concrete example to demonstrate the benefit of using searchLine here. That makes total sense for me and I've now added searchLine + your sample snippet as an additional testcase.

[rafaela-soares]

LGTM 🚀 Thank you so much, @Churro!

[cx-joao-reigota]

LGTM