From 4ea7ed56bd1c71560696216439fcb657286d6443 Mon Sep 17 00:00:00 2001 From: deraadt Date: Tue, 12 Dec 2017 01:12:34 +0000 Subject: [PATCH] pledge()'s 2nd argument becomes char *execpromises, which becomes the pledge for a new execve image immediately upon start. Also introduces "error" which makes violations return -1 ENOSYS instead of killing the program ("error" may not be handed to a setuid/setgid program, which may be missing/ignoring syscall return values and would continue with inconsistant state) Discussion with many florian has used this to improve the strictness of a daemon --- include/unistd.h | 4 +- lib/libc/sys/execve.2 | 10 ++- lib/libc/sys/pledge.2 | 82 ++++++++++++------------ sys/kern/kern_exec.c | 17 ++++- sys/kern/kern_fork.c | 4 +- sys/kern/kern_pledge.c | 129 ++++++++++++++++++++++++-------------- sys/kern/syscalls.master | 5 +- sys/sys/pledge.h | 4 +- sys/sys/proc.h | 6 +- usr.bin/kdump/ktrstruct.c | 12 ++-- 10 files changed, 164 insertions(+), 109 deletions(-) diff --git a/include/unistd.h b/include/unistd.h index ffec1538f44..dab92899383 100644 --- a/include/unistd.h +++ b/include/unistd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: unistd.h,v 1.104 2017/03/09 10:13:03 fcambus Exp $ */ +/* $OpenBSD: unistd.h,v 1.105 2017/12/12 01:12:34 deraadt Exp $ */ /* $NetBSD: unistd.h,v 1.26.4.1 1996/05/28 02:31:51 mrg Exp $ */ /*- @@ -522,7 +522,7 @@ int strtofflags(char **, u_int32_t *, u_int32_t *); int swapctl(int cmd, const void *arg, int misc); int syscall(int, ...); int getentropy(void *, size_t); -int pledge(const char *, const char **); +int pledge(const char *, const char *); pid_t __tfork_thread(const struct __tfork *, size_t, void (*)(void *), void *); #endif /* __BSD_VISIBLE */ diff --git a/lib/libc/sys/execve.2 b/lib/libc/sys/execve.2 index 5deb91919ee..7a5ab854249 100644 --- a/lib/libc/sys/execve.2 +++ b/lib/libc/sys/execve.2 @@ -1,4 +1,4 @@ -.\" $OpenBSD: execve.2,v 1.50 2017/04/13 21:49:11 millert Exp $ +.\" $OpenBSD: execve.2,v 1.51 2017/12/12 01:12:34 deraadt Exp $ .\" $NetBSD: execve.2,v 1.9 1995/02/27 12:32:25 cgd Exp $ .\" .\" Copyright (c) 1980, 1991, 1993 @@ -30,7 +30,7 @@ .\" .\" @(#)execve.2 8.3 (Berkeley) 1/24/94 .\" -.Dd $Mdocdate: April 13 2017 $ +.Dd $Mdocdate: December 12 2017 $ .Dt EXECVE 2 .Os .Sh NAME @@ -273,6 +273,12 @@ system not allowing such operations, being mounted without the .Xr mount 8 .Fl o Cm wxallowed flag. +.It Bq Er EACCESS +The parent used +.Xr pledge 2 +to declare an +.Va execpromise , +and that is not permitted for setuid or setgid images. .It Bq Er ENOEXEC The new process file has the appropriate access permission, but has an invalid magic number in its header. diff --git a/lib/libc/sys/pledge.2 b/lib/libc/sys/pledge.2 index e568d3d7ce5..ed80121e5fd 100644 --- a/lib/libc/sys/pledge.2 +++ b/lib/libc/sys/pledge.2 @@ -1,4 +1,4 @@ -.\" $OpenBSD: pledge.2,v 1.46 2017/10/22 18:26:46 tobias Exp $ +.\" $OpenBSD: pledge.2,v 1.47 2017/12/12 01:12:34 deraadt Exp $ .\" .\" Copyright (c) 2015 Nicholas Marriott .\" @@ -14,7 +14,7 @@ .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. .\" -.Dd $Mdocdate: October 22 2017 $ +.Dd $Mdocdate: December 12 2017 $ .Dt PLEDGE 2 .Os .Sh NAME @@ -23,7 +23,7 @@ .Sh SYNOPSIS .In unistd.h .Ft int -.Fn pledge "const char *promises" "const char *paths[]" +.Fn pledge "const char *promises" "const char *execpromises" .Sh DESCRIPTION The current process is forced into a restricted-service operating mode. A few subsets are available, roughly described as computation, memory @@ -33,7 +33,7 @@ In general, these modes were selected by studying the operation of many programs using libc and other such interfaces, and setting .Ar promises or -.Ar paths . +.Ar execpromises . .Pp Use of .Fn pledge @@ -58,7 +58,7 @@ with the flag. .Pp A -.Fa promises +.Ar promises value of "" restricts the process to the .Xr _exit 2 system call. @@ -68,9 +68,9 @@ with another process. Passing .Dv NULL to -.Fa promises +.Ar promises or -.Fa paths +.Ar execpromises specifies to not change the current value. .Pp Some system calls, when allowed, have restrictions applied to them: @@ -143,9 +143,10 @@ support: system sensor readings. .Pp .It Fn pledge -Can only reduce permissions; can only set a list of -.Pa paths -once. +Can only reduce permissions for +.Ar promises +and +.Ar execpromises. .El .Pp The @@ -466,9 +467,15 @@ Allows a process to call Coupled with the .Va proc promise, this allows a process to fork and execute another program. -The new program starts running without pledge active and hopefully -makes a new -.Fn pledge . +If +.Ar execpromises +has been previously set the new program begins with those promises, +unless setuid/setgid bits are set in which case execution is blocked with +.Er EACCESS . +Otherwise the new program starts running without pledge active, +and hopefully makes a new +.Fn pledge +soon. .It Va prot_exec Allows the use of .Dv PROT_EXEC @@ -552,14 +559,24 @@ for more information on using the sndio API in combination with .It Va bpf Allow .Dv BIOCGSTATS -operation for statistics collection from a bpf device. +operation for statistics collection from a +.Xr bpf 4 +device. +.It Va error +Rather than killing the process upon violation, indicate error with +.Er ENOSYS . +.Pp +Also when +.Fn pledge +is called with higher +.Ar promises +or +.Ar execpromises , +those changes will be ignored and return success. +This is useful when a parent enforces +.Ar execpromises +but an execve'd child has a different idea. .El -.Pp -A whitelist of permitted paths may be provided in -.Ar paths . -All other paths will return -.Er ENOENT . -At least one promise is required to be pledged in order to activate a whitelist. .Sh RETURN VALUES .Rv -std .Sh ERRORS @@ -567,37 +584,18 @@ At least one promise is required to be pledged in order to activate a whitelist. will fail if: .Bl -tag -width Er .It Bq Er EFAULT -.Fa paths -or one of its elements, or -.Fa promises +.Ar promises +or +.Ar execpromises points outside the process's allocated address space. .It Bq Er EINVAL .Ar promises is malformed or contains invalid keywords. -.It Bq Er ENAMETOOLONG -An element of -.Fa paths -is too large, prepending -.Fa cwd -to it would exceed -.Dv PATH_MAX -bytes, or -.Fa promises -is too long. .It Bq Er EPERM This process is attempting to increase permissions. -.It Bq Er E2BIG -The -.Ar paths -array is too large, or the total number of bytes exceeds a -system-imposed limit. -The limit in the system as released is 262144 bytes -.Pf ( Dv ARG_MAX ) . .El .Sh HISTORY The .Fn pledge system call first appeared in .Ox 5.9 . -.Sh BUGS -The path whitelist feature is not available at this time. diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 3c65a3a9a44..c12c5848240 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_exec.c,v 1.189 2017/08/29 02:51:27 deraadt Exp $ */ +/* $OpenBSD: kern_exec.c,v 1.190 2017/12/12 01:12:34 deraadt Exp $ */ /* $NetBSD: kern_exec.c,v 1.75 1996/02/09 18:59:28 christos Exp $ */ /*- @@ -139,6 +139,13 @@ check_exec(struct proc *p, struct exec_package *epp) goto bad1; } + /* SUID programs may not be started with execpromises */ + if ((epp->ep_vap->va_mode & (VSUID | VSGID)) && + (p->p_p->ps_flags & PS_EXECPLEDGE)) { + error = EACCES; + goto bad1; + } + if ((vp->v_mount->mnt_flag & MNT_NOSUID)) epp->ep_vap->va_mode &= ~(VSUID | VSGID); @@ -520,7 +527,13 @@ sys_execve(struct proc *p, void *v, register_t *retval) else atomic_clearbits_int(&pr->ps_flags, PS_SUGIDEXEC); - atomic_clearbits_int(&pr->ps_flags, PS_PLEDGE); + if (pr->ps_flags & PS_EXECPLEDGE) { + pr->ps_pledge = pr->ps_execpledge; + atomic_setbits_int(&pr->ps_flags, PS_PLEDGE); + } else { + atomic_clearbits_int(&pr->ps_flags, PS_PLEDGE); + pr->ps_pledge = 0; + } /* * deal with set[ug]id. diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index cd5f36f6378..4a463387471 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_fork.c,v 1.200 2017/09/27 06:45:00 deraadt Exp $ */ +/* $OpenBSD: kern_fork.c,v 1.201 2017/12/12 01:12:34 deraadt Exp $ */ /* $NetBSD: kern_fork.c,v 1.29 1996/02/09 18:59:34 christos Exp $ */ /* @@ -237,7 +237,7 @@ process_new(struct proc *p, struct process *parent, int flags) vref(pr->ps_textvp); pr->ps_flags = parent->ps_flags & - (PS_SUGID | PS_SUGIDEXEC | PS_PLEDGE | PS_WXNEEDED); + (PS_SUGID | PS_SUGIDEXEC | PS_PLEDGE | PS_EXECPLEDGE | PS_WXNEEDED); if (parent->ps_session->s_ttyvp != NULL) pr->ps_flags |= parent->ps_flags & PS_CONTROLT; diff --git a/sys/kern/kern_pledge.c b/sys/kern/kern_pledge.c index 9daf916f0c7..232d3003095 100644 --- a/sys/kern/kern_pledge.c +++ b/sys/kern/kern_pledge.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kern_pledge.c,v 1.225 2017/12/09 06:50:32 deraadt Exp $ */ +/* $OpenBSD: kern_pledge.c,v 1.226 2017/12/12 01:12:34 deraadt Exp $ */ /* * Copyright (c) 2015 Nicholas Marriott @@ -84,7 +84,9 @@ #endif uint64_t pledgereq_flags(const char *req); -int canonpath(const char *input, char *buf, size_t bufsize); +int parsepledges(struct proc *p, const char *kname, + const char *promises, u_int64_t *fp); +int canonpath(const char *input, char *buf, size_t bufsize); /* #define DEBUG_PLEDGE */ #ifdef DEBUG_PLEDGE @@ -367,6 +369,7 @@ static const struct { { "dns", PLEDGE_DNS }, { "dpath", PLEDGE_DPATH }, { "drm", PLEDGE_DRM }, + { "error", PLEDGE_ERROR }, { "exec", PLEDGE_EXEC }, { "fattr", PLEDGE_FATTR | PLEDGE_CHOWN }, { "flock", PLEDGE_FLOCK }, @@ -393,67 +396,91 @@ static const struct { { "wpath", PLEDGE_WPATH }, }; +int +parsepledges(struct proc *p, const char *kname, const char *promises, u_int64_t *fp) +{ + size_t rbuflen; + char *rbuf, *rp, *pn; + u_int64_t flags = 0, f; + int error; + + rbuf = malloc(MAXPATHLEN, M_TEMP, M_WAITOK); + error = copyinstr(promises, rbuf, MAXPATHLEN, + &rbuflen); + if (error) { + free(rbuf, M_TEMP, MAXPATHLEN); + return (error); + } +#ifdef KTRACE + if (KTRPOINT(p, KTR_STRUCT)) + ktrstruct(p, kname, rbuf, rbuflen-1); +#endif + + for (rp = rbuf; rp && *rp && error == 0; rp = pn) { + pn = strchr(rp, ' '); /* find terminator */ + if (pn) { + while (*pn == ' ') + *pn++ = '\0'; + } + if ((f = pledgereq_flags(rp)) == 0) { + free(rbuf, M_TEMP, MAXPATHLEN); + return (EINVAL); + } + flags |= f; + } + free(rbuf, M_TEMP, MAXPATHLEN); + *fp = flags; + return 0; +} + int sys_pledge(struct proc *p, void *v, register_t *retval) { struct sys_pledge_args /* { - syscallarg(const char *)request; - syscallarg(const char **)paths; + syscallarg(const char *)promises; + syscallarg(const char *)execpromises; } */ *uap = v; struct process *pr = p->p_p; - uint64_t flags = 0; + uint64_t promises, execpromises; int error; - if (SCARG(uap, request)) { - size_t rbuflen; - char *rbuf, *rp, *pn; - uint64_t f; - - rbuf = malloc(MAXPATHLEN, M_TEMP, M_WAITOK); - error = copyinstr(SCARG(uap, request), rbuf, MAXPATHLEN, - &rbuflen); - if (error) { - free(rbuf, M_TEMP, MAXPATHLEN); + if (SCARG(uap, promises)) { + error = parsepledges(p, "pledgereq", + SCARG(uap, promises), &promises); + if (error) return (error); - } -#ifdef KTRACE - if (KTRPOINT(p, KTR_STRUCT)) - ktrstruct(p, "pledgereq", rbuf, rbuflen-1); -#endif - - for (rp = rbuf; rp && *rp && error == 0; rp = pn) { - pn = strchr(rp, ' '); /* find terminator */ - if (pn) { - while (*pn == ' ') - *pn++ = '\0'; - } - if ((f = pledgereq_flags(rp)) == 0) { - free(rbuf, M_TEMP, MAXPATHLEN); - return (EINVAL); - } - flags |= f; - } - free(rbuf, M_TEMP, MAXPATHLEN); + /* In "error" mode, ignore promise increase requests, + * but accept promise decrease requests */ + if (ISSET(pr->ps_flags, PS_PLEDGE) && + (pr->ps_pledge & PLEDGE_ERROR)) + promises &= (pr->ps_pledge & PLEDGE_USERSET); - /* - * if we are already pledged, allow only promises reductions. - * flags doesn't contain flags outside _USERSET: they will be - * relearned. - */ + /* Only permit reductions */ if (ISSET(pr->ps_flags, PS_PLEDGE) && - (((flags | pr->ps_pledge) != pr->ps_pledge))) + (((promises | pr->ps_pledge) != pr->ps_pledge))) return (EPERM); } + if (SCARG(uap, execpromises)) { + error = parsepledges(p, "pledgeexecreq", + SCARG(uap, execpromises), &execpromises); + if (error) + return (error); - if (SCARG(uap, paths)) - return (EINVAL); + /* Only permit reductions */ + if (ISSET(pr->ps_flags, PS_EXECPLEDGE) && + (((execpromises | pr->ps_execpledge) != pr->ps_execpledge))) + return (EPERM); + } - if (SCARG(uap, request)) { - pr->ps_pledge = flags; + if (SCARG(uap, promises)) { + pr->ps_pledge = promises; pr->ps_flags |= PS_PLEDGE; } - + if (SCARG(uap, execpromises)) { + pr->ps_execpledge = execpromises; + pr->ps_flags |= PS_EXECPLEDGE; + } return (0); } @@ -489,13 +516,16 @@ pledge_fail(struct proc *p, int error, uint64_t code) codes = pledgenames[i].name; break; } - log(LOG_ERR, "%s[%d]: pledge \"%s\", syscall %d\n", - p->p_p->ps_comm, p->p_p->ps_pid, codes, p->p_pledge_syscall); - p->p_p->ps_acflag |= APLEDGE; #ifdef KTRACE if (KTRPOINT(p, KTR_PLEDGE)) ktrpledge(p, error, code, p->p_pledge_syscall); #endif + if (p->p_p->ps_pledge & PLEDGE_ERROR) + return (ENOSYS); + + log(LOG_ERR, "%s[%d]: pledge \"%s\", syscall %d\n", + p->p_p->ps_comm, p->p_p->ps_pid, codes, p->p_pledge_syscall); + p->p_p->ps_acflag |= APLEDGE; /* Send uncatchable SIGABRT for coredump */ memset(&sa, 0, sizeof sa); sa.sa_handler = SIG_DFL; @@ -1394,6 +1424,9 @@ pledge_protexec(struct proc *p, int prot) { if ((p->p_p->ps_flags & PS_PLEDGE) == 0) return 0; + /* Before kbind(2) call, ld.so and crt may create EXEC mappings */ + if (p->p_p->ps_kbind_addr == 0 && p->p_p->ps_kbind_cookie == 0) + return 0; if (!(p->p_p->ps_pledge & PLEDGE_PROTEXEC) && (prot & PROT_EXEC)) return pledge_fail(p, EPERM, PLEDGE_PROTEXEC); return 0; diff --git a/sys/kern/syscalls.master b/sys/kern/syscalls.master index eb3e0b78534..c9411db3ba1 100644 --- a/sys/kern/syscalls.master +++ b/sys/kern/syscalls.master @@ -1,4 +1,4 @@ -; $OpenBSD: syscalls.master,v 1.179 2017/11/28 06:03:41 guenther Exp $ +; $OpenBSD: syscalls.master,v 1.180 2017/12/12 01:12:34 deraadt Exp $ ; $NetBSD: syscalls.master,v 1.32 1996/04/23 10:24:21 mycroft Exp $ ; @(#)syscalls.master 8.2 (Berkeley) 1/13/94 @@ -227,7 +227,8 @@ 106 STD { int sys_listen(int s, int backlog); } 107 STD { int sys_chflagsat(int fd, const char *path, \ u_int flags, int atflags); } -108 STD { int sys_pledge(const char *request, const char **paths); } +108 STD { int sys_pledge(const char *promises, \ + const char *execpromises); } 109 STD { int sys_ppoll(struct pollfd *fds, \ u_int nfds, const struct timespec *ts, \ const sigset_t *mask); } diff --git a/sys/sys/pledge.h b/sys/sys/pledge.h index 4a59d112c1d..832f170e146 100644 --- a/sys/sys/pledge.h +++ b/sys/sys/pledge.h @@ -1,4 +1,4 @@ -/* $OpenBSD: pledge.h,v 1.32 2017/08/29 02:51:27 deraadt Exp $ */ +/* $OpenBSD: pledge.h,v 1.33 2017/12/12 01:12:34 deraadt Exp $ */ /* * Copyright (c) 2015 Nicholas Marriott @@ -59,6 +59,7 @@ #define PLEDGE_CHOWN 0x0000000080000000ULL /* chown(2) family */ #define PLEDGE_CHOWNUID 0x0000000100000000ULL /* allow owner/group changes */ #define PLEDGE_BPF 0x0000000200000000ULL /* bpf ioctl */ +#define PLEDGE_ERROR 0x0000000400000000ULL /* ENOSYS instead of kill */ /* * Bits outside PLEDGE_USERSET are used by the kernel itself @@ -105,6 +106,7 @@ static struct { { PLEDGE_VMM, "vmm" }, { PLEDGE_CHOWNUID, "chown" }, { PLEDGE_BPF, "bpf" }, + { PLEDGE_ERROR, "error" }, { 0, NULL }, }; #endif diff --git a/sys/sys/proc.h b/sys/sys/proc.h index d8861f1d896..60088f49c85 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -1,4 +1,4 @@ -/* $OpenBSD: proc.h,v 1.240 2017/08/29 02:51:27 deraadt Exp $ */ +/* $OpenBSD: proc.h,v 1.241 2017/12/12 01:12:34 deraadt Exp $ */ /* $NetBSD: proc.h,v 1.44 1996/04/22 01:23:21 christos Exp $ */ /*- @@ -220,6 +220,7 @@ struct process { u_short ps_acflag; /* Accounting flags. */ uint64_t ps_pledge; + uint64_t ps_execpledge; int64_t ps_kbind_cookie; u_long ps_kbind_addr; @@ -262,13 +263,14 @@ struct process { #define PS_NOBROADCASTKILL 0x00080000 /* Process excluded from kill -1. */ #define PS_PLEDGE 0x00100000 /* Has called pledge(2) */ #define PS_WXNEEDED 0x00200000 /* Process may violate W^X */ +#define PS_EXECPLEDGE 0x00400000 /* Has exec pledges */ #define PS_BITS \ ("\20" "\01CONTROLT" "\02EXEC" "\03INEXEC" "\04EXITING" "\05SUGID" \ "\06SUGIDEXEC" "\07PPWAIT" "\010ISPWAIT" "\011PROFIL" "\012TRACED" \ "\013WAITED" "\014COREDUMP" "\015SINGLEEXIT" "\016SINGLEUNWIND" \ "\017NOZOMBIE" "\020STOPPED" "\021SYSTEM" "\022EMBRYO" "\023ZOMBIE" \ - "\024NOBROADCASTKILL" "\025PLEDGE" "\026WXNEEDED") + "\024NOBROADCASTKILL" "\025PLEDGE" "\026WXNEEDED", "\027EXECPLEDGE") struct lock_list_entry; diff --git a/usr.bin/kdump/ktrstruct.c b/usr.bin/kdump/ktrstruct.c index d446b68a89f..0b3f06ae8eb 100644 --- a/usr.bin/kdump/ktrstruct.c +++ b/usr.bin/kdump/ktrstruct.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ktrstruct.c,v 1.23 2016/10/08 02:16:43 guenther Exp $ */ +/* $OpenBSD: ktrstruct.c,v 1.24 2017/12/12 01:12:34 deraadt Exp $ */ /*- * Copyright (c) 1988, 1993 @@ -647,12 +647,12 @@ ktrstruct(char *buf, size_t buflen) ktrcmsghdr(cmsg, datalen); free(cmsg); } else if (strcmp(name, "pledgereq") == 0) { - printf("pledge request="); - showbufc(basecol + sizeof("pledge request=") - 1, + printf("promise="); + showbufc(basecol + sizeof("promise=") - 1, (unsigned char *)data, datalen, VIS_DQ | VIS_TAB | VIS_NL); - } else if (strcmp(name, "pledgepath") == 0) { - printf("pledge path="); - showbufc(basecol + sizeof("pledge path=") - 1, + } else if (strcmp(name, "pledgeexecreq") == 0) { + printf("execpromise="); + showbufc(basecol + sizeof("execpromise=") - 1, (unsigned char *)data, datalen, VIS_DQ | VIS_TAB | VIS_NL); } else { printf("unknown structure %s\n", name); -- 2.20.1