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 },
