opendoas

commit 01c658f8c45cb92a343be5f32aa6da70b2032168
parent dbc7d06b5bbf01652744423bd8825ea7b5e92f73
Author: tedu <tedu>
Date:   Sun, 16 Jun 2019 18:16:34 +0000

redo the environment inheritance to not inherit. it was intended to make life easier, but it can be surprising or even unsafe. instead, reset just about everything to the target user's values. ok deraadt martijn Thanks to Sander Bos in particular for pointing out some nasty edge cases.

Diffstat:
M
doas.c
 | 
4
+++-
M
doas.conf.5
 | 
10
++--------
M
doas.h
 | 
5
++++-
M
env.c
 | 
45
+++++++++++++++++++++++++++++++++------------

4 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/doas.c b/doas.c
@@ -449,6 +449,7 @@ main(int argc, char **argv)
 
 #ifdef HAVE_SETUSERCONTEXT
 	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");
@@ -479,9 +480,10 @@ 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);
 	}
diff --git a/doas.conf.5 b/doas.conf.5
@@ -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.
diff --git a/doas.h b/doas.h
@@ -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
diff --git a/env.c b/env.c
@@ -24,6 +24,7 @@
 #include <err.h>
 #include <unistd.h>
 #include <errno.h>
+#include <pwd.h>
 
 #include "doas.h"
 #include "includes.h"
@@ -39,6 +40,8 @@ struct env {
 	u_int count;
 };
 
+static void fillenv(struct env *env, const char **envlist);
+
 static int
 envcmp(struct envnode *a, struct envnode *b)
 {
@@ -69,8 +72,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;
@@ -81,6 +95,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 char **environ;
 
@@ -109,6 +125,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;
@@ -187,20 +216,12 @@ fillenv(struct env *env, const char **envlist)
 }
 
 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);