Fix security hole checking unpacked kernel headers

[jordalgo]

Make sure to check that the unpacked kheaders tar
is owned by root to prevent bpftrace from loading
compromised linux headers.

Borrowed from @brendangregg here: iovisor/bcc#4928

Tested:

$ sudo chown 1000 /tmp/kheaders-6.7.4-200.fc39.aarch64
$ sudo ./src/bpftrace test-script.bt 
ERROR: header file ownership expected to be root: /tmp/kheaders-6.7.4-200.fc39.aarch64

Checklist

• Language changes are updated in man/adoc/bpftrace.adoc
• User-visible and non-trivial changes updated in CHANGELOG.md
• The new behaviour is covered by tests

[danobi]

Possible to add a unit test for file_exists_and_ownedby_root()? Can easily create a non-root owned file. And could probably use /proc/1/.. for a root owned file?

[jordalgo]

Possible to add a unit test for file_exists_and_ownedby_root()? Can easily create a non-root owned file. And could probably use /proc/1/.. for a root owned file?

@danobi Good call. Added.

[danobi]

nit

Suggested change

	if (::mkdtemp(&tmpdir[0]) == nullptr) {
	if (::mkdtemp(tmpdir.data()) == nullptr) {

[danobi]

ASSERT_GE(fd, 0) ?

[brendangregg]

Thanks!

[jeromemarchand]

I'm afraid this is not fixed yet. If a user build a valid kheaders tree in /tmp/kheaders-$(uname -r) bpftrace will show an error message, but but proceed to use use the possibly compromised headers none the less:

# /usr/share/bpftrace/tools/execsnoop.bt 
ERROR: header file ownership expected to be root: /tmp/kheaders-5.14.0-431.rh29064.el9.x86_64
Attaching 3 probes...
TIME            PID     PPID    ARGS
18:15:38.492036 3116    1934    ls --color=auto -l --color=auto

If the user has an empty keaders-$(uname -r) directory it seems that bpftrace will overwrite it, but not if it isn't empty.
I think the problem is we don't check whether moving tpmdir to shared_path succeeds at the end of unpack_kheaders_tar_xz():

  int rc = ::pclose(tar);
  if (rc == 0) {
    tmpdir.move_to(shared_path);
    return shared_path;
  }

Or maybe we should exit as soon as wrong ownership is detected.

[jordalgo]

@jeromemarchand Good catch! I think maybe there is confusion around rename as it seems to be implementation specific if the new file name exists. On my machine, it's doing what you observed in that it doesn't replace the compromised headers path and just re-uses. I'll add some deletion and extra checking (alternatively we can do what you said and just exit if the permissions are not root).

[jordalgo]

Second attempt to fix here: #3154