rule(Change thread namespace): Modify condition to detect suspicious container activity #974

rung · 2019-12-16T17:04:36Z

Signed-off-by: Hiroki Suezawa [email protected]

What type of PR is this?
/kind rule-update

Any specific area of the project related to this PR?
/area rules

What this PR does / why we need it:

What this PR does
- Modify condition to detect some process names within a container.
- Add evt.dir to avoid detection of same syscall
Why we need it
- The current rule exclude some process names used on host
  - i.e. k8s_binaries, nsenter
- But nsenter could be used to do privilege escalation
  - https://twitter.com/mauilion/status/1129468485480751104
  - (nsenter is included in many container images like alpine by default)
- So I would like to change condition to detect suspicious container activity.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

rule(Change thread namespace): Modify condition to detect suspicious container activity

rung · 2019-12-16T17:04:57Z

Procedure Step

Start a container

kubectl run malicious-container --restart=Never -ti --rm --image lol --overrides '{"spec":{"hostPID": true, "containers":[{"name":"1","image":"alpine","command":["sh"],"stdin": true,"tty":true,"securityContext":{"privileged":true}}]}}'

Change namespace to host

nsenter --mount=/proc/1/ns/mnt -- /bin/bash

Sample Output

{"output":"16:46:42.320531675: Notice Namespace change (setns) by unexpected program (user=root command=nsenter --mount=/proc/1/ns/mnt -- /bin/bash parent=sh k8s.ns=default k8s.pod=malicious-container container=3172964a195b container_id=3172964a195b image=alpine:latest) k8s.ns=default k8s.pod=malicious-container container=3172964a195b k8s.ns=default k8s.pod=malicious-container container=3172964a195b","priority":"Notice","rule":"Change thread namespace","time":"2019-12-16T16:46:42.320531675Z", "output_fields": {"container.id":"3172964a195b","container.image.repository":"alpine","container.image.tag":"latest","evt.time":1576514802320531675,"k8s.ns.name":"default","k8s.pod.name":"malicious-container","proc.cmdline":"nsenter --mount=/proc/1/ns/mnt -- /bin/bash","proc.pname":"sh","user.name":"root"}}

Kaizhe · 2019-12-16T19:34:37Z

rules/falco_rules.yaml

-    evt.type = setns
-    and not proc.name in (docker_binaries, k8s_binaries, lxd_binaries, sysdigcloud_binaries,
-                          sysdig, nsenter, calico, oci-umount, network_plugin_binaries)
+    evt.type=setns and evt.dir=<


I like this change.evt.dir=<

Kaizhe · 2019-12-16T19:39:30Z

/lgtm

poiana · 2019-12-16T19:39:34Z

LGTM label has been added.

Git tree hash: c19ce5e70645012684c11623453c3f717782b2ef

rung · 2019-12-18T00:45:42Z

I'll check why CI failed.

leodido

🚒

rung · 2019-12-27T02:26:54Z

(hmm, I tried finding the reason to get error, but I haven't solved it yet)

Kaizhe · 2019-12-27T05:31:55Z

@mstemm can you please help? The rule changes might break the existing tests. It would be great that you can point us to the right place. Thanks!

fntlnz · 2020-02-25T09:50:08Z

Closing and reopening this to let this test against the new CI

rung · 2020-02-25T10:32:11Z

oh, very sorry. I thought this was merged 🙇🙇.
I'll fix the conflict.

…container activity Signed-off-by: Hiroki Suezawa <[email protected]>

rung · 2020-02-25T11:18:20Z

rules/falco_rules.yaml

-                          sysdig, nsenter, calico, oci-umount, cilium-cni, network_plugin_binaries)
+    evt.type=setns and evt.dir=<
+    and not (container.id=host and proc.name in (docker_binaries, k8s_binaries, lxd_binaries, nsenter))
+    and not proc.name in (sysdigcloud_binaries, sysdig, calico, oci-umount, cilium-cni, network_plugin_binaries)


This line was conflicted because of the PR (48a0f51), and I fixed the conflict.

did you rebase?

Yes, after I got needs-rebase label from poiana. The point I changed in first commit is to add cilium-cni.

Signed-off-by: Hiroki Suezawa <[email protected]>

rung · 2020-02-26T01:21:38Z

Report on deep dive (why regression tests failed).

Reason 1

Related test cases
- https://github.com/falcosecurity/falco/blob/master/test/falco_traces.yaml.in#L29
- https://github.com/falcosecurity/falco/blob/master/test/falco_tests.yaml#L692
How to fix
- When I modified "Change thread namespace": 2 to "Change thread namespace": 1 on the above files, the test was successful.
Reason
- The scap file of regression test has 2 setns(enter and exit). but I added condition of evt.dir=<, so the count of detection should be 1 now.
- Related files
  - Regression test script
    - https://github.com/falcosecurity/falco/blob/master/test/run_regression_tests.sh#L36
  - scap file for regression test
    - https://s3.amazonaws.com/download.draios.com/falco-tests/traces-positive.zip

Reason 2

Error Message

15:30:21 ERROR| FAIL 55-/source/falco/test/falco_test.py:FalcoTest.test;no-kube-demo-47e1 -> TestFail: Detected 188 events when should have detected none

traces-negative/kube-demo.scap has many nsenter processes
- Old rule bypassed every nsenter, but my rule detects every nsenter on container. so the regression test failed.
- it seems kubelet(or hyperkube kubelet) use nsenter binary, (I think kubelet runs on host basically though, the regression test runs it on container)
Sample

DEBUG| [stdout] {"output":"04:46:52.887250174: Notice Namespace change (setns) by unexpected program (user=root command=nsenter --mount=/rootfs/proc/1/ns/mnt -- /bin/findmnt -o target --noheadings --target /var/lib/kubelet/pods/352aff16-2f8d-11e6-8bd9-0800270a6574/volumes/kubernetes.io~secret/default-token-pb8gr parent=hyperkube k8s-kubelet (id=af911d183773) container_id=af911d183773 image=<NA>:<NA>)","priority":"Notice","rule":"Change thread namespace","time":"2016-06-11T04:46:52.887250174Z", "output_fields": {"container.id":"af911d183773","container.image.repository":null,"container.image.tag":null,"container.name":"k8s-kubelet","evt.time":1465620412887250174,"proc.cmdline":"nsenter --mount=/rootfs/proc/1/ns/mnt -- /bin/findmnt -o target --noheadings --target /var/lib/kubelet/pods/352aff16-2f8d-11e6-8bd9-0800270a6574/volumes/kubernetes.io~secret/default-token-pb8gr","proc.pname":"hyperkube","user.name":"root"}}

How to fix
- add this line

not proc.pname in (hyperkube, kubelet)

rung · 2020-02-26T22:46:01Z

@leodido @Kaizhe Could you review this PR when you have time?
CI passed finally.
Thanks.

rung · 2020-03-06T02:33:20Z

@leodido @Kaizhe How about this PR?
Thank you! 👍

poiana · 2020-03-12T13:55:59Z

LGTM label has been added.

Git tree hash: 17f1406746d56ff07ee0721850368ffb50565b69

fntlnz · 2020-03-12T13:56:14Z

Good job as always @rung !

leodido

🔥

poiana · 2020-03-12T15:35:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, Kaizhe, leodido

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:

~~OWNERS~~ [fntlnz,leodido]

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

leodido · 2020-03-12T15:35:53Z

/milestone 0.21.0

rung · 2020-03-12T16:31:10Z

@fntlnz @leodido Thank you for your review!

poiana added release-note kind/rule-update dco-signoff: yes area/rules labels Dec 16, 2019

poiana requested review from Kaizhe and mfdii December 16, 2019 17:04

poiana added the size/S label Dec 16, 2019

Kaizhe reviewed Dec 16, 2019

View reviewed changes

poiana assigned Kaizhe Dec 16, 2019

poiana added the lgtm label Dec 16, 2019

poiana added the approved label Dec 16, 2019

leodido previously approved these changes Dec 18, 2019

View reviewed changes

poiana assigned leodido Dec 18, 2019

fntlnz changed the base branch from dev to master February 3, 2020 15:14

fntlnz closed this Feb 25, 2020

fntlnz reopened this Feb 25, 2020

poiana added the needs-rebase label Feb 25, 2020

rung dismissed leodido’s stale review via 8d02427 February 25, 2020 11:08

poiana added do-not-merge/contains-merge-commits and removed lgtm needs-rebase labels Feb 25, 2020

rule(Change thread namespace): change condition to detect suspicious …

9fcb4e6

…container activity Signed-off-by: Hiroki Suezawa <[email protected]>

poiana removed the do-not-merge/contains-merge-commits label Feb 25, 2020

rung commented Feb 25, 2020

View reviewed changes

poiana removed the approved label Feb 26, 2020

rule(Change thread namespace): fix regression test

7825a77

Signed-off-by: Hiroki Suezawa <[email protected]>

fntlnz approved these changes Mar 12, 2020

View reviewed changes

poiana assigned fntlnz Mar 12, 2020

poiana added the lgtm label Mar 12, 2020

poiana added the approved label Mar 12, 2020

leodido approved these changes Mar 12, 2020

View reviewed changes

poiana merged commit 3067af5 into falcosecurity:master Mar 12, 2020

poiana added this to the 0.21.0 milestone Mar 12, 2020

rung deleted the rule-mod-setns branch March 12, 2020 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rule(Change thread namespace): Modify condition to detect suspicious container activity #974

rule(Change thread namespace): Modify condition to detect suspicious container activity #974

rung commented Dec 16, 2019

rung commented Dec 16, 2019

Kaizhe Dec 16, 2019

Kaizhe commented Dec 16, 2019

poiana commented Dec 16, 2019

rung commented Dec 18, 2019

leodido left a comment

rung commented Dec 27, 2019

Kaizhe commented Dec 27, 2019

fntlnz commented Feb 25, 2020

rung commented Feb 25, 2020 •

edited

Loading

rung Feb 25, 2020

Kaizhe Feb 26, 2020

rung Feb 26, 2020

rung commented Feb 26, 2020 •

edited

Loading

rung commented Feb 26, 2020

rung commented Mar 6, 2020

poiana commented Mar 12, 2020

fntlnz commented Mar 12, 2020

leodido left a comment

poiana commented Mar 12, 2020

leodido commented Mar 12, 2020

rung commented Mar 12, 2020

rule(Change thread namespace): Modify condition to detect suspicious container activity #974

rule(Change thread namespace): Modify condition to detect suspicious container activity #974

Conversation

rung commented Dec 16, 2019

rung commented Dec 16, 2019

Procedure Step

Sample Output

Kaizhe Dec 16, 2019

Choose a reason for hiding this comment

Kaizhe commented Dec 16, 2019

poiana commented Dec 16, 2019

rung commented Dec 18, 2019

leodido left a comment

Choose a reason for hiding this comment

rung commented Dec 27, 2019

Kaizhe commented Dec 27, 2019

fntlnz commented Feb 25, 2020

rung commented Feb 25, 2020 • edited Loading

rung Feb 25, 2020

Choose a reason for hiding this comment

Kaizhe Feb 26, 2020

Choose a reason for hiding this comment

rung Feb 26, 2020

Choose a reason for hiding this comment

rung commented Feb 26, 2020 • edited Loading

Report on deep dive (why regression tests failed).

Reason 1

Reason 2

rung commented Feb 26, 2020

rung commented Mar 6, 2020

poiana commented Mar 12, 2020

fntlnz commented Mar 12, 2020

leodido left a comment

Choose a reason for hiding this comment

poiana commented Mar 12, 2020

leodido commented Mar 12, 2020

rung commented Mar 12, 2020

rung commented Feb 25, 2020 •

edited

Loading

rung commented Feb 26, 2020 •

edited

Loading