Make a start at supporting login.conf environment settings

[kevans]

Summary

There is currently no single place where environment variables can be set that are required by daemons and cron jobs in addition to logged-in users.

The most glaring issue with this is when setting up a system with no public connectivity other than an HTTP proxy, there is no single place that allows you to set the proxy address that is recognized by both the ntpd leapsecond file fetch (invoked from cron via "service ntpd onefetch" which clears the environment) and the package vulnerability database fetch (invoked from cron).

This patch proposes four substantive changes (and one largely cosmetic cleanup):

1. init(8) sets the environment variables of the "daemon" class when running /etc/rc (and also those of "default" when running other processes).

2. cron(8) sets the environment variables of the user and/or login class for which it is invoking a job, prior to processing environment variable settings in the crontab file.

3. env(1) gets options -L user/class and -U user/class to set the environment of the specified user either from login.conf alone (-L) or both login.conf and ~/.login_conf if present (-U). This is to enable:

4. service(8) sets the environment of the "daemon" class before invoking the rc script.

5. The MAIL environment variable gets split out into a separate "mail" capability because things work more cleanly that way (easier to suppress it in the "daemon" class where it will not be properly expanded, for example).

This is a fairly early work in progress, in particular there is no documentation yet.

Test Plan

Open to suggestions about what specifically needs testing.

[kevans]

Dropping in jilles@ and dteske@ to start... if memory serves they both might be interested in reviewing this.

[andrew_tao173.riddles.org.uk]

I've been pondering whether limits(1) also needs an option to pick up the environment from the login class, or whether that's better addressed in rc.subr for the case of servicename_class="x" settings.

[andrew_tao173.riddles.org.uk]

Further proposal (the patch doesn't do this yet): cron should not override PATH when reading a user crontab; whether it should use the user's path, or daemon's path, rather than the hardcoded default when processing the system crontab is a more open question.

[kevans]

usr.bin/env/env.c

79

This and all the following initialization should move out of the declaration block, down by altpath, per style(9)

103

Spaces around FALLTHROUGH

167

Blank line before multi-line comment, and comment should begin on line after starting comment marker in this file

[andrew_tao173.riddles.org.uk]

Style fixes. Rebase.

[kevans]

I'd like to start moving forward on this... this looks like a fine idea to me. I'll look at starting to commit it over the next week, likely broken up into the various logical changes you've outlined.

[andrew_tao173.riddles.org.uk]

env.1 manpage will need changes, want me to add those?

[kevans]

In D21481#509711, @andrew_tao173.riddles.org.uk wrote:

env.1 manpage will need changes, want me to add those?

Yes, please. =)

[andrew_tao173.riddles.org.uk]

Rebase and add docs.

(In passing, the doc changes for env(1) also add -0 to the STANDARDS and HISTORY sections, since whoever added the option apparently failed to do that.)

[bcr]

OK for the man page changes.

[andrew_tao173.riddles.org.uk]

This got brought up _again_ on IRC, but this time someone (sphex) came up with an actually interesting idea: what if there were a login.conf variable, e.g. cron.setpath, which if set caused cron(8) to respect the PATH from the login class if not overridden in the crontab file? This doesn't seem to hard to implement.

[sigsys_gmail.com]

usr.bin/login/login.conf

65

I think there should be a comment mentioning that both rc(8) and service(8) (and possibly cron(8) by default unless the new toggle is set) will override PATH with a default path (that doesn't include /usr/local/sbin and /usr/local/bin) when starting services.

It's not new that they set their own PATH, but it could be unexpected that the PATH specified there gets overridden in certain circumstances when other environment variables do get set.

[kevans]

These should have all been committed -- because I didn't want to trigger a close with an incomplete diff, I did not put in the proper tag to close it. Please do close it out if you're satisfied via "Add Action" > "Close Revision" under the diff.

[andrew_tao173.riddles.org.uk]

In D21481#516072, @kevans wrote:

These should have all been committed -- because I didn't want to trigger a close with an incomplete diff, I did not put in the proper tag to close it. Please do close it out if you're satisfied via "Add Action" > "Close Revision" under the diff.

The committed changes matched up cleanly with my local branch, so this is all good.

However, I don't see a "Close Revision" option, only "Abandon Revision" under "Revision Actions" (possibly reflecting my lack of status). If you have an option to close it out as committed, then please do use it.