From owner-freebsd-audit Mon Apr 9 21:26: 6 2001 Delivered-To: freebsd-audit@freebsd.org Received: from bazooka.unixfreak.org (bazooka.unixfreak.org [63.198.170.138]) by hub.freebsd.org (Postfix) with ESMTP id A548B37B62F for ; Mon, 9 Apr 2001 21:25:55 -0700 (PDT) (envelope-from dima@unixfreak.org) Received: from spike.unixfreak.org (spike [63.198.170.139]) by bazooka.unixfreak.org (Postfix) with ESMTP id 52D9D3E09 for ; Mon, 9 Apr 2001 21:25:55 -0700 (PDT) To: audit@freebsd.org Subject: Fixes to src/sbin/dump/traverse.c Date: Mon, 09 Apr 2001 21:25:55 -0700 From: Dima Dorfman Message-Id: <20010410042555.52D9D3E09@bazooka.unixfreak.org> Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Attached is a patch that makes some [relatively minor] fixes to src/sbin/dump/traverse.c. Most of these bugs were caused by incorrect assumptions I made when porting over NetBSD's code to handle the nodump filesystem flag on directories (revs. 1.12 and 1.10.2.2). The first two hunks' purpose should be pretty obvious from the comment update; basically, mapdirs() tests directory inodes for being in dumpdirmap but not in usedinomap to detect whether that directory has inherited, or has set, the nodump flag. Currently, this test will fail when it shouldn't when doing a dump with level > 0 and when no directories on the filesystem were changed. The third hunk corrects a small bogon with regards to handling the -h option. If someone could look this over and, if acceptable, commit it, I'd appreciate. The patch is pretty small, so it would be desireable to see it in 4.3-RELEASE (although I'm not going to push very hard for that this late). Thanks in advance, Dima Dorfman dima@unixfreak.org Index: traverse.c =================================================================== RCS file: /st/src/FreeBSD/src/sbin/dump/traverse.c,v retrieving revision 1.12 diff -u -r1.12 traverse.c --- traverse.c 2001/03/03 11:35:50 1.12 +++ traverse.c 2001/04/07 22:50:35 @@ -155,13 +155,15 @@ if ((mode = (dp->di_mode & IFMT)) == 0) continue; /* - * All dirs go in dumpdirmap; only inodes that are to - * be dumped go in usedinomap and dumpinomap, however. + * Everything must go in usedinomap so that a check + * for "in dumpdirmap but not in usedinomap" to detect + * dirs with nodump set has a chance of succeeding + * (this is used in mapdirs()). */ + SETINO(ino, usedinomap); if (mode == IFDIR) SETINO(ino, dumpdirmap); if (WANTTODUMP(dp)) { - SETINO(ino, usedinomap); SETINO(ino, dumpinomap); if (mode != IFREG && mode != IFDIR && mode != IFLNK) *tapesize += 1; @@ -169,8 +171,11 @@ *tapesize += blockest(dp); continue; } - if (mode == IFDIR) + if (mode == IFDIR) { + if (!nonodump && (dp->di_flags & UF_NODUMP)) + CLRINO(ino, usedinomap); anydirskipped = 1; + } } /* * Restore gets very upset if the root is not dumped, @@ -218,7 +223,7 @@ * it isn't in usedinomap, we have to go through it to * propagate the nodump flag. */ - nodump = (TSTINO(ino, usedinomap) == 0); + nodump = !nonodump && (TSTINO(ino, usedinomap) == 0); if ((isdir & 1) == 0 || (TSTINO(ino, dumpinomap) && !nodump)) continue; dp = getino(ino); To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Wed Apr 11 21:50:41 2001 Delivered-To: freebsd-audit@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 1428437B506; Wed, 11 Apr 2001 21:50:25 -0700 (PDT) (envelope-from robert@fledge.watson.org) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.11.3/8.11.3) with SMTP id f3C4oxf99411; Thu, 12 Apr 2001 00:50:59 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Thu, 12 Apr 2001 00:50:59 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: freebsd-audit@FreeBSD.org Cc: tmm@FreeBSD.org Subject: relationship between credential-managing daemons and normal processes Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Earlier today, I committed the beginnings of an inter-process authorization regression testing suite. My goal was to make sure that, as we replace the current authorization code, no vulnerabilities be introduced as a result of accidental changes, as well as to take a look at the correctness of the current implementation. The regression testing suite actually explores a slightly larger state space than the kernel provides to user processes, by virtue of its testing of a number of cases where the P_SUGID bit is turned off but isn't in normal operation. The following excerpt comes from executing the suite on a FreeBSD 5.0-CURRENT box this evening: test capabilities: [SETSUGID_SUPPORTED_BUT_NO_LIBC_STUB] [CHECK_CRED_SET] [20. unpriv1 on daemon1].signal: expected EPERM, got 0 (e:1000 r:1000 s:1000 P_SUGID:0)(e:1000 r:0 s:0 P_SUGID:0) [20. unpriv1 on daemon1].sched: expected EPERM, got 0 (e:1000 r:1000 s:1000 P_SUGID:0)(e:1000 r:0 s:0 P_SUGID:0) [21. unpriv1 on daemon1].signal: expected EPERM, got 0 (e:1000 r:1000 s:1000 P_SUGID:0)(e:1000 r:0 s:0 P_SUGID:1) [21. unpriv1 on daemon1].sched: expected EPERM, got 0 (e:1000 r:1000 s:1000 P_SUGID:0)(e:1000 r:0 s:0 P_SUGID:1) [22. unpriv1 on daemon1].signal: expected EPERM, got 0 (e:1000 r:1000 s:1000 P_SUGID:1)(e:1000 r:0 s:0 P_SUGID:0) [22. unpriv1 on daemon1].sched: expected EPERM, got 0 (e:1000 r:1000 s:1000 P_SUGID:1)(e:1000 r:0 s:0 P_SUGID:0) [23. unpriv1 on daemon1].signal: expected EPERM, got 0 (e:1000 r:1000 s:1000 P_SUGID:1)(e:1000 r:0 s:0 P_SUGID:1) [23. unpriv1 on daemon1].sched: expected EPERM, got 0 (e:1000 r:1000 s:1000 P_SUGID:1)(e:1000 r:0 s:0 P_SUGID:1) Each test case considers two processes with specially configured credentials, with the first process performing an operation on the second process. A table of "correct" values is included in the regression test, and the above output is generated where my expectations are not met by the implementation (not necessarily a sign that it's wrong, possibly a sign that my expectations are improper). In any case, I figured it was worth raising these cases for discussion. You can read a test by looking at the two lines associated with test output: the first identifies the test performed, as well as the difference in opinion of return value. The second prints out the contents of the subject and object credentials, where subject means the process requesting the operation, and object means the process on the receiving side of the operation. In each case above, the test scenario is one similar to that of a privileged daemon which temporarily takes on the effective identity of an unprivileged process, probably for the purposes of delivering a message, managing a spool file, etc. The test involves a second process, running with the real and effective credentials of the unprivilged user, attempting to act on the daemon process, which runs with privilege but has temporarily changed its effective identity to the unprivileged user's. In the above failure cases, the unprivileged process successfully performs a SIGHUP delivery to the daemon process, or invokes setpriority() on the process. Due to the nature of the testing engine, each test is performed a number of times with various permutations of the P_SUGID bit being set on the subject and object. For the purposes of this discussion, we're probably interested only in tests where P_SUGID is set on the object. Generally speaking, I would argue that it would be inappropriate for the unprivileged process to be able to perform these operations on, say, an lpd switched to its uid, to a web server in the same situation, sendmail during delivery, or ftpd. As such, I'd be tempted to introduce new restrictions in p_cansignal() and p_cansched() to require that the real uid be the same in the event that the effective uid's differ (with the exception of SIGCONT, which is always permitted if the sessions of the processes are the same, or if the subject credential has appropriate privilege). However, there may be situations where this is inappropriate, possibly involving SIGIO delivery or the like. Does anyone else have any reservations about making such a change? I have not implemented it yet, but it would probably be fairly straight-forward to do. On the other hand, I'd rather not break things too much in the process (a little is acceptable however). Hopefully in the near future, I (or someone else with time on their hands) will be expanding the regression tests to test other signals, add ktrace() and procfs debugging attach. Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message From owner-freebsd-audit Thu Apr 12 20:34:31 2001 Delivered-To: freebsd-audit@freebsd.org Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by hub.freebsd.org (Postfix) with ESMTP id 7B94937B42C; Thu, 12 Apr 2001 20:34:28 -0700 (PDT) (envelope-from robert@fledge.watson.org) Received: from fledge.watson.org (robert@fledge.pr.watson.org [192.0.2.3]) by fledge.watson.org (8.11.3/8.11.3) with SMTP id f3D3Z2f14610; Thu, 12 Apr 2001 23:35:02 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Thu, 12 Apr 2001 23:35:01 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: freebsd-audit@FreeBSD.org Cc: tmm@FreeBSD.org Subject: Re: relationship between credential-managing daemons and normal processes In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG After some discussion with Thomas, running testings, etc, I've modified p_cansched() and p_cansignal() to take into account more restricted policies to remove these exceptions to the expected regression test results in 5.0-CURRENT. The commit messages document the details, but essentially the goal was to remove cases where the effective uid of the target process was used in the decision-making process, preventing processes from manipulating the scheduling of the daemon processes, or delivering signals to them improperly. Here's the new uid-based logic for p_cansched(): if (p1->p_cred->p_ruid == p2->p_cred->p_ruid) return (0); if (p1->p_ucred->cr_uid == p2->p_cred->p_ruid) return (0); Here's the new uid-based logic for p_cansignal(): if (p1->p_cred->p_ruid != p2->p_cred->p_ruid && p1->p_cred->p_ruid != p2->p_cred->p_svuid && p1->p_ucred->cr_uid != p2->p_cred->p_ruid && p1->p_ucred->cr_uid != p2->p_cred->p_svuid) { There are a few possible further changes that could be made to the p_cansched() logic in particular -- it might be appropriate to add the svuid checking there such that it matches the p_cansignal() logic, which catches setuid binaries relying on POSIX saved uid behavior. As I mentioned in my commit messages, there is some risk associated with changing this behavior: it's possible that applications exist which rely on the old behavior, for example. However, I did take this opportunity to inspect the Linux protection logic for signalling, and discovered (after a bit of un-hand-micro-optimizing) that the logic is the same. The same goes for our new p_cansched() and their similar access control checks. This suggests that there is a fairly large pool of users and applications for whom these behaviors work fine, which is a little reassuring. In any case, these are changes I'm willing to back out if they cause any problems, but I figured getting them in early would offer us the best opportunity to test them in a wider audience. Robert N M Watson FreeBSD Core Team, TrustedBSD Project robert@fledge.watson.org NAI Labs, Safeport Network Services To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message