Enable jail(8) to parse all config files

[meka_tilda.center]

Summary

For a jail, following path are considered default configuration files:

• /etc/jail.conf
• /etc/jail.conf.d/<jail>.conf
• /etc/jail.<jail>.conf

Those files are now always parsed. If user specifies "-f /path/to/file", it is added to the list of configuration files. As wildcard jails (aka global parameters) can be defined in any of the configuration files, jail(8) has to parse them all to retain consistent behaviour when providing "-f" option.

To enable parsing of all configuration files, -F has to be added to jail_flags. To activate it in rc.d/jail, one has to add jail_config_file in rc.conf. The value of jail_config_file can be "single" or "multiple".

[meka_tilda.center]

Sort the includes.

[ihor_antonovs.family]

libexec/rc/rc.d/jail

458

I need to point out obvious conflicts with a similar change by @antranigv_freebsd.am https://reviews.freebsd.org/D38826

[github_evilham.com]

usr.sbin/jail/config.c

167

wouldn't cfname=='-' hit this line when there the glob doesn't find anything?

[meka_tilda.center]

usr.sbin/jail/config.c

167

I see what you mean, and yes, you are right!

[github_evilham.com]

usr.sbin/jail/config.c

167

I see, in that case, maybe this if (g.gl_pathc == 0) should go both after the loop and after the if at current L179, else this is a regression when instructing jail to read from stdin, and not having any other config files ^^.

[ihor_antonovs.family]

@meka_tilda.center please re-upload the diff with context (i.e. generate it with git diff -U9999) such that reviewers can see more unchanged lines

[ihor_antonovs.family]

usr.sbin/jail/config.c

129

nit: for consistency, and to avoid potential bugs use {} with if even when there is a single statement.

129

check_glob function name is a bit ambiguous: something like check_glob_err would be better

149

consider zero-initializing structs for safety, i.e. glob_t g = {0};

155

consider moving magic strings to either #DEFINE or static const to allow future re-use and avoid typos and duplication.

161

tentative: post-increment seems to be a more popular option in the tree, i.e. i++

164

same: consider explicit value comparison for non-boolean types,

166

consider doing explicit value comparison, i.e.:

if (yyparse() != 0 || yynerrs != 0) {

173

if I am not mistaken, this can lead to a situation where a file is parsed twice. if (op & JF_CONF_ALL) { condition does not exclude this one, so both can potentially be true, and cfname can have the last value set by the for loop a few lines above.

175

hint: this memset can be avoided by zero-initializing the struct at declaration stite, see my comment above

179

same: consider explicit value comparison for non-boolean types,

181

same: consider explicit value comparison

189

same: consider explicit value comparison

usr.sbin/jail/jail.c

299

nit:
negation of integer is confusing, docf is used as a boolean, but has integer type. IMHO converting docf to boolean and then using

doc = Rflag !=0;

or at least we can be more explicit:

doc = Rflag > 0 ? 0 : 1;

[ihor_antonovs.family]

usr.sbin/jail/jailp.h

69

extra tab, ruins the alignment

[meka_tilda.center]

Thank you for detailed review. I omitted the change for the style, as almost all of them were already present before my change, like not having {} on if. I personally don't have preferences for style so I'll do whatever maintainer thinks is right. I would like to keep style changes out of this patch, if possible.

[meka_tilda.center]

Save original value of cfname and restore it after handling glob config.

[meka_tilda.center]

libexec/rc/rc.d/jail

458

This patch supersedes the one by Antranig, so we both agreed to work on this one

[crest_freebsd_rlwinm.de]

The jail(8) man page needs to be updated to reflect the new -F flag.

usr.sbin/jail/config.c

37

Why whas the order of includes changed?

[meka_tilda.center]

Add FILES section to jail(8) and add -F option to usage() function.

[jamie]

I just created D40188, which is my take on solving this issue. It goes the include-file direction, and doesn't require and changes to rc.conf, since jail.conf is where such changes are made. The jail(8) command line also remains untouched.

The diff looks bigger than it really is, since much of the change involved changing the lex and yacc sources to use their respective re-entrant modes.

As part of this, I wanted to handle including a file within a jail definition, which meant I had to either not allow jail definitions in such files themselves, or I had to support nested definitions. I chose the latter, which I should have thought of in the first place. Specifying jail "bar" inside the specification of jail "foo" creates jail "foo.bar".

[ihor_antonovs.family]

IMHO this is superseded by https://reviews.freebsd.org/D40188