cyberark / KubiScan

A tool to scan Kubernetes cluster for risky permissions
GNU General Public License v3.0
1.31k stars 130 forks source link

Fix error when a secret with name 'token' is not a jwt. #61

Closed NeuronAddict closed 1 year ago

NeuronAddict commented 1 year ago

When a secret has a data with the name 'token' that is not a jwt token, this will raise an exception and end the script (even if scan is not finished).

Desired Outcome

When a secret has a key named 'token', sometimes its not a real jwt token.

For example, this is the case on openshift with the secret 'telemeter-client' in namespace 'openshift-monitoring'.

This PR print a warning to give the user a chance to investigate the issue.

This solution has disadvantages :

I don't know if its better to silently ignore the error or to display a warning

Implemented Changes

Describe how the desired outcome above has been achieved with this PR. In particular, consider:

(see engine.utils.get_jwt_and_decode)

When a secret is discovered, we check the name and if its 'token', we try to decode the jwt and return the user.

With this commit, and error on decode the jwt will just print a warning and return None, just like a secret without the token key.

There is other ways to manage the issue whitout a try/except :

Connected Issue/Story

Resolves #[relevant GitHub issue(s), e.g. 76]

CyberArk internal issue ID: [insert issue ID]

Definition of Done

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

Changelog

Test coverage

Documentation

Behavior

Security

g3rzi commented 1 year ago

Hi @NeuronAddict , thank you for your fix :) This issue was opened in #59 and we gave a different fix because the problem is not only because of the exception: https://github.com/cyberark/KubiScan/issues/59#issuecomment-1385749113

We still didn't decide what will be the final fix but here is a temporary repository with a fix by my colleague: https://github.com/2niknatan/KubiScan

(It's only 1 commit ahead of the main repository) Can you check it and see if it solves it?

NeuronAddict commented 1 year ago

Hi @g3rzi ,

Thank you for the response, I checked with the 2niknatan, it works with the fix (no error, event with a secret with 'token').

The issue describe the same problem than me.

I will test this version.

g3rzi commented 1 year ago

We now added the PR (#61) for the fix of #59, it is also the fix for your original PR. I want you to know that I appreciate your contribution, we didn't add it because we already had a fix but didn't publish the PR.

Thanks :)