On Mon, 23 Feb 2004, Hannes Gredler wrote:
> tx pekka - can sombeody pls test on the BSDs ? - /hannes
Works on my FreeBSD at least.
However, I noticed a different problem with dropping the privileges.
The critical questions are:
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". (Currently this this
is "yes; if the username was implicit, and then root privs are dropped
later". Thoughts?
Note that with setuid tcpdump, this has never been possible (due to
valid reasons, of course :). But root-dropping tcpdump, especially if
done automatically, might be a bit special.
I've attached a patch this results in the assumed intended behaviour:
the privileges are dropped only later, the behaviour is identical with
or without --with-user=xxx, and more detailed hackery of write files
is omitted. I've moved up the setuid-part though.
Please discuss what you feel would be the best approach! I might
personally be tempted to move up the opening of write files part..
--
Pekka Savola "You each name yourselves king, yet the
Netcore Oy kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
diff -ur tcpdump-2004.02.23/tcpdump.c tcpdump-2004.02.23.new/tcpdump.c
--- tcpdump-2004.02.23/tcpdump.c Sat Jan 31 08:14:59 2004
+++ tcpdump-2004.02.23.new/tcpdump.c Mon Feb 23 19:11:01 2004
@@ -704,6 +704,15 @@
if (tflag > 0)
thiszone = gmt2local(0);
+#ifdef WITH_USER
+ /* if run as root, prepare for dropping root privileges */
+ if (getuid() == 0 || geteuid() == 0) {
+ /* Run with '-Z root' to restore old behaviour */
+ if (!username)
+ username = WITH_USER;
+ }
+#endif
+
if (RFileName != NULL) {
int dlt;
const char *dlt_name;
@@ -718,13 +727,8 @@
* people's trace files (especially if we're set-UID
* root).
*/
- if (username) {
- droproot(username);
- }
- else {
- if (setgid(getgid()) != 0 || setuid(getuid()) != 0 )
- fprintf(stderr, "Warning: setgid/setuid failed !\n");
- }
+ if (setgid(getgid()) != 0 || setuid(getuid()) != 0 )
+ fprintf(stderr, "Warning: setgid/setuid failed !\n");
#endif /* WIN32 */
pd = pcap_open_offline(RFileName, ebuf);
if (pd == NULL)
@@ -771,6 +775,13 @@
error("%s", ebuf);
else if (*ebuf)
warning("%s", ebuf);
+ /*
+ * Let user own process after socket has been opened.
+ */
+#ifndef WIN32
+ if (setgid(getgid()) != 0 || setuid(getuid()) != 0)
+ fprintf(stderr, "Warning: setgid/setuid failed !\n");
+#endif /* WIN32 */
#ifdef WIN32
if(UserBufferSize != 1000000)
if(pcap_setbuff(pd, UserBufferSize)==-1){
@@ -808,18 +819,6 @@
netmask = 0;
warning("%s", ebuf);
}
- /*
- * Let user own process after socket has been opened.
- */
-#ifndef WIN32
- if (username) {
- droproot(username);
- }
- else {
- if (setgid(getgid()) != 0 || setuid(getuid()) != 0)
- fprintf(stderr, "Warning: setgid/setuid failed !\n");
- }
-#endif /* WIN32 */
}
if (infile)
cmdbuf = read_infile(infile);
@@ -881,6 +880,15 @@
callback = print_packet;
pcap_userdata = (u_char *)&printinfo;
}
+#ifndef WIN32
+ /*
+ * We cannot do this earlier, because we want to be able to open
+ * the file (if done) for writing before giving up permissions.
+ */
+ if (username) {
+ droproot(username);
+ }
+#endif /* WIN32 */
#ifdef SIGINFO
(void)setsignal(SIGINFO, requestinfo);
#endif
@@ -948,16 +956,6 @@
(void)fprintf(stderr, "%s: pcap_loop: %s\n",
program_name, pcap_geterr(pd));
}
-#ifdef WITH_USER
- /* if run as root, drop root; protect against remote sec problems */
- if (getuid() == 0 || geteuid() == 0) {
- /* Run with '-Z root' to restore old behaviour */
- if (!username) {
- droproot(WITH_USER);
- /* does not return if fails */
- }
- }
-#endif
if (RFileName == NULL) {
/*
* We're doing a live capture. Report the capture
