Fix iteration for rolebindings without subjects

snorwin commented 2 years ago

Desired Outcome

In OpenShift, there are RoleBindings without subjects, which leads to problems:

Traceback (most recent call last):
  File "/KubiScan/KubiScan.py", line 639, in <module>
    main()
  File "/KubiScan/KubiScan.py", line 574, in main
    print_all(days=args.less_than, priority=args.priority, read_token_from_container=args.deep)
  File "/KubiScan/KubiScan.py", line 140, in print_all
rint_all_risky_subjects(priority=priority)
  File "/KubiScan/KubiScan.py", line 126, in print_all_risky_subjects
    subjects = engine.utils.get_all_risky_subjects()
  File "/KubiScan/engine/utils.py", line 221, in get_all_risky_subjects
    for user in risky_rolebinding.subjects:
TypeError: 'NoneType' object is not iterable
command terminated with exit code 1

Implemented Changes

Add checks for subjects in RoleBindings not equals None in order to prevet TypeError: 'NoneType' object is not iterable. I saw that you added the "ApiClientTemp" for the rules in Roles, but I don't think it's worth cop/pasting more external code, so I decided to just add checks.

Connected Issue/Story

Definition of Done

At least 1 todo must be completed in the sections below for the PR to be merged.

Changelog

[ ] The CHANGELOG has been updated, or
[x] This PR does not include user-facing changes and doesn't require a CHANGELOG update

Test coverage

[ ] This PR includes new unit and integration tests to go with the code changes, or
[x] The changes in this PR do not require tests

Documentation

[ ] Docs (e.g. READMEs) were updated in this PR
[ ] A follow-up issue to update official docs has been filed here: [insert issue ID]()
[x] This PR does not require updating any documentation

Behavior

[ ] This PR changes product behavior and has been reviewed by a PO, or
[ ] These changes are part of a larger initiative that will be reviewed later, or
[x] No behavior was changed with this PR

Security

[ ] Security architect has reviewed the changes in this PR,
[ ] These changes are part of a larger initiative with a separate security review, or
[x] There are no security aspects to these changes

g3rzi commented 2 years ago

Great, thank you! We will have a look at that.

g3rzi commented 2 years ago

We don't have OpenShift, so we are trying to reproduce it without OpenShift. In your case you had risky_rolebinding.subjects=None ? Because I thought that the for loop won't access the loop if they are None.

snorwin commented 2 years ago

Example of a ClusterRoleBinding in OCP4 causing the issue:

kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1
metadata:
  name: 'system:node'
  labels:
    kubernetes.io/bootstrapping: rbac-defaults
  annotations:
    rbac.authorization.kubernetes.io/autoupdate: 'true'
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: 'system:node'

g3rzi commented 2 years ago

Thanks, we merged. We will change the or [] to is not None before the loop so it will be readable

cyberark / KubiScan