When kern.nosuidcoredump=3, act like =2 but try to dump cores into

the /var/crash/programname/ directory, as root. For instance,
    # mkdir /var/crash/bgpd/
    # chmod 700 /var/crash/bgpd/    # If you skip this step, you are a moron
    # sysctl kern.nosuidcoredump=3
    # bgpd
    # pkill -ABRT bgpd
    # ls /var/crash/bgpd/
    14764.core   23207.core   6423.core
Of course, in real life the idea is that you don't kill the daemon but it
crashes and you collect parallel cores.  Careful you don't fill your /var.
Further tuneables are being considered.

Sorry to be picking on bgpd for this example.  I've watched the "too
difficult to debug privsep code" angst for far too long.
ok guenther

4 files changed, 67 insertions, 25 deletions

diff --git a/lib/libc/gen/sysctl.3 b/lib/libc/gen/sysctl.3
index 70762ff8d75..c1e5f714036 100644
--- a/lib/libc/gen/sysctl.3
+++ b/lib/libc/gen/sysctl.3
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: sysctl.3,v 1.232 2014/04/27 16:58:08 jmc Exp $
+.\"	$OpenBSD: sysctl.3,v 1.233 2014/05/04 03:53:37 deraadt Exp $
 .\"
 .\" Copyright (c) 1993
 .\"	The Regents of the University of California.  All rights reserved.
@@ -27,7 +27,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: April 27 2014 $
+.Dd $Mdocdate: May 4 2014 $
 .Dt SYSCTL 3
 .Os
 .Sh NAME
@@ -667,6 +667,7 @@ Whether a process may dump core after changing user or group ID:
 .It 0 Ta "euid == 0" Ta "current directory"
 .It 1 Ta "never" Ta ""
 .It 2 Ta "always" Ta Pa "/var/crash"
+.It 2 Ta "depends" Ta Pa "/var/crash/$programname/"
 .El
 .It Dv KERN_NPROCS
 The number of entries in the kernel process table.
diff --git a/sbin/sysctl/sysctl.8 b/sbin/sysctl/sysctl.8
index e0b02563bc7..dac573f121d 100644
--- a/sbin/sysctl/sysctl.8
+++ b/sbin/sysctl/sysctl.8
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: sysctl.8,v 1.177 2014/04/27 16:56:51 jmc Exp $
+.\"	$OpenBSD: sysctl.8,v 1.178 2014/05/04 03:53:37 deraadt Exp $
 .\"	$NetBSD: sysctl.8,v 1.4 1995/09/30 07:12:49 thorpej Exp $
 .\"
 .\" Copyright (c) 1993
@@ -30,7 +30,7 @@
 .\"
 .\"	@(#)sysctl.8	8.2 (Berkeley) 5/9/95
 .\"
-.Dd $Mdocdate: April 27 2014 $
+.Dd $Mdocdate: May 4 2014 $
 .Dt SYSCTL 8
 .Os
 .Sh NAME
@@ -521,6 +521,18 @@ the maximum number of shared memory segments:
 # sysctl kern.shminfo.shmmax=33554432
 # sysctl kern.shminfo.shmseg=32
 .Ed
+.Pp
+To place core dumps from
+.Xr issetugid 2
+programs (in this example
+.Xr bgpd 8 )
+into a safe place for debugging purposes
+.Bd -literal -offset indent
+# mkdir /var/crash/bgpd
+# chmod 700 /var/crash/bgpd
+# sysctl kern.nosuidcoredump=3
+.Ed
+.Pp
 .Sh SEE ALSO
 .Xr sysctl 3 ,
 .Xr options 4 ,
diff --git a/share/man/man5/core.5 b/share/man/man5/core.5
index 21f5d162c33..c89c8ae88ba 100644
--- a/share/man/man5/core.5
+++ b/share/man/man5/core.5
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: core.5,v 1.16 2014/01/21 03:15:46 schwarze Exp $
+.\"	$OpenBSD: core.5,v 1.17 2014/05/04 03:53:38 deraadt Exp $
 .\"	$NetBSD: core.5,v 1.4 1994/11/30 19:31:11 jtc Exp $
 .\"
 .\" Copyright (c) 1980, 1991, 1993
@@ -30,7 +30,7 @@
 .\"
 .\"     @(#)core.5	8.3 (Berkeley) 12/11/93
 .\"
-.Dd $Mdocdate: January 21 2014 $
+.Dd $Mdocdate: May 4 2014 $
 .Dt CORE 5
 .Os
 .Sh NAME
@@ -164,10 +164,13 @@ A
 file format appeared in
 .At v3 .
 .Sh CAVEATS
-Programs with their set-user-ID bit set will not dump core,
-to prevent sensitive information from inadvertently ending up on disk.
-See
+Programs which are started with (either) the set-user-ID or
+set-group-ID bits set,
+or which change their uid or gid after starting, will normally not
+dump core.
+This is to prevent sensitive information from inadvertently ending
+up on disk.
+This behaviour can be changed (for debugging purposes) by changing
 .Li kern.nosuidcoredump
-in
 .Xr sysctl 3
-for more information.
+variable to the right settings.
diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c
index 5356365dc7b..dd42647a2e6 100644
--- a/sys/kern/kern_sig.c
+++ b/sys/kern/kern_sig.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_sig.c,v 1.164 2014/04/18 11:51:17 guenther Exp $	*/
+/*	$OpenBSD: kern_sig.c,v 1.165 2014/05/04 03:53:37 deraadt Exp $	*/
 /*	$NetBSD: kern_sig.c,v 1.54 1996/04/22 01:38:32 christos Exp $	*/
 
 /*
@@ -51,6 +51,7 @@
 #include <sys/buf.h>
 #include <sys/acct.h>
 #include <sys/file.h>
+#include <sys/filedesc.h>
 #include <sys/kernel.h>
 #include <sys/wait.h>
 #include <sys/ktrace.h>
@@ -1446,21 +1447,20 @@ coredump(struct proc *p)
 	struct nameidata nd;
 	struct vattr vattr;
 	struct coredump_iostate	io;
-	int error, len;
-	char name[sizeof("/var/crash/") + MAXCOMLEN + sizeof(".core")];
-	char *dir = "";
+	int error, len, incrash = 0;
+	char name[MAXPATHLEN];
+	const char *dir = "/var/crash";
 
 	pr->ps_flags |= PS_COREDUMP;
 
 	/*
-	 * Don't dump if not root and the process has used set user or
-	 * group privileges, unless the nosuidcoredump sysctl is set to 2,
-	 * in which case dumps are put into /var/crash/.
+	 * If the process has inconsistant uids, nosuidcoredump
+	 * determines coredump placement policy.
 	 */
 	if (((pr->ps_flags & PS_SUGID) && (error = suser(p, 0))) ||
 	   ((pr->ps_flags & PS_SUGID) && nosuidcoredump)) {
-		if (nosuidcoredump == 2)
-			dir = "/var/crash/";
+		if (nosuidcoredump == 3 || nosuidcoredump == 2)
+			incrash = 1;
 		else
 			return (EPERM);
 	}
@@ -1470,16 +1470,42 @@ coredump(struct proc *p)
 	    p->p_rlimit[RLIMIT_CORE].rlim_cur)
 		return (EFBIG);
 
-	len = snprintf(name, sizeof(name), "%s%s.core", dir, p->p_comm);
+	if (nosuidcoredump == 3) {
+		/*
+		 * If the program directory does not exist, dumps of
+		 * that core will silently fail.
+		 */
+		len = snprintf(name, sizeof(name), "%s/%s/%u.core",
+		    dir, p->p_comm, p->p_pid);
+	} else if (nosuidcoredump == 2)
+		len = snprintf(name, sizeof(name), "%s/%s.core",
+		    dir, p->p_comm);
+	else
+		len = snprintf(name, sizeof(name), "%s.core", p->p_comm);
 	if (len >= sizeof(name))
 		return (EACCES);
 
 	/*
-	 * ... but actually write it as UID
+	 * Control the UID used to write out.  The normal case uses
+	 * the real UID.  If the sugid case is going to write into the
+	 * controlled directory, we do so as root.
 	 */
-	cred = crdup(cred);
-	cred->cr_uid = cred->cr_ruid;
-	cred->cr_gid = cred->cr_rgid;
+	if (incrash == 0) {
+		cred = crdup(cred);
+		cred->cr_uid = cred->cr_ruid;
+		cred->cr_gid = cred->cr_rgid;
+	} else {
+		if (p->p_fd->fd_rdir) {
+			vrele(p->p_fd->fd_rdir);
+			p->p_fd->fd_rdir = NULL;
+		}
+		p->p_ucred = crdup(p->p_ucred);
+		crfree(cred);
+		cred = p->p_ucred;
+		crhold(cred);
+		cred->cr_uid = 0;
+		cred->cr_gid = 0;
+	}
 
 	NDINIT(&nd, LOOKUP, NOFOLLOW, UIO_SYSSPACE, name, p);