Document the per-session DNS resolution feature
Records the design decision (one dnsmasq instance per session, not per network -- avoids the NXDOMAIN-fallthrough problem a per-network design would have hit for multi-network containers) and the three real bugs found while building it (dnsmasq's --pid-file needing daemonize mode, its default privilege drop breaking $XDG_STATE_HOME access, and REFUSED AAAA answers breaking getaddrinfo()-based tools), matching the level of detail already recorded for the other networking features in this document and in CLAUDE.md's own file-by-file reference. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
This commit is contained in:
@@ -1391,6 +1391,133 @@ Source layout (all under `src/`):
|
||||
actually removed, reported as `removed stale tap-relay processes for
|
||||
'faketest-999999'`); killing the real session and re-running
|
||||
`--clean-processes` then correctly swept its own now-stale record too.
|
||||
- `network_dns.{h,cpp}` — per-session DNS resolution via `dnsmasq`: a
|
||||
container joined to a network can resolve any other container's
|
||||
`--hostname` on any network the two share, plus `host.containers.internal`
|
||||
(podman's own convention) resolving to the first `extern` network's own
|
||||
gateway, if any. `is_dnsmasq_available()` (`find_in_path("dnsmasq")`) gates
|
||||
everything here — DNS resolution is best-effort, not a hard dependency the
|
||||
way `ip`/`nsenter` are; missing `dnsmasq` degrades with a warning rather
|
||||
than failing `-r/--run`. **One `dnsmasq` instance per session, deliberately
|
||||
not per network**: an earlier per-network design (mirroring how the uplink
|
||||
provisions one persistent process per network) was rejected before being
|
||||
built, once a real correctness gap was worked out — a container joined to
|
||||
two networks would list two `nameserver` lines in `/etc/resolv.conf`, and
|
||||
standard stub resolvers (glibc/musl/busybox) don't fall through to the
|
||||
next nameserver on NXDOMAIN, only on timeout, so a name that exists only
|
||||
on the *second* network would silently fail to resolve. Running one
|
||||
instance per session instead — entered into the *container's own* network
|
||||
namespace (`resolve_namespace_pid()`, `sandbox_process.h`) and bound to
|
||||
`127.0.0.1:53` inside it, so `/etc/resolv.conf` is always just `nameserver
|
||||
127.0.0.1` regardless of which/how many networks were joined — sidesteps
|
||||
the whole problem: there's only ever one nameserver to ask, and it already
|
||||
knows about every network that specific container joined via dnsmasq's own
|
||||
repeatable `--hostsdir=<dir>` (inotify-based, no reload signal needed).
|
||||
`dns_hosts_dir(network_name)` is `$XDG_STATE_HOME/slocker-lite/dns-hosts/
|
||||
<sanitized-network-name>` — one shared directory per network;
|
||||
`record_dns_host()`/`remove_dns_host_record()` write/remove one
|
||||
`<sanitized-container-name>-<pid>` file per (network, session) into it (a
|
||||
plain `/etc/hosts`-syntax line, `<ip> <hostname>`, only written when
|
||||
`--hostname` was actually given), so sibling sessions on the same network
|
||||
can discover each other — and, since a session's own record lands in the
|
||||
same directory its own resolver watches, self-resolution works for free,
|
||||
no special-casing needed. `start_dns_resolver()` builds the
|
||||
`nsenter --net=/proc/<ns_pid>/ns/net -- dnsmasq ...` argv (one `--hostsdir`
|
||||
per joined network) and forks/execs it directly (no `process.h` helper
|
||||
fits: `run_process()`/`run_process_foreground()` both block until exit,
|
||||
wrong for a long-running daemon, and `close_inherited_fds()`-style fd
|
||||
hygiene isn't needed here the way `network_tap_relay.cpp`'s relay needs it,
|
||||
since this process *does* `exec()`, so `O_CLOEXEC` just works normally).
|
||||
**Three real bugs found via direct testing while building this, not
|
||||
assumed**, each confirmed by isolating the change and re-testing:
|
||||
1. dnsmasq only writes `--pid-file` while actually daemonizing —
|
||||
`-d`/`--no-daemon` (tried first, for a simpler "the forked pid is the
|
||||
real pid" model) suppresses it entirely, confirmed directly: dnsmasq
|
||||
started and successfully read the hosts file (visible in its own log
|
||||
output) but the pid-file this function polls for (same bounded-poll
|
||||
shape as `network_join.cpp`'s `wait_for_isolated_net_namespace()`)
|
||||
never appeared. Fixed by letting dnsmasq daemonize normally — the
|
||||
forked/exec'd process is then only the *intermediate* one (reaped
|
||||
immediately, not tracked), and the real, final daemon pid is read back
|
||||
from the pid-file itself once it appears.
|
||||
2. dnsmasq drops root privileges to an unprivileged user by default, which
|
||||
then couldn't read anything under `$XDG_STATE_HOME` at all (typically
|
||||
`/root/.local/state/slocker-lite/...`, mode `0700` — a non-root user
|
||||
can't even traverse into `/root`) — every query came back `REFUSED`,
|
||||
traced to `bad dynamic directory ...: Permission denied` in dnsmasq's
|
||||
own log. Fixed with an explicit `--user=root --group=root`, matching
|
||||
this project's existing root-only networking model (`bwrap.cpp` never
|
||||
requests `--unshare-user` when already root, for the same "no privilege
|
||||
drop needed/wanted here" reasoning) — tracked as a security follow-up in
|
||||
`TODO.md` (run it as a low-privilege user instead, with the relevant
|
||||
state relocated somewhere that user can reach).
|
||||
3. an AAAA query for a name with only an A record (every record here is
|
||||
IPv4-only, matching `JoinedNetwork::container_ip`'s own existing scope)
|
||||
came back `REFUSED` rather than a clean "no data" answer — confirmed to
|
||||
break `ping <name>` even though the exact same name's A record had just
|
||||
resolved correctly via `nslookup` moments earlier, since `ping` (like
|
||||
most `getaddrinfo()`-based tools) queries both A and AAAA together and
|
||||
treats `REFUSED` on either as a hard failure for the whole lookup, not
|
||||
merely "no IPv6 available." Fixed with `--filter-AAAA` (turns an AAAA
|
||||
answer into a clean empty one instead). `host.containers.internal`
|
||||
needed a *second*, related fix: serving it via dnsmasq's own
|
||||
`--address=/name/ip` option kept returning `REFUSED` for AAAA even with
|
||||
`--filter-AAAA` given — confirmed `--address` records aren't treated the
|
||||
same internally as ordinary hosts-file entries — so it's instead served
|
||||
via a small, session-private `--addn-hosts=<file>` (plain hosts-file
|
||||
syntax, generated once per session, removed by `stop_dns_resolver()`
|
||||
alongside the process itself) exactly like every other record here.
|
||||
`stop_dns_resolver()` (`SIGTERM` + `waitpid()`, plus removing the
|
||||
`host.containers.internal` file if one was generated) and
|
||||
`remove_dns_resolver_record()` (removes the pid-file — the same file
|
||||
dnsmasq itself wrote, so no separate "record" write step was ever needed,
|
||||
just a removal one) are called as a pair from `run_container()`
|
||||
(`commands.cpp`) after `run_bwrap()` returns, the same two-calls shape
|
||||
`stop_tap_relay()`/`remove_tap_relay_record()` already use — **a real gap
|
||||
caught by testing this exact pairing, not assumed**: an earlier version
|
||||
only called `stop_dns_resolver()`, and since dnsmasq's own pid-file was
|
||||
never separately removed, `--clean-processes` (harmlessly, but not
|
||||
cleanly) picked up every finished session's own leftover record on its
|
||||
next run instead of finding nothing to sweep. `clean_stale_dns_resolvers()`
|
||||
covers three independent crash-orphan sweeps — the resolver process
|
||||
itself (`dns-resolvers/<container>-<pid>`, `SIGKILL`, same
|
||||
`list_sessions()`-cross-reference staleness check every other sweep in
|
||||
this project uses), each network's own per-session hosts record
|
||||
(`dns-hosts/<network>/<container>-<pid>`, just deleted — dnsmasq's own
|
||||
inotify watch, wherever some other still-running session's resolver is
|
||||
watching that directory, notices the removal on its own), and the
|
||||
`host.containers.internal` file (`dns-internal-hosts/<container>-<pid>`,
|
||||
likewise just deleted) — called from `clean_processes_command()`
|
||||
(`commands.cpp`) alongside the three existing sweeps. `--no-dns`
|
||||
(`cli_args.{h,cpp}`, long-option only, same precedent as `--no-veth`) opts
|
||||
out even when `dnsmasq` is available. `build_bwrap_args()`/`run_bwrap()`
|
||||
(`bwrap.{h,cpp}`) gained a `inject_dns_resolv_conf` bool, resolved once in
|
||||
`run_container()` from `!network_specs.empty() && is_dnsmasq_available() &&
|
||||
!no_dns_flag` (keeping the policy decision out of `bwrap.cpp`, the same
|
||||
pattern `NamespaceConfig` already uses) — when true, a single static,
|
||||
idempotently-generated file (`ensure_generated_resolv_conf()`, content
|
||||
always just `nameserver 127.0.0.1\n`, since every session's resolver binds
|
||||
there regardless of which networks it joined) is `--ro-bind`-mounted over
|
||||
`/etc/resolv.conf`, which `build_bwrap_args()` otherwise never touches at
|
||||
all (confirmed: whatever's baked into the OCI image is what's live in the
|
||||
sandbox by default, no prior bind-mount or generation existed here). No
|
||||
special-casing needed for `-x/--exec` (already joins the same net
|
||||
namespace as the original session when one was requested, so it shares the
|
||||
same loopback and thus the same running resolver for free) or
|
||||
`-D/--daemonize` (spawned/stopped by the exact same callback/cleanup path
|
||||
as any other run). `-t/--test` exercises the full create/answer/teardown
|
||||
cycle in isolation (root-only skip, same as the persistent-netns/tap-relay
|
||||
tests): a throwaway network namespace stands in for a real session's own
|
||||
(same technique `test_tap_relay()` already uses), a hand-rolled minimal
|
||||
DNS query (`build_dns_a_query()`/`query_dns_a_record()`, `self_test.cpp`
|
||||
— real UDP wire format, not just "the process started") confirms the
|
||||
resolver actually answers a hand-written hosts record correctly.
|
||||
**Verified end-to-end** both on this dev machine and on the real Android
|
||||
target device: two containers on a shared network resolve each other
|
||||
(including self-resolution) and can `ping` by name; a container joined to
|
||||
both an `intern` and an `extern` network resolves both its `intern` peer
|
||||
and `host.containers.internal` simultaneously — the specific scenario the
|
||||
per-network-instance design would have broken.
|
||||
- `session_cgroup.{h,cpp}` — gives `--kill` (`kill_session.{h,cpp}`, see
|
||||
below) a reliable way to find every process a session ever started, however
|
||||
deeply forked/daemonized/reparented, by putting it in a dedicated cgroup v2
|
||||
@@ -1892,7 +2019,7 @@ Build directory is `buildDir/` (already configured).
|
||||
`-l/--list-images`, `-i/--inspect`, `-x/--exec`, `--kill`, `--no-nsenter`, `-D/--daemonize`,
|
||||
`--user`, `--group`, `--hostname`, `--env`, `--env-file`, `-v/--volume`, `--list-volumes`, `--delete-volume`,
|
||||
`--delete-volume-full`, `-n/--network`, `--extern`, `--intern`, `--subnet`, `--no-ipv6`, `--subnet6`,
|
||||
`--no-veth`, `--list-networks`, `--delete-network`, `-p/--port-forward`, `--list-processes`, `--clean-processes`,
|
||||
`--no-veth`, `--list-networks`, `--delete-network`, `-p/--port-forward`, `--no-dns`, `--list-processes`, `--clean-processes`,
|
||||
`-w/--write-config`, `-t/--test`, `--log-level`, `-h/--help`, `-V/--version`)
|
||||
- Run tests: `meson test -C buildDir`
|
||||
|
||||
|
||||
Reference in New Issue
Block a user