Extend /usr/bin/service with the possibility to set ENV vars

[netchild]

Summary

This allows for quicker testing/debugging/modifications of service scripts by specifying env vars at the command line at service start/stop/...
It is required to be able to implement automatic service jails (the implementation will come in a different review and requires to pass info via env vars into services).

[se]

Could the option to allow arbitrary environment variables being passed to the rc scripts create vulnerabilities?
I do not see any, since the service scripts could already be started without the use of the service command with arbitrary environment variables.
But I'm not sure whether the possibility to override HOME and PATH is a good idea, and excluding LD_PRELOAD and LD_LIBRARY_PATH from being passed this way might be a good idea.

usr.sbin/service/service.sh

43

Minor nit: the use of \t causes the option descriptions to occur in different columns, since "-E n=val" has 8 chracters:

-j	Perform actions within the named jail
-E n=val	Set variable n to val before executing the rc.d script
-e	Show services that are enabled
-R	Stop and start enabled /usr/local/etc/rc.d services
-l	List all scripts in /etc/rc.d and /usr/local/etc/rc.d
-r	Show the results of boot time rcorder
-v	Verbose

78

Is there a risk that quoting is lost?

Maybe better use:

	args="${args} -E \"${var}\""

The same comment may apply to line 56 where $VARS collects the arguments provided with -E, in case of e.g. '-E var="value with blanks"' being passed ...

179

Should it be possible to override HOME and PATH with -E?

Maybe re-order to make HOME and PATH override earlier values passed in via $VARS:

	exec env -i -L -/daemon ${VARS} HOME=/ PATH=/sbin:/bin:/usr/sbin:/usr/bin "$dir/$script" "$@"

But perhaps the extra flexibility to modify HOME and PATH is of use in legitimate use-cases?

[netchild]

The sanitization in the service script is mainly to decouple the users shell env from the services, so that the services start like at boot (as a real security feature, the sanitization would have to happen in the rc infrastructure so that calling the rc.d script with the full path automatically gets the sanitization). See also https://cgit.freebsd.org/src/commit/usr.sbin/service/service.sh?id=ee55fdb8fa1ce60d39bd7f2afde7d9772a2e3d58 for the corresponding log entry.

As services normally have to be started as root anyway, the normal use case is not affected by this. There may be cases with unexpected behavior if someone tries to start as an user (e.g. overwriting or creation of something), but as our normal start doesn't involve sudo or such, this would happen in the user context, not with elevated privileagues.

The possibility to override LD_ vars I currently see as a feature. There are different malloc implementations in the ports tree (e.g. mimalloc) and this would allow to experiment on a per service basis instead of on a global basis with such stuff. And I can use the full path to the script if the service command would filter it out.

Additionally, there is currently no way to automatically insert "-E X=Y" with or without arbritary values of X or Y in the automated startup, as such any use would be manually, and any malicious use would then be able to use the full path to the script instead of the service command.

usr.sbin/service/service.sh

78

As you can specify -E multiple times: do you have an idea how to not lose quoting in such cases (the args="$args -E $var" and VARS="$VARS $OPTARG" parts above may have quoted vars already and to my knowledge I would lose those quotes when -E is used one more time after such a quoted var)?

179

For my use case it doesn't matter. But in the general case if I would use -E I would assume it will do what I want and if I have HOME inside there, I would scratch my head why it doesn't work. Once I changed one of my ports from no home (/nonexistent) to an existing home as it started to need it. It's easy to use the full path in the testing to the rc.d script where no such sanitization exists, but using the service command is faster/shorter.

See also my upcoming comment about vulnerabilities.

[netchild]

Fix formatting in usage().

[netchild]

Updated patch fixes the formatting of the output of usage().

[se]

My comments have been addressed and I think it makes sense to go ahead with the proposed patches.