falcosecurity / rules

Falco rule repository
https://falcosecurity.github.io/rules/
Apache License 2.0
91 stars 64 forks source link

Update rule "Redirect STDOUT/STDIN to Network Connection in Container" #131

Closed incertum closed 5 months ago

incertum commented 10 months ago

Motivation

@allanembedded would you be in a position to help improve the upstream rule "Redirect STDOUT/STDIN to Network Connection in Container" based on the libs improvements from https://github.com/falcosecurity/libs/pull/1077? Thanks a bunch in advance!

CC @darryk10 @loresuso

allanembedded commented 10 months ago

@incertum thanks for kicking me about this.

I've put together a proposed change in https://github.com/allanembedded/falco-rules/commit/e92ce21b8f53ddb49d0aaf74b06a4e3155bd726e, however I thought it was worth getting your input on whether or not it's an issue if the new rule means we don't trigger the event if only one or two of std{in,out,err} are redirected?

I mean, it's kind of the point of the change however it is a modification that existing users of the rule could be impacted by.

incertum commented 10 months ago

Thanks @allanembedded, agreed we should do some more experimentations, like what's the minimum that needs to be true for even a more exotic reverse shell using this general tactic? Based on that ensure we still only alert once instead of multiple times? Bunch of additional conditionals for various scenarios would be totally fair game from my perspective as it will improve the logging quality.

@darryk10?

darryk10 commented 10 months ago

I would go with a new rule for this, keeping the old one as it is. I would just place the new one above since a more precise use case to avoid conflicts. I think it makes sense to keep them. Since the overall change has been done to reduce the number of alerts, after some testing we might also decide to keep the new one enabled by default and set the old one as disabled. However, before to proceed with this change I would advice to test the new rule with different rev shell types to see the detection.

incertum commented 10 months ago

@allanembedded and @darryk10 should we instead of changing the rule condition, add this suggestion into the description as tuning advice? And yes the testing we need is non significant before cutting over.

I would prefer not to add a new rule.

allanembedded commented 10 months ago

I'm fine with the approach of just amending the description for now.

FWIW, I think a better future approach for reverse shell overall is something like:

spawned_process and shell_procs and (fd.types[0] in (ip_sockets) or fd.types[1] in (ip_sockets) or fd.types[2] in (ip_sockets))

This is better than hanging off of dup events since there's only one execve for a process and we can check for any combination of std{out,err,in} being redirected once. Unfortunately I believe this will require improvements to the file descriptor table handling across clones/execs, @Andreagit97 can correct me if I'm wrong about that though.

incertum commented 10 months ago

Perfect I just approved the PR. Would it be ok to keep this issue open until the next release and hopefully we can then update the upstream condition?

poiana commented 7 months ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

poiana commented 6 months ago

Stale issues rot after 30d of inactivity.

Mark the issue as fresh with /remove-lifecycle rotten.

Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle rotten