Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(plugins/k8saudit/rules) add detection for portforwarding #375

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

RichardoC
Copy link
Contributor

@RichardoC RichardoC commented Nov 15, 2023

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area plugins

What this PR does / why we need it:
Currently there aren't any detections for port forwarding, which isn't great. Port forwarding can be used to avoid controls such as service meshes, and can be used for data exfiltration. There's rarely a good reason for anyone to be using these regularly so it should have a detection, just like exec already does.

For anyone wondering, you can only port forward a pod. When you kubectl port forward svc/myservice it actually checks the selectors for the service and port mapping, and port forwards one of the pods which matches those selectors, on the required port after mapping through the service. This is why I'm only reporting the pod that is port forwarded, there's no way to know if the user was trying to port forward a pod, or service unless we correlated back to whether this user had done a service lookup and then a pod list before issuing this API call.

Which issue(s) this PR fixes:

Fixes #230

Special notes for your reviewer:

This hasn't been tested locally, it's based on the Kubernetes API spec so should be tested before merging

@poiana
Copy link
Contributor

poiana commented Nov 15, 2023

Welcome @RichardoC! It looks like this is your first PR to falcosecurity/plugins 🎉

@poiana poiana added the size/S label Nov 15, 2023
Copy link

Rules files suggestions

k8s_audit_rules.yaml

Comparing 73180db9813022bc5eb41a3cf81729c72a32baaa with latest tag k8saudit-0.6.1

Minor changes:

  • Rule port-forward has been added
  • Macro user_known_portforward_activities has been added

@Andreagit97
Copy link
Member

Thank you for this, not a big expert here but we will try to take a look ASAP

@loresuso
Copy link
Member

loresuso commented Nov 23, 2023

Hi @RichardoC and thanks for the contribution!
I do agree that this is something we need and the NOTICE priority. I think it might be worth it to introduce an allow list also at the namespace level
In any case, LGTM!

@RichardoC
Copy link
Contributor Author

Hi @RichardoC and thanks for the contribution! I do agree that this is something we need and the NOTICE priority. I think it might be worth it to introduce an allow list also at the namespace level In any case, LGTM!

I could add this, but nothing else in this file is allowlisted by namespace. Wouldn't it be better to have that as a macro for all rules on namespaced objects?

Also, did you have any luck testing these with a real cluster?

@loresuso
Copy link
Member

That's right, let's keep the namespace allow list for a second PR in case.
Re testing the rule, no I didn't have the chance to try it so far. We could follow this guide https://falco.org/docs/install-operate/third-party/learning/#temp-qa-integrations-to-minikube to set up Falco with the k8s audit log plugin in a k8s cluster to try it out.

@RichardoC
Copy link
Contributor Author

RichardoC commented Nov 24, 2023

Good news, after some faffing about got this working on minikube
To be clear, the faffing was me getting falco running with helm, and overriding the rules with the changes in this PR rather than anything else

image

@RichardoC
Copy link
Contributor Author

@loresuso Need anything on this?

@loresuso
Copy link
Member

Great news, thanks for trying it out!
not a maintainer here, @leogr mind approving if you agree?

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

A little disclaimer: I think it's okay to merge this. However, note that the new k8s ruleset will not released soon. We will likely release all new plugin versions near the next Falco release. We can open a follow-up PR if we need to introduce any minor change to this new rule before releasing it (like enforcing the Rules Maturity Framework policies).

cc @falcosecurity/rules-maintainers @falcosecurity/plugins-maintainers

@poiana
Copy link
Contributor

poiana commented Nov 27, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leogr, RichardoC

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana
Copy link
Contributor

poiana commented Nov 27, 2023

LGTM label has been added.

Git tree hash: c2015d0f0d40d13a871c23a97bb4896c3ef5a66b

@poiana poiana merged commit 028fa19 into falcosecurity:master Nov 27, 2023
11 checks passed
@RichardoC RichardoC deleted the #230-detect-port-forwards branch November 27, 2023 17:17
@RichardoC
Copy link
Contributor Author

@leogr are you planning to move these rules out to the rules repo?

@leogr
Copy link
Member

leogr commented Nov 28, 2023

@leogr are you planning to move these rules out to the rules repo?

No, we don't have any plans for that yet. We have just to understand if and how the Rules Maturity Framework applies to plugin rulesets. Eventually, it will have a minor impact on plugin rules (ie. probably we will just add maturity tags).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

k8s plugins should detect portforwards as well as execs
5 participants