Add DEFAULT_PATH to match de-facto Linux standards

[TheDcoder]

DEFAULT_PATH replaces safepath for setting the PATH variable in the executed process's environment.

DEFAULT_PATH follows the de-facto standard of Linux distributions which place /usr/local directories before their non-local counterparts in $PATH.

Unlike BSD, Linux distributions don't put packaged executables under /usr/local, instead it is used by the local user to place their own executables, potentially to replace system executables.

This fixes #117

[Duncaen]

I feel like the argument that BSD's install ports to /usr/local is more an argument for removing /usr/local from the default path for linux, not that it should have higher priority by default.

If this is changed then it should probably be a build time configuration option and not just a new order that is the de-facto standard for linux distributions.

[TheDcoder]

I agree that it's more of an argument for removing /usr/local, however since this has always been present in doas and most of OpenDoas's users are on Linux anyway, we should use this for simplicity and backward compatibility's sake.

I propose that we make safepath configurable at build time but let's set the default value to the one I proposed in this patch.

[Duncaen]

Looking over this again, I don't think safepath should be changed. The safepath variable is for running specifically permitted commands and I don't really want to change that behavior.

I think there should be a new variable like DEFAULTPATH that is used for the resulting environment.

IMHO doas.c:400 should use that default path, this is where opendoas differes from the BSDs where setusercontext sets the default path from /etc/login.conf, opendoas used safepath instead:

OpenDoas/doas.c

Lines 387 to 402 in b96106b

	#ifdef HAVE_LOGIN_CAP_H
	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");
	#else
	if (setresgid(targpw->pw_gid, targpw->pw_gid, targpw->pw_gid) != 0)
	err(1, "setresgid");
	if (initgroups(targpw->pw_name, targpw->pw_gid) != 0)
	err(1, "initgroups");
	if (setresuid(target, target, target) != 0)
	err(1, "setresuid");
	if (setenv("PATH", safepath, 1) == -1)
	err(1, "failed to set PATH '%s'", safepath);
	#endif

This way the behavior of looking up specific binaries doesn't change, but the resulting environment of executed commands reflects the more lax de-facto standard path and is more like the workaround with setenv { PATH=/usr/local/bin... }

[TheDcoder]

Ah, that makes more sense. In that case I think we're better off removing /usr/local from safepath entirely and implementing DEFAULTPATH. A way to make it user-configurable would be nice too but I don't think it can be easily added to doas.conf, or am I wrong?

[Duncaen]

I think changing safepath now would be too much of a regression for very little to no real benefit, the important part IMHO is the order so that we don't change what commands existing rules for specific commands may execute.

I don't think making safepath configurable is an option, I don't want to divert from the configuration format or the behavior of the original doas to avoid fragmentation.

The default path for rules that basically permit everything this is different since this is also configurable through login.conf in the original doas and opendoas diverted here and used the restrictive safepath instead. Users who want a different binary than the ones that comes from the safepath lookup can use permit /some/random/path/bin if required.

[TheDcoder]

Understandable, I wasn't really suggesting making safepath configurable, but I do want the default path to be user configurable so that they won't have to use the env option for each rule.

I also agree that deviating from the configuration format is a bad idea... however maybe we can add something which still conforms to the existing format spec? Or maybe we can add an additional config file which is optional but it can be used to change the default path.

The latter seems overkill to me since we don't really need it for anything else.

For now though let's just make default path configurable at build time, that should satisfy everyone.

[Duncaen]

The default path is already configurable through setenv or am I misunderstanding this?

[TheDcoder]

Maybe I misunderstood what you meant by this:

I think there should be a new variable like DEFAULTPATH that is used for the resulting environment.

I thought we were going to implement a new DEFAULTPATH which will be used for the resulting environment for the program which will be executed once authentication is successful. safepath would still be present but it will only be used for searching for the executable if the full path is not specified in config.

[Duncaen]

I thought we were going to implement a new DEFAULTPATH which will be used for the resulting environment for the program which will be executed once authentication is successful.

Correct, and with will still be overwritable with setenv { PATH="something else" }.

safepath would still be present but it will only be used for searching for the executable if the full path is not specified in config.

Correct safepath is used when there is a rule for a specific command like permit as root cmd poweroff.
If a user wants to allow poweroff from rules/users can define specific binaries if safepath is too restrictive for them.
The PATH variable for those commands will be the safepath by default, but setenv can still overwrite it.
The search path/safepath is not and IMHO should not be configurable to have at least a somewhat predictable behavior.

rules like permit :wheel don't use safepath at all, those basically grant everything, they use the executing user PATH, but will set the PATH to DEFAULTPATH (currently safepath) or whatever is defined in the configuration as permit :wheel setenv { PATH=... }.

[TheDcoder]

I'm sorry for the confusion, I'll outline what I'm proposing:

1. safepath should not be configurable at all
2. DEFAULTPATH should get assigned a new value at build time, this value will put /usr/local paths before their counterparts.
3. DEFAULTPATH should be configurable at build time so that it can be adjusted by distributors for their specific needs
4. DEFAULTPATH is the default value for setenv { PATH = ...}
5. (optional) In the future, ideally we will figure out a way to set DEFAULTPATH with user configuration so that they don't have to use setenv for each rule.

I hope I'm clear now 😄

[Duncaen]

Agreed.

[Wabuo]

Just checking in.

Is this project still alive?
Just stumbled on this issue following a link from the ArchWiki ...

[TheDcoder]

@Wabuo I don't think so. I've started writing an alternative doas implementation in Rust, the basics are working but I've to implement PAM support so it might be a good replacement once it is released.

[Wabuo]

Cheers, so I might as well uninstall it again ...

@TheDcoder
Are you aware of https://github.com/LeChatP/RootAsRole, and https://github.com/memorysafety/sudo-rs the sudo rewrite in rust?
And this Reddit https://www.reddit.com/r/rust/comments/165ack9/rootasrole_a_secure_alternative_to_sudosu_on/

[TheDcoder]

@Wabuo I was aware of the sudo rewrite project but it does not interest me as I don't like sudo. I wasn't aware of RootAsRole, it is an interesting project taking a novel approach, thanks for sharing.

I like doas because it's pretty simple and fits my needs, no need for complex configuration... and you probably should reconsider if you really want to be able to do something as root while logged in as a user if it requires complex configuration which can't be provided by simple instructions in doas.con.