This has come up a few times before. For background, the default rule for doas
is to copy a few environment settings from the user and omit the rest. This is
to prevent confusion, and also supposedly for security. However, some of the
alleged safe variables like PATH probably aren't that safe. And things like
USER are confusing? And why even bother with MAIL? The list is kinda adhoc and
mostly copied from what I understood sudo to do at the time, but I believe
sudo has changed defaults as well.
So here's a new model which I think is safer and more consistent.
1. Always add a DOAS_USER with the invoking user's name.
2. When doing keepenv, nothing changes, except addition of above.
3. When starting with a new environment, fill in USER and HOME and such with
the values of the target user, instead of copying from the invoking user. You
run a command as a user, you should have that user's environment.
4. DISPLAY and TERM are still copied, since they represent a description of
the actual environment in which we are running. (Not sure how useful display
is without xauth cookies, but not my problem.)
5. As before, anything can be overridden with setenv in the config.
This is a kinda breaking change, but probably in a good way.
For simple configurations, I expect nothing changes. For more complicated
setups, this new handling is probably closer to what the admin expects or
desires.
Index: doas.c
===================================================================
RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.76
diff -u -p -r1.76 doas.c
--- doas.c 12 Jun 2019 02:50:29 -0000 1.76
+++ doas.c 13 Jun 2019 02:11:07 -0000
@@ -421,6 +421,7 @@ main(int argc, char **argv)
errx(1, "no passwd entry for target");
if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
+ LOGIN_SETPATH |
LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
LOGIN_SETUSER) != 0)
errx(1, "failed to set user context for target");
@@ -439,8 +440,13 @@ main(int argc, char **argv)
syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
mypw->pw_name, cmdline, targpw->pw_name, cwd);
- envp = prepenv(rule);
+ envp = prepenv(rule, mypw, targpw);
+ if (rule->cmd) {
+ /* do this again after setusercontext reset it */
+ if (setenv("PATH", safepath, 1) == -1)
+ err(1, "failed to set PATH '%s'", safepath);
+ }
execvpe(cmd, argv, envp);
fail:
if (errno == ENOENT)
Index: doas.conf.5
===================================================================
RCS file: /home/cvs/src/usr.bin/doas/doas.conf.5,v
retrieving revision 1.35
diff -u -p -r1.35 doas.conf.5
--- doas.conf.5 7 Feb 2018 05:13:57 -0000 1.35
+++ doas.conf.5 13 Jun 2019 01:41:18 -0000
@@ -51,15 +51,9 @@ again for some time.
.It Ic keepenv
The user's environment is maintained.
The default is to reset the environment, except for the variables
-.Ev DISPLAY ,
-.Ev HOME ,
-.Ev LOGNAME ,
-.Ev MAIL ,
-.Ev PATH ,
-.Ev TERM ,
-.Ev USER
+.Ev DISPLAY
and
-.Ev USERNAME .
+.Ev TERM .
.It Ic setenv { Oo Ar variable ... Oc Oo Ar variable=value ... Oc Ic }
In addition to the variables mentioned above, keep the space-separated
specified variables.
Index: doas.h
===================================================================
RCS file: /home/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 Jun 2019 01:10:29 -0000
@@ -29,7 +29,10 @@ extern struct rule **rules;
extern int nrules;
extern int parse_errors;
-char **prepenv(const struct rule *);
+struct passwd;
+
+char **prepenv(const struct rule *, const struct passwd *,
+ const struct passwd *);
#define PERMIT 1
#define DENY 2
Index: env.c
===================================================================
RCS file: /home/cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.6
diff -u -p -r1.6 env.c
--- env.c 6 Apr 2017 21:12:06 -0000 1.6
+++ env.c 13 Jun 2019 02:12:57 -0000
@@ -24,6 +24,7 @@
#include <err.h>
#include <unistd.h>
#include <errno.h>
+#include <pwd.h>
#include "doas.h"
@@ -38,6 +39,8 @@ struct env {
u_int count;
};
+static void fillenv(struct env *env, const char **envlist);
+
static int
envcmp(struct envnode *a, struct envnode *b)
{
@@ -68,8 +71,19 @@ freenode(struct envnode *node)
free(node);
}
+static void
+addnode(struct env *env, const char *key, const char *value)
+{
+ struct envnode *node;
+
+ node = createnode(key, value);
+ RB_INSERT(envtree, &env->root, node);
+ env->count++;
+}
+
static struct env *
-createenv(const struct rule *rule)
+createenv(const struct rule *rule, const struct passwd *mypw,
+ const struct passwd *targpw)
{
struct env *env;
u_int i;
@@ -80,6 +94,8 @@ createenv(const struct rule *rule)
RB_INIT(&env->root);
env->count = 0;
+ addnode(env, "DOAS_USER", mypw->pw_name);
+
if (rule->options & KEEPENV) {
extern const char **environ;
@@ -108,6 +124,19 @@ createenv(const struct rule *rule)
env->count++;
}
}
+ } else {
+ static const char *copyset[] = {
+ "DISPLAY", "TERM",
+ NULL
+ };
+
+ addnode(env, "HOME", targpw->pw_dir);
+ addnode(env, "LOGNAME", targpw->pw_name);
+ addnode(env, "PATH", getenv("PATH"));
+ addnode(env, "SHELL", targpw->pw_shell);
+ addnode(env, "USER", targpw->pw_name);
+
+ fillenv(env, copyset);
}
return env;
@@ -186,20 +215,12 @@ fillenv(struct env *env, const char **en
}
char **
-prepenv(const struct rule *rule)
+prepenv(const struct rule *rule, const struct passwd *mypw,
+ const struct passwd *targpw)
{
- static const char *safeset[] = {
- "DISPLAY", "HOME", "LOGNAME", "MAIL",
- "PATH", "TERM", "USER", "USERNAME",
- NULL
- };
struct env *env;
- env = createenv(rule);
-
- /* if we started with blank, fill some defaults then apply rules */
- if (!(rule->options & KEEPENV))
- fillenv(env, safeset);
+ env = createenv(rule, mypw, targpw);
if (rule->envlist)
fillenv(env, rule->envlist);
