iovisor / bcc

BCC - Tools for BPF-based Linux IO analysis, networking, monitoring, and more
Apache License 2.0
20.57k stars 3.88k forks source link

check effective userid on run #850

Closed rickysarraf closed 7 years ago

rickysarraf commented 7 years ago

Hi,

Thank you for the BPF tools. They really are a great set, for a myriad of use cases. I've taken up the task of packaging it for Debian. It currently is under the NEW queue, and should hopefully clear in time to become part of the Debian Stretch release.

Meanwhile, here's an exception that keeps throwing up, when run unprivileged.

rrs@learner:~/rrs-home/Community/Packaging/bpfcc (master)$ execsnoop 
In file included from /virtual/main.c:4:
include/linux/fs.h:2680:9: warning: comparison of unsigned enum expression < 0 is always false
      [-Wtautological-compare]
        if (id < 0 || id >= READING_MAX_ID)
            ~~ ^ ~
1 warning generated.
bpf: Operation not permitted

Traceback (most recent call last):
  File "/usr/sbin/execsnoop", line 148, in <module>
    b = BPF(text=bpf_text)
  File "/usr/lib/python2.7/dist-packages/bcc/__init__.py", line 203, in __init__
    self._trace_autoload()
  File "/usr/lib/python2.7/dist-packages/bcc/__init__.py", line 679, in _trace_autoload
    fn = self.load_func(func_name, BPF.KPROBE)
  File "/usr/lib/python2.7/dist-packages/bcc/__init__.py", line 243, in load_func
    raise Exception("Failed to load BPF program %s" % func_name)
Exception: Failed to load BPF program kprobe__sys_execve
2016-12-12 / 14:26:44 ♒♒♒  ☹  => 1  

Can we make it cleaner? Something like the following. But it'd have to be added to all the command files.

if os.geteuid() != 0:
    sys.stderr.write("Need superuser privileges for %s\n" % (sys.argv[0]))
    sys.exit(1)

I thought of adding this check into the bcc module. But then, it'd have to be added to multiple functions. So instead I thought it'd make more sense to just update the command files, for once.

goldshtn commented 7 years ago

I think it belongs in the BPF class. Various functions like attach_kprobe could just call this code as a helper method.

rickysarraf commented 7 years ago

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

On Mon, 2016-12-12 at 11:19 -0800, Sasha Goldshtein wrote:

I think it belongs in the BPF class. Various functions like attach_kprobe could just call this code as a helper method.

I just added it to the initialization section. This helps cover almost all the different callers.

Signed-off-by: Ritesh Raj Sarraf rrs@debian.org


 src/python/bcc/init.py | 3 +++  1 file changed, 3 insertions(+)

diff --git a/src/python/bcc/init.py b/src/python/bcc/init.py index 8dba8e5..67ce095 100644

iQIzBAEBCgAdFiEEQCVDstmIVAB/Yn02pjpYo/LhdWkFAlhPw+AACgkQpjpYo/Lh dWnQTA/6AuN5UZz4+yO0Wt8eLQTXc+8POM4a9p2/n5Al+KRCv5KPfa7Jux2ELiRF KTTgC4fKsZkovOkP9sBJeUIOEE2PfWrQBV8J221daQ/UsQuDOQ3XkSKBLwwo1cyw zxke0KozE4Fo4TCC+pnrwuQimxCZeMtrzi6XyrysU2ZY4Na/a9eWaif4X8Rpfhgk JhbvhtQOwcMhFk6y4/OxN90rXxzjaz19uJoyXS9CLOYWZp6DGXvIO3pzcLQ6yJc9 9qBEJ/DrUCqaGoIoxQejpzULhnOI0wvGZC9sP06ehPE6a12J8tAOECtMu6f3gFkZ ShgS/F5zpJQJYYIitlRjkPUBi6XMADeS6IEBnlWRpPEVRhtvaMs34Cm72OriedZb FNyGzQ55vhxZtoqFWJUpeuzhTxwzA+fdjO76js7Ci1QpG7j8Mlaa6jHDeDmalGja 9HWhlUcslJ27c8CkGpTJyiJxPqne7VBKLajt5B1qNhC+kMDtOmwh3/qOCHyB00km Zg4zd5fub9uXRchERIeact/dj7YjKSsik0yqmMnnKgUtykb4CTLyKj71AShf4d+b bCBla1hG3v8BleswErbeIX11MODK5GNy92UwUxP2PXpD9SLw3gklSvxbhC8G+84l LQ/WS5XJoaJnKovyNMkEBCKROwd6Ma4zYbGJTGvBF7Zl1cFkPkk= =LR34 -----END PGP SIGNATURE-----

brendangregg commented 7 years ago

Thanks @rickysarraf, I'll submit it as a PR (please feel free to submit some yourself too) -- I'm going to move the exception higher in the function, before the atexit.register(), otherwise we throw more exceptions on exit cleanup that don't make sense.

brendangregg commented 7 years ago

Ok, now I'm hesitating due to the "Non-root programs on sockets" stuff[1]. Thinking...

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=1be7f75d1668d6296b80bf35dcf6762393530afc

brendangregg commented 7 years ago

How about:

diff --git a/src/python/bcc/__init__.py b/src/python/bcc/__init__.py
index d13c03d..d95dc77 100644
--- a/src/python/bcc/__init__.py
+++ b/src/python/bcc/__init__.py
@@ -282,12 +282,13 @@ class BPF(object):
             print(log_buf.value.decode(), file=sys.stderr)

         if fd < 0:
+            atexit.register(self.donothing)
+            if ct.get_errno() == errno.EPERM:
+                raise Exception("Need super-user privilges to run")
+
             errstr = os.strerror(ct.get_errno())
-            if self.debug & DEBUG_BPF:
-                raise Exception("Failed to load BPF program %s: %s" %
-                                (func_name, errstr))
-            else:
-                raise Exception("Failed to load BPF program %s: %s" % (func_name, errstr))
+            raise Exception("Failed to load BPF program %s: %s" %
+                            (func_name, errstr))

         fn = BPF.Function(self, func_name, fd)
         self.funcs[func_name] = fn
@@ -1027,6 +1028,9 @@ class BPF(object):
         except KeyboardInterrupt:
             exit()

+    def donothing(self):
+        """the do nothing exit handler"""
+
     def cleanup(self):
         for k, v in list(self.open_kprobes.items()):
             lib.perf_reader_free(v)

(I also cleaned up a redundant DEBUG_BPF test).

Output is now:

$ ./execsnoop.py 
In file included from /virtual/main.c:4:
/lib/modules/4.9.0-rc5-virtual/build/include/linux/fs.h:2693:9: warning: comparison of unsigned enum expression < 0 is always false [-Wtautological-compare]
        if (id < 0 || id >= READING_MAX_ID)
            ~~ ^ ~
1 warning generated.
bpf: Operation not permitted
??JT
Traceback (most recent call last):
  File "./execsnoop.py", line 143, in <module>
    b = BPF(text=bpf_text)
  File "/usr/lib/python2.7/dist-packages/bcc/__init__.py", line 247, in __init__
    self._trace_autoload()
  File "/usr/lib/python2.7/dist-packages/bcc/__init__.py", line 846, in _trace_autoload
    fn = self.load_func(func_name, BPF.KPROBE)
  File "/usr/lib/python2.7/dist-packages/bcc/__init__.py", line 287, in load_func
    raise Exception("Need super-user privilges to run")
Exception: Need super-user privilges to run

The weird thing printed ("??JT" in this case) I think is the BPF buffer. That doesn't look right, but would be a different bug.

brendangregg commented 7 years ago

closed by #856