Page MenuHomeFreeBSD

Add the ability have executable jail.conf
Needs ReviewPublic

Authored by crest_freebsd_rlwinm.de on Aug 13 2024, 7:34 PM.
Tags
Referenced Files
Unknown Object (File)
Fri, Oct 4, 11:13 PM
Unknown Object (File)
Oct 1 2024, 1:11 PM
Unknown Object (File)
Sep 28 2024, 2:44 AM
Unknown Object (File)
Sep 27 2024, 6:28 PM
Unknown Object (File)
Sep 25 2024, 8:51 PM
Unknown Object (File)
Sep 25 2024, 6:08 AM
Unknown Object (File)
Sep 22 2024, 11:44 PM
Unknown Object (File)
Sep 22 2024, 3:25 AM
Subscribers

Details

Reviewers
jamie
Summary

Jails often have repetitive boilerplate that can't be factored out of jail.conf using .include "<glob>"; or requires external lookups that require wrappers around jail(8) e.g. network interface creation or IP allocation, database lookups.

This patch checks if a jail.conf(5) is executable by trying to reopen its file descriptor for execution in which case it runs it as child process of the jail command and parses the child's standard output.

Test Plan
  • Write example configurations e.g. create a jail from each sub-directory in a directory of jail names.
  • Emit invalid jail.conf output.
  • Emit valid jail.conf, but exit with status != 0

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I'm a little torn on the idea itself, but here's some review

usr.sbin/jail/config.c
319

Please move side-effecty initialization like this out of the declarations

320

__DECONST(char *, cfname) gets the point across a little better imo

322

These should be converted to /* single line comments */

353

Extra spacing inside the if should be removed (just after and just before opening and closing parentheses)

372

file should be declared at the top

373

file == NULL

385

Should be initialized to 0 or so to be safe.

404

Space after the above declarations

406

Extraneous spacing again around the parentheses

Using this /etc/jail.conf:

.include "/usr/local/etc/jail[.]conf";
.include "/etc/jail.d/*.conf";
.include "/usr/local/etc/jail.d/*.conf";

Place this script into /usr/local/etc/jail.d/dirs.conf:

#!/bin/sh
set -Ceu
cd "${1%/*}"
for dir in *.d; do
        [ -d "$dir" ] || continue
        name="${dir%.d}"
        printf '%s {\n\t.include "%s/*.conf";\n}\n' "$name" "$dir"
done

To write a block like this for each ${dir}.d jail directory, deriving the jail name from the directory name (stripping the .d suffix):

jail_name { # variable references aren't allowed in jail names
    .include "$name.d/*.conf";
}
usr.sbin/jail/config.c
385

open_config() sets it to -1 before attempting to fork. It's an out parameter not an in-out parameter so imo it doesn't have to be initialised, but there is no harm in initialising it.

It would make sense for the exec.clean parameter to apply to the config execution. Bit of a chicken and egg problem there, but there's still the "-l" flag.

But I'm still just not convinced this is a good idea. It's pretty far outside the norm of how configuration files are expected to work.