Initial userspace instrumentation implementation #1195

krisnova · 2020-05-06T20:17:14Z

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

ALL.

What this PR does / why we need it:

Support in the Falco CLI for the new ptrace implementation being merged in the driver code.

Which issue(s) this PR fixes:

NONE

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new: Falco now supports userspace instrumentation with the -u flag

krisnova · 2020-05-06T20:17:35Z

/hold

We will need to update the cmake reference once draios/sysdig#1626 is merged.

krisnova · 2020-07-01T21:53:47Z

/unhold
/continue
/please-merge

I don't know how to un-hold something - maybe one of these will work

leodido · 2020-07-01T22:14:07Z

/hold cancel

On Wed, 1 Jul 2020 at 23:54, Kris Nova ***@***.***> wrote: /unhold /continue /please-merge I don't know how to un-hold something - maybe one of these will work — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#1195 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA5J43OQUR3ZFOWLIPMI3TRZOV7RANCNFSM4M2X5P7Q> .

-- L.

leodido

The first round of review.

Left some comments for further discussion and thinking next week. Nothing definitive, yet :)

Also, the PR corpus needs to be updated and refer to the actual PR introducing userspace capture - ie., draios/sysdig#1636

My main question for now is: do we want to refer to this as udig? Or do we want to refer to it in a more eloquent way (to avoid any confusion in the users) and let this word only for specific implementations (udig, pdig, etc.)?

userspace/falco/falco.cpp

CMakeLists.txt

udig support through the -u command line flag Signed-off-by: Kris Nóva <kris@nivenly.com> Co-authored-by: Kris Nóva <kris@nivenly.com>

Signed-off-by: Kris Nova <kris@nivenly.com>

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com> Co-authored-by: Leonardo Di Donato <leodidonato@gmail.com>

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>

fntlnz · 2020-07-15T09:39:47Z

Just tested this with @leodido on my machine:

# ./userspace/falco/falco -u -r ../rules/falco_rules.yaml
Wed Jul 15 10:31:05 2020: Falco initialized with configuration file /home/fntlnz/Projects/falcosecurity/falco/falco.yaml
Wed Jul 15 10:31:05 2020: Loading rules from file ../rules/falco_rules.yaml:
Wed Jul 15 10:31:05 2020: Starting internal webserver, listening on port 8765

I also needed to use a root shell, otherwise:

$ ./userspace/falco/falco -u -r ../rules/falco_rules.yaml 
Wed Jul 15 10:32:32 2020: Falco initialized with configuration file /home/fntlnz/Projects/falcosecurity/falco/falco.yaml
Wed Jul 15 10:32:32 2020: Loading rules from file ../rules/falco_rules.yaml:
[1]    454395 segmentation fault (core dumped)  ./userspace/falco/falco -u -r ../rules/falco_rules.yaml

Root is needed because userspace instrumentation scans the /proc filesystem with scap_proc_scan_proc_dir

This is what happens now if you run Falco without root privileges, see how the error now is very clear.

$ ./userspace/falco/falco -u -r ../rules/falco_rules.yaml 
Wed Jul 15 11:28:44 2020: Falco initialized with configuration file /home/fntlnz/Projects/falcosecurity/falco/falco.yaml
Wed Jul 15 11:28:44 2020: Loading rules from file ../rules/falco_rules.yaml:
Wed Jul 15 11:28:44 2020: Runtime error: error creating the process list. Make sure you have root credentials.. Exiting.

This fix was already done by @leodido almost one year ago on the module check PR (https://github.com/falcosecurity/falco/pull/758/files) - We really need to get that one into!

fntlnz · 2020-07-15T09:54:52Z

I've been trying to use the pdig userspace implementation but I had no luck in compiling it (will investigate later):

compilation output

FAILED: CMakeFiles/pdig.dir/pdig_ptrace_amd64.cc.o 
/usr/bin/g++  -DUDIG -I../ -I/home/fntlnz/Projects/draios/sysdig/driver -I/home/fntlnz/Projects/draios/sysdig/userspace/libscap -O3 -DNDEBUG -Wall   -std=gnu++11 -MD -MT CMakeFiles/pdig.dir/pdig_ptrace_amd64.cc.o -MF CMakeFiles/pdig.dir/pdig_ptrace_amd64.cc.o.d -o CMakeFiles/pdig.dir/pdig_ptrace_amd64.cc.o -c ../pdig_ptrace_amd64.cc
../pdig_ptrace_amd64.cc: In function ‘int inject_getXXXXname(pid_t, int, sockaddr*, socklen_t*, long unsigned int)’:
../pdig_ptrace_amd64.cc:179:17: error: ‘process_vm_readv’ was not declared in this scope; did you mean ‘SYS_process_vm_readv’?
  179 |  size_t nread = process_vm_readv(tid, local_iov, 2, remote_iov, 1, 0);
      |                 ^~~~~~~~~~~~~~~~
      |                 SYS_process_vm_readv
[10/16] Building C object CMakeFiles/pdig.dir/pdig_compat.c.o
../pdig_compat.c: In function ‘record_event’:
../pdig_compat.c:331:15: warning: pointer targets in assignment from ‘uint8_t *’ {aka ‘unsigned char *’} to ‘char *’ differ in signedness [-Wpointer-sign]
  331 |   args.buffer = g_ring + head + sizeof(struct ppm_evt_hdr);
      |               ^
../pdig_compat.c:255:11: warning: variable ‘usedspace’ set but not used [-Wunused-but-set-variable]
  255 |  uint32_t usedspace;
      |           ^~~~~~~~~
[11/16] Building C object CMakeFiles/pdig.dir/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c.o
/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c: In function ‘fd_to_socktuple’:
/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c:1002:8: warning: unused variable ‘dest’ [-Wunused-variable]
 1002 |  char *dest;
      |        ^~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c:1000:22: warning: unused variable ‘usrsockaddr_un’ [-Wunused-variable]
 1000 |  struct sockaddr_un *usrsockaddr_un;
      |                      ^~~~~~~~~~~~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c:991:15: warning: unused variable ‘speer’ [-Wunused-variable]
  991 |  struct sock *speer;
      |               ^~~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c:990:8: warning: unused variable ‘us_name’ [-Wunused-variable]
  990 |  char *us_name;
      |        ^~~~~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c:989:20: warning: unused variable ‘us’ [-Wunused-variable]
  989 |  struct unix_sock *us;
      |                    ^~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c:986:17: warning: unused variable ‘sock’ [-Wunused-variable]
  986 |  struct socket *sock;
      |                 ^~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c: In function ‘parse_readv_writev_bufs’:
/home/fntlnz/Projects/draios/sysdig/driver/ppm_events.c:1321:15: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 1321 |    args->fd = (int)val;
      |               ^~~~~~~~
[12/16] Building C object CMakeFiles/pdig.dir/udig_procs.c.o
../udig_procs.c: In function ‘udig_proc_startupdate’:
../udig_procs.c:496:40: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 252 [-Wformat-truncation=]
  496 |  snprintf(filename, sizeof(filename), "%sexe", dir_name);
      |                                        ^~      ~~~~~~~~
../udig_procs.c:496:2: note: ‘snprintf’ output between 4 and 259 bytes into a destination of size 252
  496 |  snprintf(filename, sizeof(filename), "%sexe", dir_name);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../udig_procs.c:529:40: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 252 [-Wformat-truncation=]
  529 |  snprintf(filename, sizeof(filename), "%sstatus", dir_name);
      |                                        ^~         ~~~~~~~~
../udig_procs.c:529:2: note: ‘snprintf’ output between 7 and 262 bytes into a destination of size 252
  529 |  snprintf(filename, sizeof(filename), "%sstatus", dir_name);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../udig_procs.c:556:40: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 252 [-Wformat-truncation=]
  556 |  snprintf(filename, sizeof(filename), "%scmdline", dir_name);
      |                                        ^~          ~~~~~~~~
../udig_procs.c:556:2: note: ‘snprintf’ output between 8 and 263 bytes into a destination of size 252
  556 |  snprintf(filename, sizeof(filename), "%scmdline", dir_name);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../udig_procs.c:579:46: warning: ‘%s’ directive output may be truncated writing up to 4095 bytes into a region of size 1024 [-Wformat-truncation=]
  579 |    snprintf(tinfo->exe, SCAP_MAX_PATH_SIZE, "%s", line);
      |                                              ^~   ~~~~
../udig_procs.c:579:4: note: ‘snprintf’ output between 1 and 4096 bytes into a destination of size 1024
  579 |    snprintf(tinfo->exe, SCAP_MAX_PATH_SIZE, "%s", line);
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../udig_procs.c:603:40: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 252 [-Wformat-truncation=]
  603 |  snprintf(filename, sizeof(filename), "%senviron", dir_name);
      |                                        ^~          ~~~~~~~~
../udig_procs.c:603:2: note: ‘snprintf’ output between 8 and 263 bytes into a destination of size 252
  603 |  snprintf(filename, sizeof(filename), "%senviron", dir_name);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[13/16] Building CXX object CMakeFiles/pdig.dir/pdig.cc.o
FAILED: CMakeFiles/pdig.dir/pdig.cc.o 
/usr/bin/g++  -DUDIG -I../ -I/home/fntlnz/Projects/draios/sysdig/driver -I/home/fntlnz/Projects/draios/sysdig/userspace/libscap -O3 -DNDEBUG -Wall   -std=gnu++11 -MD -MT CMakeFiles/pdig.dir/pdig.cc.o -MF CMakeFiles/pdig.dir/pdig.cc.o.d -o CMakeFiles/pdig.dir/pdig.cc.o -c ../pdig.cc
../pdig.cc: In function ‘bool handle_ptrace_clone_event(pid_t, pdig_process_context&, pdig_context&)’:
../pdig.cc:154:25: error: ‘CLONE_THREAD’ was not declared in this scope
  154 |  if (pctx.clone_flags & CLONE_THREAD) {
      |                         ^~~~~~~~~~~~
[14/16] Building C object CMakeFiles/pdig.dir/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c.o
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c: In function ‘f_sys_socketpair_x’:
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:1360:15: warning: unused variable ‘speer’ [-Wunused-variable]
 1360 |  struct sock *speer;
      |               ^~~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:1359:20: warning: unused variable ‘us’ [-Wunused-variable]
 1359 |  struct unix_sock *us;
      |                    ^~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:1358:17: warning: unused variable ‘sock’ [-Wunused-variable]
 1358 |  struct socket *sock;
      |                 ^~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:1357:6: warning: unused variable ‘err’ [-Wunused-variable]
 1357 |  int err;
      |      ^~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:1356:6: warning: unused variable ‘fds’ [-Wunused-variable]
 1356 |  int fds[2];
      |      ^~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:1355:16: warning: unused variable ‘val’ [-Wunused-variable]
 1355 |  unsigned long val;
      |                ^~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c: In function ‘f_sys_accept_x’:
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:1738:17: warning: unused variable ‘sock’ [-Wunused-variable]
 1738 |  struct socket *sock;
      |                 ^~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:1737:6: warning: unused variable ‘err’ [-Wunused-variable]
 1737 |  int err = 0;
      |      ^~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c: In function ‘f_sys_pipe_x’:
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:2546:15: warning: unused variable ‘file’ [-Wunused-variable]
 2546 |  struct file *file;
      |               ^~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c: In function ‘f_sys_connect_x’:
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:1294:5: warning: ‘fd’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 1294 |  if (fd >= 0) {
      |     ^
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c: In function ‘f_sys_recvfrom_x’:
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:2139:12: warning: ‘fd’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 2139 |     size = fd_to_socktuple(fd,
      |            ^~~~~~~~~~~~~~~~~~~
 2140 |      (struct sockaddr *)&address,
      |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2141 |      addrlen,
      |      ~~~~~~~~
 2142 |      true,
      |      ~~~~~  
 2143 |      true,
      |      ~~~~~  
 2144 |      targetbuf,
      |      ~~~~~~~~~~
 2145 |      STR_STORAGE_SIZE);
      |      ~~~~~~~~~~~~~~~~~
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c: In function ‘f_sys_recvmsg_x’:
/home/fntlnz/Projects/draios/sysdig/driver/ppm_fillers.c:2476:12: warning: ‘fd’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 2476 |     size = fd_to_socktuple(fd,
      |            ^~~~~~~~~~~~~~~~~~~
 2477 |      (struct sockaddr *)&address,
      |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2478 |      addrlen,
      |      ~~~~~~~~
 2479 |      true,
      |      ~~~~~  
 2480 |      true,
      |      ~~~~~  
 2481 |      targetbuf,
      |      ~~~~~~~~~~
 2482 |      STR_STORAGE_SIZE);
      |      ~~~~~~~~~~~~~~~~~
[15/16] Building CXX object CMakeFiles/pdig.dir/proc_tree.cc.o
ninja: build stopped: subcommand failed.


</details>

fntlnz · 2020-07-15T09:56:23Z

Talked with @leodido and we have a couple concerns in merging this for 0.24.0

We have no release process in place for pdig (users will not be able to find a pdig artifact around, that makes this flag useless)
Lack of documentation, again, user experience is important here and we have many new moving pieces to explain. e.g: the relation between falco --userspace and pdig.
Release is tomorrow

We propose to postpone this to 0.25.0

Let's vote (add a reaction):

👍 to move this to 0.25.0 and release next month after addressing the concerns above
👎 to keep this to 0.24.0 and release tomorrow as is

leodido · 2020-07-15T10:01:46Z

Reached majority

/milestone 0.25.0

fntlnz · 2020-07-15T15:35:06Z

Community call is happening right now and we all agreed on a variation of the idea I expressed in my previous comment.

In particular, we agreed on the fact that the contract that Falco exposes to talk to userspace implementations needs to be dcoupled from the userspace implementation itself.

With this reasoning in mind, we decided to get this change in now, for 0.24.0 while letting the implementation to follow their own lifecycle.

This means that:

The -u flag is an officially supported feature as of Falco 0.24.0 , along with its backend implementation and the contract. This means that users will be able to write their own implementations, use a stable one from a provider or try the - not yet supported - pdig
The implementation of userspace instrumentation that implements the contract on the other side, and is owned by this community https://github.com/falcosecurity/pdig is not yet ready and will have more iterations to reach the Official support status.

fntlnz · 2020-07-15T15:35:15Z

/milestone 0.24.0

leodido

🎉

poiana · 2020-07-15T15:56:28Z

LGTM label has been added.

Git tree hash: 30319e4da638be9e1247a1023e2656fe983a6cb9

poiana · 2020-07-15T15:58:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, 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

poiana added release-note dco-signoff: no kind/feature area/engine labels May 6, 2020

poiana requested review from leodido and mstemm May 6, 2020 20:17

poiana added the size/M label May 6, 2020

poiana added the do-not-merge/hold label May 6, 2020

krisnova force-pushed the udig branch from c11ba83 to fb8a2de Compare May 6, 2020 20:21

poiana added dco-signoff: yes and removed dco-signoff: no labels May 6, 2020

leogr added this to the 0.24.0 milestone May 18, 2020

krisnova force-pushed the udig branch from e81e2f9 to c2f5465 Compare July 1, 2020 21:49

poiana removed the do-not-merge/hold label Jul 1, 2020

leodido removed the area/engine label Jul 3, 2020

leodido reviewed Jul 3, 2020

View reviewed changes

poiana added dco-signoff: no needs-rebase and removed dco-signoff: yes labels Jul 13, 2020

ldegio and others added 5 commits July 14, 2020 18:09

feat(cli): adding -u to flip inspector method calls

6f153d7

udig support through the -u command line flag Signed-off-by: Kris Nóva <kris@nivenly.com> Co-authored-by: Kris Nóva <kris@nivenly.com>

feat(build): fixing MD5 of tpp for udig/pdig build

0e15e8d

Signed-off-by: Kris Nova <kris@nivenly.com>

feat(cli): adding -u to the usage text

f75b5d4

Signed-off-by: Kris Nova <kris@nivenly.com>

update(userspace/falco): edits to the falco CLI

7b38f78

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com> Co-authored-by: Leonardo Di Donato <leodidonato@gmail.com>

update(userspace/falco): userspace instrumentation help line

358ca11

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>

leodido force-pushed the udig branch from 4976c77 to 358ca11 Compare July 14, 2020 18:12

poiana added dco-signoff: yes and removed dco-signoff: no labels Jul 14, 2020

fntlnz mentioned this pull request Jul 15, 2020

Area userspace in Falco repo falcosecurity/test-infra#155

Closed

poiana modified the milestones: 0.24.0, 0.25.0 Jul 15, 2020

leodido mentioned this pull request Jul 15, 2020

CI and release process for pdig falcosecurity/pdig#16

Closed

5 tasks

poiana modified the milestones: 0.25.0, 0.24.0 Jul 15, 2020

leodido approved these changes Jul 15, 2020

View reviewed changes

poiana assigned leodido Jul 15, 2020

poiana added the lgtm label Jul 15, 2020

poiana added the approved label Jul 15, 2020

fntlnz approved these changes Jul 15, 2020

View reviewed changes

poiana assigned fntlnz Jul 15, 2020

leodido removed the request for review from mstemm July 15, 2020 16:06

leodido closed this Jul 15, 2020

leodido reopened this Jul 15, 2020

leodido added lgtm approved and removed lgtm approved labels Jul 15, 2020

poiana merged commit a447b69 into master Jul 15, 2020

poiana deleted the udig branch July 15, 2020 17:03

fntlnz mentioned this pull request Jul 15, 2020

Userspace instrumentation docs falcosecurity/falco-website#230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial userspace instrumentation implementation #1195

Initial userspace instrumentation implementation #1195

krisnova commented May 6, 2020 •

edited by fntlnz

Loading

krisnova commented May 6, 2020

krisnova commented Jul 1, 2020

leodido commented Jul 1, 2020 via email

leodido left a comment •

edited

Loading

fntlnz commented Jul 15, 2020 •

edited

Loading

fntlnz commented Jul 15, 2020 •

edited

Loading

fntlnz commented Jul 15, 2020

leodido commented Jul 15, 2020 •

edited

Loading

fntlnz commented Jul 15, 2020

fntlnz commented Jul 15, 2020

leodido left a comment

poiana commented Jul 15, 2020

poiana commented Jul 15, 2020

Initial userspace instrumentation implementation #1195

Initial userspace instrumentation implementation #1195

Conversation

krisnova commented May 6, 2020 • edited by fntlnz Loading

krisnova commented May 6, 2020

krisnova commented Jul 1, 2020

leodido commented Jul 1, 2020 via email

leodido left a comment • edited Loading

Choose a reason for hiding this comment

fntlnz commented Jul 15, 2020 • edited Loading

fntlnz commented Jul 15, 2020 • edited Loading

fntlnz commented Jul 15, 2020

leodido commented Jul 15, 2020 • edited Loading

fntlnz commented Jul 15, 2020

fntlnz commented Jul 15, 2020

leodido left a comment

Choose a reason for hiding this comment

poiana commented Jul 15, 2020

poiana commented Jul 15, 2020

krisnova commented May 6, 2020 •

edited by fntlnz

Loading

leodido left a comment •

edited

Loading

fntlnz commented Jul 15, 2020 •

edited

Loading

fntlnz commented Jul 15, 2020 •

edited

Loading

leodido commented Jul 15, 2020 •

edited

Loading