newsyslog.conf: Restrict included files in default config to [!.]*.conf

[woodsb02]

Summary

newsyslog.conf: Restrict included files in default config to [!.]*.conf

The new default config will only include files from the following
directories which end with '.conf' and do not beginning with a '.'
character:

• /etc/newsyslog.conf.d/
• /usr/local/etc/newsyslog.conf.d/

This matches the syslog.conf(5) functionality, and also prevents '.sample' or
'.pkgnew' files being included. This is important for ports which install files
in /usr/local/etc/newsyslog.conf.d/ and also for pkgbase.

Test Plan

newsyslog -Nv
Review output to ensure the correct files are being included, and not others.
When testing this, also create the following new files:
/etc/newsyslog.conf.d/testing
/etc/newsyslog.conf.d/testing.conf
/etc/newsyslog.conf.d/testingconf
/etc/newsyslog.conf.d/.testing.conf

[eadler]

This is fine to me, though I could imagine some users getting confused when their config stops working.

Don't forget to bump .Dd

usr.sbin/newsyslog/newsyslog.8

271

Perhaps
"By default each file..."

[mat]

This breaks POLA in a big way, if this goes through, it will need a very large warning in the release notes.

[cem]

Hmm. I'm definitely ok rejecting the (hidden) names .sample or .pkgnew, and I think probably rejecting all hidden (dot-prefixed) names would be reasonable.

I don't see a good reason to require files be named something.conf, though. If they aren't configuration, outside of internal pkg machinations, they shouldn't be in newsyslog.conf.d/!

If we are going to require newsyslog conf files be named foo.conf, there needs to be automation that detects files with other names and at least produces a warning syslog message so admins with such files have a prayer of determining why their logs are not rotating. (This could just be a boot-time rc script.)

[eadler]

To clarify my accept: mechanically it is fine. I assumed that the social aspects were already resolved.

[dvl]

The proposed change makes newsyslog.conf consistent with syslog.conf

I like it. Looking forward to it.

[woodsb02]

I think I like cem's idea to only include files if they do not start with a "." or finish with ".pkgnew", ".sample" or ".bak" (any others?).

Can I ask for some globbing assistance please? I can't seem to find a way to match all filenames which do not end with ".sample", ".pkgnew" or ".bak"...

[eadler]

In D17086#364666, @woodsb02 wrote:

I think I like cem's idea to only include files if they do not start with a "." or finish with ".pkgnew", ".sample" or ".bak" (any others?).

If we do this, we should update other utilities that have .d directories or the like.

[bapt]

I think there is absolutely no POLA here as this is a configuration, there should be some warning in the release Release note about it and that is all imho.
Considering the option to only blacklist certain patterns: dotfiles, .pkgnew etc the problem is one would need to do more than that:
remove patch leftovers: .orig, .rej, .bak
editors leftovers file~ file.bak, etc

There are too many patterns to consider

[dvl]

In D17086#364678, @bapt wrote:

I think there is absolutely no POLA here as this is a configuration, there should be some warning in the release Release note about it and that is all imho.
Considering the option to only blacklist certain patterns: dotfiles, .pkgnew etc the problem is one would need to do more than that:
remove patch leftovers: .orig, .rej, .bak
editors leftovers file~ file.bak, etc

There are too many patterns to consider

Given that, should we instead just take *.conf and be done with it? Allow .files to be brought in.

[woodsb02]

In D17086#364678, @bapt wrote:

I think there is absolutely no POLA here as this is a configuration, there should be some warning in the release Release note about it and that is all imho.
Considering the option to only blacklist certain patterns: dotfiles, .pkgnew etc the problem is one would need to do more than that:
remove patch leftovers: .orig, .rej, .bak
editors leftovers file~ file.bak, etc

There are too many patterns to consider

The worst part is, these types of files are being executed today as valid newsyslog scripts (would this be causing double rotation?).

@mat - as the strongest voice of POLA violations, what about if we proceed with the change as is, and take a few extra steps to notify admins:

• include checks in rc.d/newsyslog which look for files in newsyslog.conf.d not ending with .conf or .sample etc, and print a warning list to the console explaining the change?
• when changing the 5 ports which install newsyslog.d files without the .conf suffix, include a pkg-install script which moves the old file to the new location (to bring in local user changes) and also prints an output to the console.

You also raised the concern about if admins were using automatic deployment tools to update newsyslog.d files (ansible, salt, puppet, chef, etc). This type of user would be exposed to both of the above. Firstly, the newsyslog.d file would be moved during pkg install, and then later when their deployment tool installs the script back in the old location, they will see commands printed to the console.

In my opinion, there is no perfect answer here. The fact seems to be the current system has issues (in that newsyslog is ingesting any .sample, .bak and .orig files and actioning them) and this should be fixed. The best we can hope for is to try and ease this transition for most people through migration tools, and detect/warn the others through console error messages.

[allanjude]

I agree with @dvl and @bapt, including only *.conf is common across other parts of FreeBSD and the wider conf.d directory idiom elsewhere. There are too many changes for a rouge file to break your configuration otherwise.