On 5/13/19 10:00 AM, Martijn van Duren wrote: > On 5/13/19 9:13 AM, Marc Espie wrote: >> So, in dpb, I've been forking a lot of 'chroot -u user /build' >> to build various things, and it works just great. >> >> I was wondering about the benefits of >> su ${BUILDUSER} -c 'some quoted command' >> vs >> chroot -u ${BUILDUSER} / some unquoted command >> >> Superficially, it looks mostly similar. >> >> The very nice thing about chroot (IMO) being that you can pass the >> command verbatim without having to re-quote anything. The other >> difference being that chroot doesn't fork an extra shell, which >> might make things more transparent wrt running commands. >> >> I'm also wondering about doas. >> By default, it's not configured at all. >> >> But what would it hurt to allow root usage ? >> Specifically, >> >> doas -u ${BUILDUSER} some unquoted command >> >> as run by root. This would not open any security hole, would it ? > > I don't see any and I've been bitten by having a rootshell open and > typing doas out of habit. > > lightly tested diff below. >> >> Finally, I'm wondering if people would have any use for a chroot'd >> option in doas, and whether it's a security issue (again). >> >> Like, people have some hardened doas.conf which only allows running >> some commands as root. >> >> Some of these commands are basically game over, as they allow anything >> to be run, actually. Specifically, /usr/bin/env, or chroot... >> >> Would >> doas -c /rootdir somecmd >> be of any use ? > > Not particularly opposed, but the extend of this option should be > examined. E.g. do we want to extend it to the config to be something > similar to -u and limit it's use for certain commands? > Working this out I'm not particularly a fan of the extra code this would take, but the diff below seems to do the trick.
martijn@ Index: doas.c =================================================================== RCS file: /cvs/src/usr.bin/doas/doas.c,v retrieving revision 1.74 diff -u -p -r1.74 doas.c --- doas.c 17 Jan 2019 05:35:35 -0000 1.74 +++ doas.c 13 May 2019 09:23:41 -0000 @@ -89,8 +89,9 @@ parsegid(const char *s, gid_t *gid) } static int -match(uid_t uid, gid_t *groups, int ngroups, uid_t target, const char *cmd, - const char **cmdargs, struct rule *r) +match(uid_t uid, gid_t *groups, int ngroups, uid_t target, + const char *chrootpath, const char *cmd, const char **cmdargs, + struct rule *r) { int i; @@ -110,6 +111,12 @@ match(uid_t uid, gid_t *groups, int ngro } if (r->target && uidcheck(r->target, target) != 0) return 0; + if (r->chroot != NULL) { + if (chrootpath == NULL) + return 0; + if (strcmp(r->chroot, chrootpath) != 0) + return 0; + } if (r->cmd) { if (strcmp(r->cmd, cmd)) return 0; @@ -130,13 +137,13 @@ match(uid_t uid, gid_t *groups, int ngro static int permit(uid_t uid, gid_t *groups, int ngroups, const struct rule **lastr, - uid_t target, const char *cmd, const char **cmdargs) + uid_t target, const char *chrootpath, const char *cmd, const char **cmdargs) { int i; *lastr = NULL; for (i = 0; i < nrules; i++) { - if (match(uid, groups, ngroups, target, cmd, + if (match(uid, groups, ngroups, target, chrootpath, cmd, cmdargs, rules[i])) *lastr = rules[i]; } @@ -173,7 +180,7 @@ parseconfig(const char *filename, int ch } static void __dead -checkconfig(const char *confpath, int argc, char **argv, +checkconfig(const char *confpath, int argc, char **argv, const char *chrootpath, uid_t uid, gid_t *groups, int ngroups, uid_t target) { const struct rule *rule; @@ -183,7 +190,7 @@ checkconfig(const char *confpath, int ar if (!argc) exit(0); - if (permit(uid, groups, ngroups, &rule, target, argv[0], + if (permit(uid, groups, ngroups, &rule, target, chrootpath, argv[0], (const char **)argv + 1)) { printf("permit%s\n", (rule->options & NOPASS) ? " nopass" : ""); exit(0); @@ -286,6 +293,7 @@ main(int argc, char **argv) const char *confpath = NULL; char *shargv[] = { NULL, NULL }; char *sh; + const char *chrootpath = NULL; const char *cmd; char cmdline[LINE_MAX]; char myname[_PW_NAME_LEN + 1]; @@ -309,11 +317,14 @@ main(int argc, char **argv) uid = getuid(); - while ((ch = getopt(argc, argv, "a:C:Lnsu:")) != -1) { + while ((ch = getopt(argc, argv, "a:c:C:Lnsu:")) != -1) { switch (ch) { case 'a': login_style = optarg; break; + case 'c': + chrootpath = optarg; + break; case 'C': confpath = optarg; break; @@ -369,8 +380,8 @@ main(int argc, char **argv) } if (confpath) { - checkconfig(confpath, argc, argv, uid, groups, ngroups, - target); + checkconfig(confpath, argc, argv, chrootpath, uid, groups, + ngroups, target); exit(1); /* fail safe */ } @@ -389,7 +400,7 @@ main(int argc, char **argv) } cmd = argv[0]; - if (!permit(uid, groups, ngroups, &rule, target, cmd, + if (!permit(uid, groups, ngroups, &rule, target, chrootpath, cmd, (const char **)argv + 1)) { syslog(LOG_AUTHPRIV | LOG_NOTICE, "failed command for %s: %s", myname, cmdline); @@ -403,8 +414,17 @@ main(int argc, char **argv) authuser(myname, login_style, rule->options & PERSIST); } - if (unveil(_PATH_LOGIN_CONF, "r") == -1) - err(1, "unveil"); + pw = getpwuid(target); + if (!pw) + errx(1, "no passwd entry for target"); + + if (chrootpath != NULL) { + if (chroot(chrootpath) == -1) + err(1, "failed to chroot"); + if (chdir("/") == -1) + err(1, "failed to chdir to chroot"); + } + if (rule->cmd) { if (setenv("PATH", safepath, 1) == -1) err(1, "failed to set PATH '%s'", safepath); @@ -415,9 +435,6 @@ main(int argc, char **argv) if (pledge("stdio rpath getpw exec id", NULL) == -1) err(1, "pledge"); - pw = getpwuid(target); - if (!pw) - errx(1, "no passwd entry for target"); if (setusercontext(NULL, pw, target, LOGIN_SETGROUP | LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK | Index: doas.h =================================================================== RCS file: /cvs/src/usr.bin/doas/doas.h,v retrieving revision 1.13 diff -u -p -r1.13 doas.h --- doas.h 6 Apr 2017 21:12:06 -0000 1.13 +++ doas.h 13 May 2019 09:23:41 -0000 @@ -20,6 +20,7 @@ struct rule { int options; const char *ident; const char *target; + const char *chroot; const char *cmd; const char **cmdargs; const char **envlist; Index: parse.y =================================================================== RCS file: /cvs/src/usr.bin/doas/parse.y,v retrieving revision 1.27 diff -u -p -r1.27 parse.y --- parse.y 11 Jul 2018 07:39:22 -0000 1.27 +++ parse.y 13 May 2019 09:23:41 -0000 @@ -69,7 +69,7 @@ arraylen(const char **arr) %} -%token TPERMIT TDENY TAS TCMD TARGS +%token TPERMIT TDENY TAS TCHROOT TCMD TARGS %token TNOPASS TPERSIST TKEEPENV TSETENV %token TSTRING @@ -81,7 +81,7 @@ grammar: /* empty */ | error '\n' ; -rule: action ident target cmd { +rule: action ident target chroot cmd { struct rule *r; r = calloc(1, sizeof(*r)); if (!r) @@ -91,8 +91,9 @@ rule: action ident target cmd { r->envlist = $1.envlist; r->ident = $2.str; r->target = $3.str; - r->cmd = $4.cmd; - r->cmdargs = $4.cmdargs; + r->chroot = $4.str; + r->cmd = $5.cmd; + r->cmdargs = $5.cmdargs; if (nrules == maxrules) { if (maxrules == 0) maxrules = 63; @@ -170,6 +171,12 @@ target: /* optional */ { $$.str = $2.str; } ; +chroot: /* optional */ { + $$.str = NULL; + } | TCHROOT TSTRING { + $$.str = $2.str; + } ; + cmd: /* optional */ { $$.cmd = NULL; $$.cmdargs = NULL; @@ -206,6 +213,7 @@ static struct keyword { { "deny", TDENY }, { "permit", TPERMIT }, { "as", TAS }, + { "chroot", TCHROOT }, { "cmd", TCMD }, { "args", TARGS }, { "nopass", TNOPASS },