Re: OpenBSD work on Tcpdump privilege separation

[Andrew Pimlott <andrew () pimlott net>]

Obviously I did not read the thread carefully before posting my last
message.  Here are some comments on Pekka's latest patch.

On Mon, Feb 23, 2004 at 07:10:25PM +0200, Pekka Savola wrote:
> 1) does one have to be able to record files (with '-w') also to
> directories you yourself (root) have write access to, but the user to 
> which you drop the privileges does not?
>
> 2) is there any difference whether dropping the privileges was
> implicit (with '--with-user') or explicit ('-Z')?
>
> 3) would we want to hack tcpdump a bit further, so that the write file 
> would be opened as early as possible, to be able to drop the 
> privileges earlier (if yes to 1)? [this might also help with 
> chrooting, if we wanted to do it.]
>
> I assume the answers are "yes", "no" and "no".

I agree with all of these decisions.  Realistically, the only benefit
we're trying to gain from dropping privileges is to mitigate damages if
someone takes over tcpdump via the packet analysis code.  So there is
not much reward for drop privileges a little earlier.

This is complicated slightly by the setuid case, where need to drop the
euid before opening any files.  Pekka simply calls setuid(getuid())
(which permanently changes back to the running user) to achieve this.
This means that we can not run droproot() in the setuid case.
(Actually, Pekka's code tries to run droproot after having called
setuid, fails, and exits.  For example, I compiled tcpdump
--with-user=nobody, chmoded it setuid root, and ran it as myself:

    % ./tcpdump.setuid 
    Couldn't change to 'nobody' uid=65534 gid=65534

)

In my patch, I tried to handle this by only dropping euid before opening
files, so that we can later restore euid and do a full droproot(), which
is (arguably) better than just dropping back to ruid.  Since anyone with
setuid tcpdump probably doesn't care for security, it may not be worth
catering to this.  (Note to anyone looking at that patch: I didn't get
it exactly right, but the idea is there.)

Finally, here is a patch that doesn't address any of the above (pending
discussion), but only modifies droproot along the lines of my previous
patch.  Please see the comments there[1] regarding the issues with the
chroot directory.  It also occurred to me that honoring TMPDIR may not
be a good idea if tcpdump is setuid; however, since I'm not sure whether
this code will be run in that case, I've left it in for now.

Andrew

[1] http://www.tcpdump.org/lists/workers/2004/01/msg00064.html

diff -u -r1.231 tcpdump.c
--- tcpdump.c   24 Feb 2004 08:12:18 -0000      1.231
+++ tcpdump.c   24 Feb 2004 16:59:36 -0000
@@ -324,25 +324,45 @@
 #define U_FLAG
 #endif
 
-/* Drop root privileges */
+/* 
+ * Drop privileges.  Assumes we currently have euid 0.
+ * Parts cribbed from Wietse Venema's chrootuid.
+ */
 static void
 droproot(const char *username)
 {
        struct passwd *pw = NULL;
+       char *tmpdir;
+       char chrootdir[1024];
 
-       pw = getpwnam(username);
-       if (pw) {
-               if (initgroups(pw->pw_name, 0) != 0 || setgid(pw->pw_gid) != 0 ||
-                                setuid(pw->pw_uid) != 0) {
-                       fprintf(stderr, "Couldn't change to '%.32s' uid=%d gid=%d\n", username, 
-                                                       pw->pw_uid, pw->pw_gid);
-                       exit(1);
-               }
-       }
-       else {
-               fprintf(stderr, "Couldn't find user '%.32s'\n", username);
-               exit(1);
-       }
+       if (! (pw = getpwnam(username)))
+               error("Couldn't find user '%.32s'", username);
+       if (setgid(pw->pw_gid) != 0)
+               error("setgid to %d failed", pw->pw_gid);
+       if (initgroups(pw->pw_name, pw->pw_gid) != 0)
+               error("initgroups for '%.32s' failed");
+       endpwent();  /* in case passwd file is still open */
+       endgrent();  /* in case group file is still open */
+
+       /* Try to chroot to an empty dir. */
+
+       tmpdir = getenv("TMPDIR");
+       if (tmpdir == NULL)
+               tmpdir = "/tmp";
+
+       snprintf(chrootdir, sizeof(chrootdir), "%s/tcpdump.XXXXXX", tmpdir);
+
+       if (mkdtemp(chrootdir) == NULL)
+               error("couldn't create %s", chrootdir);
+       if (chdir(chrootdir) != 0)
+               error("couldn't chdir to %s", chrootdir);
+       if (rmdir(chrootdir) != 0)
+               error("couldn't remove %s", chrootdir);
+       if (chroot(".") != 0)
+               error("couldn't chroot");
+
+       if (setuid(pw->pw_uid) != 0)
+               error("setuid to %d failed", pw->pw_uid);
 }
 
 static int

-
This is the TCPDUMP workers list. It is archived at
http://www.tcpdump.org/lists/workers/index.html
To unsubscribe use mailto:tcpdump-workers-request () tcpdump org?body=unsubscribe