update(userspace/libsinsp): rely on proc root for user and group container lookup

[deepskyblue86]

What type of PR is this?

/kind feature

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

Does this PR require a change in the driver versions?

What this PR does / why we need it:
Instead of checking /proc//ns/mnt, just check /proc//root inode. The mount namespace gets created before the process actually gets into that namespace, so it was flaky and we also had a workaround getting it to work only for containers being created with non-root user.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

update(userspace/libsinsp): rely on proc root for user and group container lookup

[deepskyblue86]

@FedeDP cf35391 and 54e19b5 should address the issue you've been encountering from time to time on test_non_sudo_setuid 🙂

[FedeDP]

I saw that, thank you very much!
/cc @gnosek care to take a look?

[FedeDP]

Nice work and simplification!
/approve

[poiana]

LGTM label has been added.

Git tree hash: 5aeb70c7ebe6c16b76c36c56a8d3fd8a496966df

[FedeDP]

/milestone 0.11.0

[Andreagit97]

/approve

[gnosek]

LGTM, but I have a few questions:

1. Why do we need the new -E option in that test? I assume something we did was unreliable but isn't this PR supposed to address that?

2. How is /proc/pid/root more reliable than /proc/pid/ns/mnt? The only scenarios I can imagine where the two are inconsistent are:

• unshare(CLONE_NEWNS) changes ns/mnt but not root
• we're catching the thread after the switch to the new ns but before pivot_root; it feels like the window is tiny here but 🤷

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, deepskyblue86, FedeDP

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,FedeDP]

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

[Andreagit97]

/hold

[Andreagit97]

hold this until all doubts are solved

[deepskyblue86]

LGTM, but I have a few questions:

1. Why do we need the new -E option in that test? I assume something we did was unreliable but isn't this PR supposed to address that?

That test was already failing from time to time (e.g. https://github.com/falcosecurity/libs/actions/runs/3563324665/jobs/5985981759).
I checked it a few weeks ago, but this time I didn't spend much time on it, I just thought the user/group name resolution wasn't relevant for that test and I decided to remove the noise.

2. How is /proc/pid/root more reliable than /proc/pid/ns/mnt? The only scenarios I can imagine where the two are inconsistent are:

• unshare(CLONE_NEWNS) changes ns/mnt but not root
• we're catching the thread after the switch to the new ns but before pivot_root; it feels like the window is tiny here but 🤷

From my tests /proc/pid/ns/mnt exists before the process actually changes root, that's why I originally added the workaround on the uid/gid. I suppose the handle gets created before actually changing the root. I gave it a try with /proc/pid/root, thinking that if that one changes, maybe the process finally was in its own root.
I didn't know pivot_root, and I based my changes on where sinsp_threadinfo::set_user was already called from. Actually I also tried to put it only where PPME_SYSCALL_UNSHARE_X is handled but that didn't work. I also did several other attempts in these months 😅

Specifically about this:

• we're catching the thread after the switch to the new ns but before pivot_root; it feels like the window is tiny here but 🤷

$ grep -e setuid -e pivot_root -e unshare -e execve docker-run-root.out

{"container.id":"c650ae1b4fd7","evt.args":"res=0 exe=runc args=init. tid=5394(6) pid=5394(6) ptid=5389(runc) cwd= fdlimit=1048576 pgft_maj=0 pgft_min=348 vm_size=9816 vm_rss=4 vm_swap=0 comm=6 cgroups=cpuset=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4.cpu=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4.cpuacct=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4.io=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4.memory=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4. env=GOMAXPROCS=4._LIBCONTAINER_INITPIPE=3._LIBCONTAINER_STATEDIR=/var/run/docker/runtime-runc/moby/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4._LIBCONTAINER_LOGPIPE=4._LIBCONTAINER_LOGLEVEL=4._LIBCONTAINER_FIFOFD=5._LIBCONTAINER_INITTYPE=standard._LIBCONTAINER_CLONED_BINARY=1. tty=0 pgid=5376(containerd-shim) loginuid=-1 flags=1(EXE_WRITABLE) cap_inheritable=0 cap_permitted=3FFFFFFFFF cap_effective=3FFFFFFFFF ","evt.category":"process","evt.type":"execve","group.gid":0,"group.name":"root","proc.cmdline":"6 init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"flags=248(CLONE_NEWIPC|CLONE_NEWNET|CLONE_NEWNS|CLONE_NEWPID|CLONE_NEWUTS) ","evt.category":"process","evt.type":"unshare","group.gid":0,"group.name":"root","proc.cmdline":"runc:[0:PARENT] init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"res=0 ","evt.category":"process","evt.type":"unshare","group.gid":0,"group.name":"root","proc.cmdline":"runc:[0:PARENT] init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"uid=0(<NA>) ","evt.category":"user","evt.type":"setuid","group.gid":0,"group.name":"root","proc.cmdline":"runc:[1:CHILD] init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"res=0 ","evt.category":"user","evt.type":"setuid","group.gid":0,"group.name":"root","proc.cmdline":"runc:[1:CHILD] init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"","evt.category":"unknown","evt.type":"pivot_root","group.gid":0,"group.name":"root","proc.cmdline":"runc:[1:CHILD] init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"","evt.category":"unknown","evt.type":"pivot_root","group.gid":0,"group.name":"root","proc.cmdline":"runc:[1:CHILD] init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"uid=0(<NA>) ","evt.category":"user","evt.type":"setuid","group.gid":0,"group.name":"root","proc.cmdline":"runc:[1:CHILD] init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"res=0 ","evt.category":"user","evt.type":"setuid","group.gid":0,"group.name":"root","proc.cmdline":"runc:[1:CHILD] init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"filename=/bin/sh ","evt.category":"process","evt.type":"execve","group.gid":0,"group.name":"root","proc.cmdline":"runc:[1:CHILD] init","user.name":"root","user.uid":0}
{"container.id":"c650ae1b4fd7","evt.args":"res=0 exe=sh args=-c.exit. tid=5396(sh) pid=5396(sh) ptid=5376(containerd-shim) cwd= fdlimit=1048576 pgft_maj=0 pgft_min=483 vm_size=1520 vm_rss=4 vm_swap=0 comm=sh cgroups=cpuset=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4.cpu=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4.cpuacct=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4.io=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4.memory=/docker/c650ae1b4fd7cc661633a8a9535c74cfbab331313f5ff161a92b6c19081195d4. env=PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin.HOSTNAME=c650ae1b4fd7.NGINX_VERSION=1.14.2.HOME=/root. tty=0 pgid=1(systemd) loginuid=-1 flags=1(EXE_WRITABLE) cap_inheritable=0 cap_permitted=A80425FB cap_effective=A80425FB ","evt.category":"process","evt.type":"execve","group.gid":0,"group.name":"root","proc.cmdline":"sh -c exit","user.name":"root","user.uid":0}

I think we are.

[gnosek]

That test was already failing from time to time (e.g. https://github.com/falcosecurity/libs/actions/runs/3563324665/jobs/5985981759). I checked it a few weeks ago, but this time I didn't spend much time on it, I just thought the user/group name resolution wasn't relevant for that test and I decided to remove the noise.

Hmm okay. I wonder if this PR makes it stable though.

• we're catching the thread after the switch to the new ns but before pivot_root; it feels like the window is tiny here but 🤷

$ grep -e setuid -e pivot_root -e unshare -e execve docker-run-root.out

(...)

I think we are.

Perfect. So we have an explanation and a patch. SHIP IT! ;)

[Andreagit97]

/unhold