Use the last instead of the first match (per jail) to enable dynamic configuration.

Make dynamic (aka executable) jail.conf opt-in via the already parsed partial jail.conf.

In D49158#1121383, @dave_freedave.net wrote:

I vastly under appreciated that folks rely upon the ng_eiface not moving with the struct ifnet. Probably because I've been using them in jails for over a decade and only recently noticed myself.

I encountered that situation because I was exploring the ordering of ng_eiface creation, move to jail, and rename. The idea being I could have the same interface names in jails if I waited until after the move to rename them and therefore use the same config for dhcpcd. I was then surprised at what happened when an interface was renamed in a jail and then the jail was shut down. I then went off with the wrong assumption that it was missed, moving ng_eiface to the new vnet.

In D46284#1299021, @crest_freebsd_rlwinm.de wrote:

In D46284#1298773, @kevans wrote:

I had hoped to attend the jail user call today to be able to discuss this, but the Discord event has it an hour off and I didn't discover the authoritative source @ callfortesting.org until I was wondering why nobody was showing up. *deep sigh*

I don't think we're comfortable, as a project, to enable this for all users of jail(8) by default without an additional flag. I appreciate that you want to do stuff like this with existing jail scripts but this is a huge POLA violation (even assuming proper communication across a major branch update) with security implications, and I don't think enabling maybe-executable config scripts is a pattern that we really want propagating.

There was some discussion out-of-band after a concerned user reached out about this, and it was pointed out to me that automountd does the same thing, so I've pitched a review to try and neuter that a bit because that's terrifying: D56680.

The jail.conf(5) format already defines hooks that by design execute as root on the host (exec.prepare/created/prestart/poststart/prestop/poststop/release). Having any untrusted jail.conf(5) on the system is a game over scenario similar to having a malicous /etc/crontab or rc.d script installed. Moving the attack surface a few milliseconds forward from the exec.prepare (or exec.prestop when removing a jail) stage to the config parsing stage doesn't increase the attack surface in a meaningful way.

In D46284#1299031, @imp wrote:

In D46284#1299021, @crest_freebsd_rlwinm.de wrote:

@kevans: Can you think of a realistic situation where someone will have their jail configuration unintentionally executable? I don't think chmod -R 777 /etc is a supported configuration. Are we forced to support FAT32 as root file system on any strange platform or something like that?

That's the wrong question. For security related things, you have to default to 'fail safe' and this feature fails to meet that criteria.

In D46284#1299021, @crest_freebsd_rlwinm.de wrote:

@kevans: Can you think of a realistic situation where someone will have their jail configuration unintentionally executable? I don't think chmod -R 777 /etc is a supported configuration. Are we forced to support FAT32 as root file system on any strange platform or something like that?

In D46284#1298773, @kevans wrote:

I had hoped to attend the jail user call today to be able to discuss this, but the Discord event has it an hour off and I didn't discover the authoritative source @ callfortesting.org until I was wondering why nobody was showing up. *deep sigh*

I don't think we're comfortable, as a project, to enable this for all users of jail(8) by default without an additional flag. I appreciate that you want to do stuff like this with existing jail scripts but this is a huge POLA violation (even assuming proper communication across a major branch update) with security implications, and I don't think enabling maybe-executable config scripts is a pattern that we really want propagating.

There was some discussion out-of-band after a concerned user reached out about this, and it was pointed out to me that automountd does the same thing, so I've pitched a review to try and neuter that a bit because that's terrifying: D56680.

I had hoped to attend the jail user call today to be able to discuss this, but the Discord event has it an hour off and I didn't discover the authoritative source @ callfortesting.org until I was wondering why nobody was showing up. *deep sigh*

New attempt with git format-patch -U999999 *sigh*.

Since Phabricator ignores my attempts to update this review to modify multiple files with the diff I created a GitHub pull request at https://github.com/freebsd/freebsd-src/pull/2164 .

Try to copy and paste the patch instead of uploading as a file. Sorry for the noise.

For some reason Phabricator doesn't register that I edited two additional files in the latest patch I uploaded and the raw diff doesn't include them either?!?

Second attempt to update the other source files too.

Use the parser's existing stack of nested jails instead of incorrectly assuming the last jail parsed is also the current jail. This incorrect assumption is not true after one or more jail blocks have been closed and no new jail block has been opened.

As discussed putting this feature behind a paranoia flag to not cause issues for anyone that just happens to have an accidentally executable jail.conf(5) makes it effectively useless.

Reformatted the code to 80 columns.

Reformat the code to 80 columns.

Switch to single line comments inside the open_file() function.

Use single line comments in open_file() where appropriate.

Replace fexecve(2) with execve(2) since fexecve(2) doesn't work with (shell) scripts unless /dev/fd is mounted with the non-standard "nodup" option.

Use normal pointer syntax for the pid pointer instead of pid_t pid[static 1].

Thank you!

• Use jexec_args variable with all parameters in both getopt calls.

• Add missing error handling for putenv(3).

In D54660#1249395, @jamie wrote:

Why does it matter that putenv(3) doesn't create a copy?

• Avoid memory allocations.
• Parse -e twice. Once to verify correctness. Again to set the variable.

Why does it matter that putenv(3) doesn't create a copy?

Align putenv_copy() function declaration for consistency.

Since putenv(3) does not create a copy, I had to implement a function to emulate the old behavior, so now setenv(3) is used after parsing the environment variable.

Document new -x flag in jail(8).

I had considered that the -l (exec clean) flag should be considered, but decided it really only makes sense for keeping the jail environment clean.

Put the executable jail.conf(5) behind the -x option (as chicken bit).

I vastly under appreciated that folks rely upon the ng_eiface not moving with the struct ifnet. Probably because I've been using them in jails for over a decade and only recently noticed myself.

In D49158#1121374, @zec wrote:

Your obsession with getting rid of the "flawed" if_vmove() is noted, but for the sake of other people who may have a different view, and who have applications relying on this very concept for 20+ years, please do not take that route.

In D49158#1121338, @glebius wrote:

I do not like the plan. The picture drawn shows that a netgraph node in one vnet is connected to a node in a different vnet. This is basically a violation of the idea of vnet. Virtualized stacks should communicate with each other via network protocols, not kernel pointers. The only legal exclusion is epair(4).