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

new(driver, libscap, libsinsp): Add support for detecting executions from binaries referenced by a memfd #1066

Merged
merged 5 commits into from
Jun 22, 2023

Conversation

lrishi
Copy link
Contributor

@lrishi lrishi commented Apr 27, 2023

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap-engine-udig

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:
This PR deals with executions from memfds, ie execution from anonymous files created using the syscall memfd_create. memfd_create creates an anonymous file and returns a file descriptor that refers to it. The file behaves like a regular file and can be modified, truncated, memory-mapped, executed, etc. However, unlike a regular file, it lives in RAM and has a volatile backing storage. Once all references to the file are dropped, it is automatically released. Anonymous memory is used for all backing pages of the file.
Specifically, This PR tries to identify when a process being executed is from a memfd and set a corresponding flag in the execve event. Such executions can evade otherwise concrete security rules owing to lack of a standard unix-like name and typical executions from procfs as /proc/[pid]/fd/[fd-num]. By adding this capability, the offending parent can be flagged as an executor of a fileless binary.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

To better understand memfds, refer to this man page.
This PR runs two checks:

  1. It checks if a dentry's parent is itself, which is the case for memfds
  2. It checks first 6 chars of the filename are memfd:

Does this PR introduce a user-facing change?:

new(driver, libscap, libsinsp): Add support for detecting executions from binaries referenced by a memfd

@poiana
Copy link
Contributor

poiana commented Apr 27, 2023

Welcome @lrishi! It looks like this is your first PR to falcosecurity/libs 🎉

@github-actions
Copy link

Please double check driver/API_VERSION file. See versioning.

Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

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

Hi 👋 @lrishi !

This is amazing ❤️ thanks for helping with closing this gap.

Sharing some initial thoughts and we will work together to get this over the finish line.
Proposing lib 0.13.0 release as we already have so many changes for 0.11.0 and 0.12.0 will be a short one leading up to Falco 0.35 intended to only include less intrusive changes. Would this be ok with you? We can prioritize this right at the beginning of June.

Recommending to sync with @loresuso, see one of our HackMDs where we discussed a few gaps, "memfd+exec" was one of them https://hackmd.io/@leogr/SJKUMEbWo. @loresuso also did some more research on symlink path resolution for the executable path (Lorenzo will publish a new issue shortly), therefore it seems June is gonna be the month where stars will align to close many gaps around process executions 🎉 . And I am also still interested in solving the "interpreter script" issues plus some other related items ...

Primary concern here is performance, let's collectively check if we could squeeze out some optimizations kernel side, also have to take a closer look and happy to help with testing! @Andreagit97 and @FedeDP will also help with the driver changes. userspace changes mostly look good already, left one comment around scap file.

Action Items:

  • new tests are missing
  • address all CI failures and polish details in the drivers w/ performance in mind
  • test with multiple clang versions and kernels (we are developing even more automated test suites, but some manual testing by various community members will be required as well)

driver/bpf/fillers.h Outdated Show resolved Hide resolved
driver/bpf/fillers.h Show resolved Hide resolved
userspace/libscap/scap_savefile.c Show resolved Hide resolved
userspace/libsinsp/filterchecks.cpp Outdated Show resolved Hide resolved
@FedeDP
Copy link
Contributor

FedeDP commented May 3, 2023

/milestone next-driver

@poiana poiana added this to the next-driver milestone May 3, 2023
@leogr leogr added this to the next-driver milestone May 3, 2023
Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

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

@lrishi did some hands-on testing:

  • Re more perf optimization, was checking if there could be even a cheaper initial filter to know if the file is an anonymous file or on disk, but damn it doesn't seem to be anything there and the check you already have parent != dentry appears to be the best we can do, also excellent work here! Anyone having more optimization ideas @falcosecurity/libs-maintainers?
  • ✔️ Code changes correctly detect what they are intending to detect.
  • Only minor additional suggestions wrt reordering lookups in the bpf drivers and adding some more comments throughout
  • Re tests proposing to not block this PR as our test frameworks are not yet ready to meaningfully support such more sophisticated tests we would need. This feedback also applies to kernel version testing, we are in the process of formalizing a "kernel version testing framework" and as we will merge this at the beginning of 0.12.0 we will have time to apply the extended new streamlined framework post-merge. CC @falcosecurity/libs-maintainers

Testing I have performed (may be insightful for anyone reading this):

http://0x90909090.blogspot.com/2019/02/executing-payload-without-touching.html
https://www.exploit-db.com/exploits/46043
https://magisterquis.github.io/2018/03/31/in-memory-only-elf-execution.html

My patience for a quick little tests was 10 min 😎 hence here is my little unsophisticated py script adopted from internet resources:

#! /usr/bin/python3

import ctypes
import ctypes.util
import time
import subprocess
import os


libc = ctypes.cdll.LoadLibrary(ctypes.util.find_library('c'))
fd = libc.syscall(319,b'libstest', 1)
assert fd >= 0
pid = os.getpid()
self_fd_dir = '/proc/{}/fd/'.format(pid)
memfd_exe_fd = '/proc/{}/fd/{}'.format(pid, fd)

with open('/usr/bin/ls', mode='rb') as f1:
    with open(memfd_exe_fd, mode='wb') as f2:
        f2.write(f1.read())

# https://docs.python.org/3/library/os.html#os.execve -> didn't work for me with an fd
print(memfd_exe_fd)
cmd = "bash -c 'exec ls -l {}'".format(self_fd_dir)
subprocess.run([cmd], shell=True)
cmd = "bash -c 'exec {} -l {}'".format(memfd_exe_fd, self_fd_dir)
subprocess.run([cmd], shell=True)
[XXX@fedora]$ python memfd.py
/proc/204537/fd/3
total 0
lrwx------ 1 m m 64 May 29 16:37 0 -> /dev/pts/6
lrwx------ 1 m m 64 May 29 16:37 1 -> /dev/pts/6
lrwx------ 1 m m 64 May 29 16:37 2 -> /dev/pts/6
lrwx------ 1 m m 64 May 29 16:37 3 -> '/memfd:libstest (deleted)'
total 0
lrwx------ 1 m m 64 May 29 16:37 0 -> /dev/pts/6
lrwx------ 1 m m 64 May 29 16:37 1 -> /dev/pts/6
lrwx------ 1 m m 64 May 29 16:37 2 -> /dev/pts/6
lrwx------ 1 m m 64 May 29 16:37 3 -> '/memfd:libstest (deleted)'

record in /sys/kernel/debug/tracing/trace_pipe when running bpf driver in debug mode and adding a new print statement:

<...>-203445  [007] d..3. 259697.535199: bpf_trace_printk: memfd:libstest

driver/bpf/fillers.h Show resolved Hide resolved
driver/bpf/fillers.h Outdated Show resolved Hide resolved
userspace/libscap/scap_savefile.c Show resolved Hide resolved
userspace/libsinsp/filterchecks.cpp Outdated Show resolved Hide resolved
userspace/libsinsp/filterchecks.cpp Outdated Show resolved Hide resolved
lrishi and others added 4 commits June 22, 2023 11:30
Signed-off-by: Lovel Rishi <lovelworks@gmail.com>

Address review comments #1:

1. Remove extra lookup for exe file
2. Directive to unroll loops
3. Fix compilation issues in modern bpf probe

Signed-off-by: Lovel Rishi <lovelworks@gmail.com>
1. Move `parent != dentry` checks before reading the name
2. Reword text in filterchecks
3. Add comments to function definitions

Signed-off-by: Lovel Rishi <lovelworks@gmail.com>
1. Correct typo proces -> process
2. Add missing initialization of tinfo.exe_from_memfd = false in scap_savefile.c

Signed-off-by: Lovel Rishi <lovelworks@gmail.com>
…sion

Signed-off-by: Lovel Rishi <lovelworks@gmail.com>
Andreagit97
Andreagit97 previously approved these changes Jun 22, 2023
Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Jun 22, 2023

LGTM label has been added.

Git tree hash: 817136d020597e1930de7b2764c4aad137fbc635

…emfd flag logic in modern bpf execveat

Signed-off-by: Lovel Rishi <lovelworks@gmail.com>
Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

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

/approve

@poiana poiana added the lgtm label Jun 22, 2023
@poiana
Copy link
Contributor

poiana commented Jun 22, 2023

LGTM label has been added.

Git tree hash: 1fd7f3169dffbb8bdce605062e16bc3b44a4226b

Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Jun 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, incertum, lrishi

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 [Andreagit97,incertum]

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

@Andreagit97
Copy link
Member

/unhold

@poiana poiana merged commit 8f7e963 into falcosecurity:master Jun 22, 2023
24 checks passed
@FedeDP
Copy link
Contributor

FedeDP commented Jul 28, 2023

/milestone 5.1.0+driver

@poiana poiana modified the milestones: next-driver, 5.1.0+driver Jul 28, 2023
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.

7 participants