Date: Sat, 16 Aug 2008 13:18:24 +0200 From: Ed Schouten <ed@80386.nl> To: FreeBSD Arch <freebsd-arch@freebsd.org> Cc: Jille Timmermans <jille@quis.cx> Subject: [Reviews requested] kern/121073: chroot for non-root users Message-ID: <20080816111824.GL99951@hoeg.nl>
next in thread | raw e-mail | index | archive | help
--0DcgAuOrObJpcAxl Content-Type: multipart/mixed; boundary="3MtLaQwm2ZGHj8IC" Content-Disposition: inline --3MtLaQwm2ZGHj8IC Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hello everyone, When I visited FOSDEM back in February, I was talking with Jille Timmermans about the chroot() call. After discussing that the problem with chroot() is that it cannot be safely be executed by non-root users w.r.t. setuid binaries*, we wrote this patchset for the kernel to add something similar to `MNT_NOSUID' to the process flags. The result being: http://bugs.FreeBSD.org/121073 The patch even adds a small security improvement to the system. Say, you'd change the typical chroot() + setuid() order the other way around, you're guaranteed the chrooted process will never change users afterwards, because it won't honour set[ug]id binaries anymore. Our security officer was wise enough to add the following to the PR: +----------------------------------------------------------+ |UNDER NO CONDITIONS SHOULD THIS PATCH BE COMMITTED WITHOUT| |EXPLICIT APPROVAL FROM THE FREEBSD SECURITY OFFICER. | +----------------------------------------------------------+ After having a discussion with Colin on IRC, there are a couple of questions we would like to be answered (or discussed) before getting this in the tree: - Are there any comments on the patch itself? - Colin was concerned if turned on, would it be possible for the user to do things which it normally couldn't and shouldn't? It would be great to get many reviews on this before we'd land it in the source tree. I've attached the patch to this email as well. Thanks! --=20 Ed Schouten <ed@80386.nl> WWW: http://80386.nl/ * Hardlink a setuid binary to a directory containing a fake C library and executing it. --3MtLaQwm2ZGHj8IC Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="chroot.diff" Content-Transfer-Encoding: quoted-printable --- lib/libc/sys/chroot.2 +++ lib/libc/sys/chroot.2 @@ -61,7 +61,13 @@ .Fn chroot has no effect on the process's current directory. .Pp -This call is restricted to the super-user. +By default, this call is restricted to the super-user. If +.Ql kern.chroot_allow_unprivileged +is set to a non-zero value, all users are capable of performing the +.Fn chroot +call. When called by an unprivileged user, the process and its children +won't honor the setuid and setgid bits when performing an +.Xr execve 2 . .Pp Depending on the setting of the .Ql kern.chroot_allow_open_directories @@ -140,3 +146,8 @@ open directories, or a MAC check), it is possible that this system call may return an error, with the working directory of the process left changed. +.Pp +When a call to +.Fn chroot +fails when invoked by an unprivileged user, the process is not properly +capable of executing setuid or setgid applications anymore. --- sys/kern/kern_exec.c +++ sys/kern/kern_exec.c @@ -633,7 +633,7 @@ =20 if (credential_changing && (imgp->vp->v_mount->mnt_flag & MNT_NOSUID) =3D=3D 0 && - (p->p_flag & P_TRACED) =3D=3D 0) { + (p->p_flag & (P_NOSUGID|P_TRACED)) =3D=3D 0) { /* * Turn off syscall tracing for set-id programs, except for * root. Record any set-id flags first to make sure that --- sys/kern/kern_fork.c +++ sys/kern/kern_fork.c @@ -595,7 +595,7 @@ * Preserve some more flags in subprocess. P_PROFIL has already * been preserved. */ - p2->p_flag |=3D p1->p_flag & P_SUGID; + p2->p_flag |=3D p1->p_flag & P_INHERITED; td2->td_pflags |=3D td->td_pflags & TDP_ALTSTACK; SESS_LOCK(p1->p_session); if (p1->p_session->s_ttyvp !=3D NULL && p1->p_flag & P_CONTROLT) --- sys/kern/vfs_syscalls.c +++ sys/kern/vfs_syscalls.c @@ -860,9 +860,12 @@ */ =20 static int chroot_allow_open_directories =3D 1; +static int chroot_allow_unprivileged =3D 0; =20 SYSCTL_INT(_kern, OID_AUTO, chroot_allow_open_directories, CTLFLAG_RW, &chroot_allow_open_directories, 0, ""); +SYSCTL_INT(_kern, OID_AUTO, chroot_allow_unprivileged, CTLFLAG_RW, + &chroot_allow_unprivileged, 0, ""); =20 /* * Change notion of root (``/'') directory. @@ -880,12 +883,27 @@ } */ *uap; { int error; + struct proc *p; struct nameidata nd; int vfslocked; =20 error =3D priv_check(td, PRIV_VFS_CHROOT); - if (error) - return (error); + if (error) { + if (!chroot_allow_unprivileged) + return (error); + + /* + * Disallow this process and its children to use setuid + * bits. Users could hardlink setuid applications into a + * chroot which contains a fake C library to obtain + * super-user privileges. + */ + p =3D td->td_proc; + PROC_LOCK(p); + p->p_flag |=3D P_NOSUGID; + PROC_UNLOCK(p); + } + NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF | MPSAFE | AUDITVNODE1, UIO_USERSPACE, uap->path, td); error =3D namei(&nd); --- sys/sys/proc.h +++ sys/sys/proc.h @@ -575,7 +575,9 @@ #define P_INMEM 0x10000000 /* Loaded into memory. */ #define P_SWAPPINGOUT 0x20000000 /* Process is being swapped out. */ #define P_SWAPPINGIN 0x40000000 /* Process is being swapped in. */ +#define P_NOSUGID 0x80000000 /* Ignore set[ug]id on exec. */ =20 +#define P_INHERITED (P_SUGID|P_NOSUGID) #define P_STOPPED (P_STOPPED_SIG|P_STOPPED_SINGLE|P_STOPPED_TRACE) #define P_SHOULDSTOP(p) ((p)->p_flag & P_STOPPED) =20 --3MtLaQwm2ZGHj8IC-- --0DcgAuOrObJpcAxl Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkimt4AACgkQ52SDGA2eCwV9uQCfWMRPwbvuyvLsCphhfFMKUxEL 9UgAnixkKdX13A6U77mFgHL47RIJ/0GY =z37V -----END PGP SIGNATURE----- --0DcgAuOrObJpcAxl--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080816111824.GL99951>