Martijn van Duren wrote:
> > 2. When doing keepenv, nothing changes, except addition of above.
>
> It feels inconsistent to make keepenv behave different here.
> - It may allow certain applications to behave unexpected compared to
> without keepenv (e.g. scripts that use $HOME/.cache).
> - The values are hard to retrofit into doas.conf by the end-user, while
> it's easy to keep the original with setenv { HOME ... }.
Yes, I think it's better to always reset these things. Here's a diff.
1. Always set HOME, PATH, SHELL etc to the target.
2. Without keepenv, other environment variables are discarded.
3. With keepenv, other variables are retained, but the above are still reset.
4. Possible to override this with setenv.
This is much more consistent and predictable.
Index: doas.conf.5
===================================================================
RCS file: /cvs/src/usr.bin/doas/doas.conf.5,v
retrieving revision 1.36
diff -u -p -r1.36 doas.conf.5
--- doas.conf.5 16 Jun 2019 18:16:34 -0000 1.36
+++ doas.conf.5 16 Jun 2019 18:20:20 -0000
@@ -54,6 +54,14 @@ The default is to reset the environment,
.Ev DISPLAY
and
.Ev TERM .
+The variables
+.Ev HOME ,
+.Ev LOGNAME ,
+.Ev PATH ,
+.Ev SHELL ,
+and
+.Ev USER
+are always reset.
.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: env.c
===================================================================
RCS file: /cvs/src/usr.bin/doas/env.c,v
retrieving revision 1.7
diff -u -p -r1.7 env.c
--- env.c 16 Jun 2019 18:16:34 -0000 1.7
+++ env.c 16 Jun 2019 18:20:20 -0000
@@ -85,6 +85,10 @@ static struct env *
createenv(const struct rule *rule, const struct passwd *mypw,
const struct passwd *targpw)
{
+ static const char *copyset[] = {
+ "DISPLAY", "TERM",
+ NULL
+ };
struct env *env;
u_int i;
@@ -95,6 +99,13 @@ createenv(const struct rule *rule, const
env->count = 0;
addnode(env, "DOAS_USER", mypw->pw_name);
+ 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);
if (rule->options & KEEPENV) {
extern const char **environ;
@@ -124,19 +135,6 @@ createenv(const struct rule *rule, const
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;
