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

Refactor shell rules #301

Merged
merged 20 commits into from
Nov 28, 2017
Merged

Refactor shell rules #301

merged 20 commits into from
Nov 28, 2017

Conversation

mstemm
Copy link
Contributor

@mstemm mstemm commented Nov 17, 2017

Refactor the shell-related rules to reduce false positives. These changes significantly decrease the scope of the rules so they trigger only for shells spawned below specific processes instead of anywhere. See the commit messages for more details.

Refactoring the shell related rules to avoid FPs. Instead of considering
all shells suspicious and trying to carve out exceptions for the
legitimate uses of shells, only consider shells spawned below certain
processes suspicious.

The set of processes is a collection of commonly used web servers,
databases, nosql document stores, mail programs, message queues, process
monitors, application servers, etc.

Also, runsv is also considered a top level process that denotes a
service. This allows a way for more flexible servers like ad-hoc nodejs
express apps, etc to denote themselves as a full server process.
spawn_shell is now a silent action. its replacement is
spawn_shell_under_httpd, which respawns itself as httpd and then runs a
shell.

db_program_spawn_binaries now runs ls instead of a shell so it only
matches db_program_spawn_process.
Start the express server using runit's runsv, which allows falco to
consider any shells run by it as suspicious.
In draios/sysdig#757 the path argument for mkdir
moved to the second argument. This only became visible in the unit tests
once the trace files were updated to reflect the other shell rule
changes--the trace files had the old format.
Shell in container doesn't exist any longer and its functionality has
been subsumed by run shell untrusted.
In some cases, these are run below a service runsv so we still need
exceptions for them.
There's enough evidence of people spawning general commands that we
can't protect it.
Move the nginx exception to the main rule instead of the
protected_shell_spawner macro. Also add erl_child_setup (related to
rabbitmq) as an allowed shell spawner.
All off these are either below nginx, httpd, or runsv but should still
be allowed to spawn shells.
Skip shells when any process ancestor (parent, gparent, etc) is a
package management binary. This includes the program needrestart. This
is a deep search but should prevent a lot of other more detailed
exceptions trying to find the specific scripts run as a part of
installations.
Serf is a service discovery tool and can in some cases be spawned by
apache/nginx. Also allow shells that are just checking the status of
pids via kill -0.
Add several exclusions back from the shell in container rule. These are
all allowed shell spawns that happen to be below
nginx/fluentd/apache/etc.
This saves space as well as cleanup. I haven't yet removed the
macros/lists used by these rules and not used anywhere else. I'll do
that cleanup in a separate step.
Add back the exclusions based on command lines, using the existing set
of command lines.
Of note is runsv, which means it can directly run shells (the ./run and
./finish scripts), but the things it runs can not.
We'll detect the first shell and not any other shells it spawns.
In some cases, the initial process for a container can have a parent
"runc:[0:PARENT]", so also allow those cases to count as a container
entrypoint.
Use the container_entrypoint macro to denote entering a container and
also allow exe to be one of the processes that's the parent of an
entrypoint.
@mstemm mstemm merged commit d6d975e into dev Nov 28, 2017
@mstemm mstemm deleted the refactor-shell-rules branch November 28, 2017 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant