The init fence closes a port it cannot redirect #18

Merged

marcos-mendez merged 5 commits from fix/fence-no-silent-skip into master

2026-09-29 01:39:36 -03:00

marcos-mendez commented

2026-09-28 06:06:46 -03:00

(Migrated from github.com)

Closes #17.

The problem

iptables_add_redirect skipped the REDIRECT rule for an address family
whose nat table cannot be used, warned, and carried on. bin/turnkey-init-fence
then started the mini server and reported success, and the unit sat
active, while the ports the fence was asked to stand in front of reached
the application over that family.

In an LXC container whose host has not loaded ip6table_nat that is the
normal path, not an edge case, and the warning lands in the journal of a
first boot nobody is watching. A mitigation that declines and reports
success is worse than one that is not there, because the rest of the system
is written as though it worked.

Why the skip is not simply reverted

#3 put it there deliberately. Before it, the fence exited 3 on the first
family it could not install a rule for: the unit failed, the mini server
never started and 29tagid then failed reloading it. Going back to that
trades one wrong answer for another, and a louder warning would still leave
the port open.

The container case #3 was serving is legitimate. What was wrong was the
conclusion: if the port cannot be redirected to the fence, the answer is
that the application behind it must not answer either, not that the fence
should shrug.

The change

fence_close_port refuses the port in the filter table, which a container
does have, inserted at the head of the INPUT chain so that it precedes the
ACCEPT rules webmin-fw installs for the same ports. Appending would have
put it after them and matched nothing. The rule is
! -i lo ... -j REJECT --reject-with tcp-reset: loopback is spared, as a
REDIRECT in PREROUTING always spared it, and the reset is because an IPv6
client given the default ICMP error waits for its timeout instead of
failing (measured, below).

The head of the chain is the chain as it stands when the fence starts. A
restore of a saved firewall flushes the tables it names, and with them the
refusal and the REDIRECT rules alike. The unit is ordered after the
services that restore one at boot (netfilter-persistent, Webmin's
webmin-iptables and webmin-ip6tables); a restore while the fence is up
still unfences every port, and the comment and changelog say so.

What the fence promises then holds either way:

the application behind a fenced port does not answer;
the fence page stays reachable on HTTP_FENCE_PORT and
HTTPS_FENCE_PORT, which keeps #3's outcome, including the 29tagid
reload;
a port that can be neither redirected nor refused is fatal, with a <3>
message, because then nothing at all is fencing it. That is the only case
left where the unit fails, and it is a case where failing is the truth.
Failing does not mean closed: stop-post then removes every rule the
fence installed and the application answers on every port. It
does not cascade the way the pre-#3 failure did: 29tagid has asked
systemctl is-active before reloading since 2.3.6+keel1, so a failed fence
is skipped rather than fatal to the hook.

fence_open_port undoes it, and iptables_delete_redirect calls it for both
families, so stop and the stop-post cleanup after a failed run leave no
port closed behind them. That keeps the existing policy: a failed fence
cleans up rather than locking the appliance out, and the port is open again
because the fence is gone, not because it pretended.

The message moved from WARNING: to the <4> prefix the rest of the file
uses for a journal priority.

Tests

31 in the file, 116 in the suite: the refusal for one family and for both,
the insertion at the head of the chain, the removal loop, and the fatal path
twice, through the library and through the script. The second one matters on
its own: a library function that returns non-zero as an error only stops its
caller if the caller runs under errexit, and bats run turns errexit off, so
a test that only drives the library cannot see it (docs/traps.md, "A bats
suite cannot see a library that kills its caller").

One assertion in the pre-existing "both nat tables" test was a bare !
before a later command, so it asserted nothing; it is now a count that does.
Keel-Linux/inithooks#16 is rewriting the other negations in this file, so
expect a conflict in tests/test-init-fence.bats between the two.

tests/test-init-fence-netfilter.bats, 8 tests, asks the real netfilter:
tests/netns-sandbox runs the library against real iptables and
ip6tables in a network namespace of its own, with a client namespace on
the other end of a veth pair, and connects over IPv6, IPv4 and loopback.
The only stand-in is the answer "no nat table" for one family. Against the
branch as first reviewed it fails 4, which are the review's findings plus
the silent IPv6 refusal.

kcov (first version): lib/init-fence.sh 78 of 79 lines, 98.73 percent,
total 99.57; now 83 of 84, 98.81, total 99.58, 125 bats. Over
the 98 gate and decision 0003's 95 percent bar. The uncovered line is line
154, the first line of the multi-line simplehttpd.py invocation that kcov
attributes to a later line; it was uncovered before this branch and is not
part of it. Every line this branch adds is covered.

What this does not do

Nothing is built or published and no build lock was taken, so this has not
run on a booted container; the rules are measured in a network namespace,
with the nf_tables backend where the appliance uses iptables-legacy.

2.3.6+keel6 is this one's; #14 has moved to keel7 and lands after it.

Closes #17. ## The problem `iptables_add_redirect` skipped the REDIRECT rule for an address family whose nat table cannot be used, warned, and carried on. `bin/turnkey-init-fence` then started the mini server and reported success, and the unit sat `active`, while the ports the fence was asked to stand in front of reached the application over that family. In an LXC container whose host has not loaded `ip6table_nat` that is the normal path, not an edge case, and the warning lands in the journal of a first boot nobody is watching. A mitigation that declines and reports success is worse than one that is not there, because the rest of the system is written as though it worked. ## Why the skip is not simply reverted #3 put it there deliberately. Before it, the fence exited 3 on the first family it could not install a rule for: the unit failed, the mini server never started and `29tagid` then failed reloading it. Going back to that trades one wrong answer for another, and a louder warning would still leave the port open. The container case #3 was serving is legitimate. What was wrong was the conclusion: if the port cannot be redirected to the fence, the answer is that the application behind it must not answer either, not that the fence should shrug. ## The change `fence_close_port` refuses the port in the filter table, which a container does have, inserted at the head of the INPUT chain so that it precedes the ACCEPT rules `webmin-fw` installs for the same ports. Appending would have put it after them and matched nothing. The rule is `! -i lo ... -j REJECT --reject-with tcp-reset`: loopback is spared, as a REDIRECT in PREROUTING always spared it, and the reset is because an IPv6 client given the default ICMP error waits for its timeout instead of failing (measured, below). The head of the chain is the chain as it stands when the fence starts. A restore of a saved firewall flushes the tables it names, and with them the refusal and the REDIRECT rules alike. The unit is ordered after the services that restore one at boot (`netfilter-persistent`, Webmin's `webmin-iptables` and `webmin-ip6tables`); a restore while the fence is up still unfences every port, and the comment and changelog say so. What the fence promises then holds either way: - the application behind a fenced port does not answer; - the fence page stays reachable on `HTTP_FENCE_PORT` and `HTTPS_FENCE_PORT`, which keeps #3's outcome, including the `29tagid` reload; - a port that can be neither redirected nor refused is fatal, with a `<3>` message, because then nothing at all is fencing it. That is the only case left where the unit fails, and it is a case where failing is the truth. Failing does not mean closed: `stop-post` then removes every rule the fence installed and the application answers on every port. It does not cascade the way the pre-#3 failure did: `29tagid` has asked `systemctl is-active` before reloading since 2.3.6+keel1, so a failed fence is skipped rather than fatal to the hook. `fence_open_port` undoes it, and `iptables_delete_redirect` calls it for both families, so `stop` and the `stop-post` cleanup after a failed run leave no port closed behind them. That keeps the existing policy: a failed fence cleans up rather than locking the appliance out, and the port is open again because the fence is gone, not because it pretended. The message moved from `WARNING: ` to the `<4>` prefix the rest of the file uses for a journal priority. ## Tests 31 in the file, 116 in the suite: the refusal for one family and for both, the insertion at the head of the chain, the removal loop, and the fatal path twice, through the library and through the script. The second one matters on its own: a library function that returns non-zero as an error only stops its caller if the caller runs under errexit, and `bats run` turns errexit off, so a test that only drives the library cannot see it (`docs/traps.md`, "A bats suite cannot see a library that kills its caller"). One assertion in the pre-existing "both nat tables" test was a bare `!` before a later command, so it asserted nothing; it is now a count that does. Keel-Linux/inithooks#16 is rewriting the other negations in this file, so expect a conflict in `tests/test-init-fence.bats` between the two. `tests/test-init-fence-netfilter.bats`, 8 tests, asks the real netfilter: `tests/netns-sandbox` runs the library against real `iptables` and `ip6tables` in a network namespace of its own, with a client namespace on the other end of a veth pair, and connects over IPv6, IPv4 and loopback. The only stand-in is the answer "no nat table" for one family. Against the branch as first reviewed it fails 4, which are the review's findings plus the silent IPv6 refusal. kcov (first version): `lib/init-fence.sh` 78 of 79 lines, 98.73 percent, total 99.57; now 83 of 84, 98.81, total 99.58, 125 bats. Over the 98 gate and decision 0003's 95 percent bar. The uncovered line is line 154, the first line of the multi-line `simplehttpd.py` invocation that kcov attributes to a later line; it was uncovered before this branch and is not part of it. Every line this branch adds is covered. ## What this does not do Nothing is built or published and no build lock was taken, so this has not run on a booted container; the rules are measured in a network namespace, with the nf_tables backend where the appliance uses iptables-legacy. `2.3.6+keel6` is this one's; #14 has moved to `keel7` and lands after it.

marcos-mendez commented

2026-09-28 07:04:20 -03:00

(Migrated from github.com)

Reviewed alongside Keel-Linux/common#12. Coverage reproduced exactly: 116
bats, lib/init-fence.sh 78/79 at 98.73, total 99.57, and the one uncovered
line is 154 and is uncovered on the parent commit too. The fatal path tested
twice, once through the library and once through the script, is the right
call and the only one that proves it — docs/traps.md, "A bats suite cannot
see a library that kills its caller". I also confirmed stop-post reaches
iptables_redirect stop on a failed start, so nothing this branch inserts
survives a failed run.

HIGH — lib/init-fence.sh:41 and the comment at :29-31: position 1 in the
live chain is not where the appliance's own rules come from. common's
webmin-fw does not append to a running chain; it writes
/etc/iptables/rules.v4 and rules.v6 as whole tables, *filter through
COMMIT, meant to be applied with a restore. A restore flushes the chain,
so it takes out this branch's REJECTs and the fence's own ACCEPTs together
and the insertion position buys nothing against it. On the appliance I could
inspect, nothing applies those files at boot — INPUT is empty with policy
ACCEPT, so the insertion is uncontested there — but Webmin's Firewall module
applies them on demand, and an administrator who touches it while the fence
is up silently unfences everything. The unit orders itself after
iptables.service, ip6tables.service, nftables.service,
firewalld.service and ipset.service, none of which is what applies those
two files. Either order against the thing that does, or narrow the comment
to say the position is about the live chain only.

MEDIUM — lib/init-fence.sh:91: the fatal path ends unfenced, not closed.
return 1 reaches bin/turnkey-init-fence under -eu, the unit fails,
ExecStopPost runs stop-post, SERVICE_RESULT is not success and
iptables_redirect stop opens every port again — including the ones that
were successfully closed a moment earlier. So the outcome of "a port that can
be neither redirected nor refused is fatal" is: application answering on all
of them, no fence, and a unit that says so. That is the right trade against
the silent version and it is consistent with the cleanup policy, but the
changelog and the comment both read as though failing means closed. One
sentence in each saying which it is.

MEDIUM — lib/init-fence.sh:41: the refusal has no interface match. A
REDIRECT in PREROUTING never applied to locally originated traffic; a REJECT
in INPUT does. I grepped firstboot.d, everyboot.d and every Keel
appliance overlay and nothing talks to a fenced port over loopback today, so
this costs nothing now — but it is a real behaviour change for a first-boot
hook that one day does, and it will present as a hook that works everywhere
except where the fence had to fall back. -i lo -j ACCEPT ahead of it, or
! -i lo on the rule, is one line and keeps the two paths equivalent.

LOW — lib/init-fence.sh:39: the message precedes the rule.
fence_close_port announces the refusal and then tries to install it. On the
failing path the journal carries the claim and then its retraction, and on
the caller's return 1 the claim is the last positive thing said about that
port. Move the echo below the insert.

LOW — lib/init-fence.sh:54-57: fence_open_port cannot tell "nothing
left to remove" from "cannot ask". Deliberate, and stopping does have to
succeed, so this is right — but the comment says "tolerates a family that
cannot be asked at all" without saying that the two are indistinguishable to
the caller. Half a sentence.

LOW — landing order. This branch and #16 collide on exactly one hunk of
tests/test-init-fence.bats: the bare ! before
printf ... | grep -q WARNING. This branch's replacement is a grep -c
that is compared, which is what #16 is after and does not need run ! to
get it, so this should land first and #16 drop that hunk — it still has four
other bare negations in the same file that this branch does not touch. Same
answer for 2.3.6+keel6: this one first, #14 bumps.

The reasoning for not reverting #3 is correct, and closing what cannot be
redirected is the right shape. The findings are about what the insertion
position actually guarantees, what the fatal path leaves behind, and saying
both out loud where the next reader will look.

Warning

Reviewed alongside Keel-Linux/common#12. Coverage reproduced exactly: 116 bats, `lib/init-fence.sh` 78/79 at 98.73, total 99.57, and the one uncovered line is 154 and is uncovered on the parent commit too. The fatal path tested twice, once through the library and once through the script, is the right call and the only one that proves it — `docs/traps.md`, "A bats suite cannot see a library that kills its caller". I also confirmed `stop-post` reaches `iptables_redirect stop` on a failed start, so nothing this branch inserts survives a failed run. --- **HIGH — `lib/init-fence.sh:41` and the comment at :29-31: position 1 in the live chain is not where the appliance's own rules come from.** `common`'s `webmin-fw` does not append to a running chain; it writes `/etc/iptables/rules.v4` and `rules.v6` as whole tables, `*filter` through `COMMIT`, meant to be applied with a restore. A restore flushes the chain, so it takes out this branch's REJECTs and the fence's own ACCEPTs together and the insertion position buys nothing against it. On the appliance I could inspect, nothing applies those files at boot — INPUT is empty with policy ACCEPT, so the insertion is uncontested there — but Webmin's Firewall module applies them on demand, and an administrator who touches it while the fence is up silently unfences everything. The unit orders itself after `iptables.service`, `ip6tables.service`, `nftables.service`, `firewalld.service` and `ipset.service`, none of which is what applies those two files. Either order against the thing that does, or narrow the comment to say the position is about the live chain only. **MEDIUM — `lib/init-fence.sh:91`: the fatal path ends unfenced, not closed.** `return 1` reaches `bin/turnkey-init-fence` under `-eu`, the unit fails, `ExecStopPost` runs `stop-post`, `SERVICE_RESULT` is not success and `iptables_redirect stop` opens every port again — including the ones that were successfully closed a moment earlier. So the outcome of "a port that can be neither redirected nor refused is fatal" is: application answering on all of them, no fence, and a unit that says so. That is the right trade against the silent version and it is consistent with the cleanup policy, but the changelog and the comment both read as though failing means closed. One sentence in each saying which it is. **MEDIUM — `lib/init-fence.sh:41`: the refusal has no interface match.** A REDIRECT in PREROUTING never applied to locally originated traffic; a REJECT in INPUT does. I grepped `firstboot.d`, `everyboot.d` and every Keel appliance overlay and nothing talks to a fenced port over loopback today, so this costs nothing now — but it is a real behaviour change for a first-boot hook that one day does, and it will present as a hook that works everywhere except where the fence had to fall back. `-i lo -j ACCEPT` ahead of it, or `! -i lo` on the rule, is one line and keeps the two paths equivalent. **LOW — `lib/init-fence.sh:39`: the message precedes the rule.** `fence_close_port` announces the refusal and then tries to install it. On the failing path the journal carries the claim and then its retraction, and on the caller's `return 1` the claim is the last positive thing said about that port. Move the echo below the insert. **LOW — `lib/init-fence.sh:54-57`: `fence_open_port` cannot tell "nothing left to remove" from "cannot ask".** Deliberate, and stopping does have to succeed, so this is right — but the comment says "tolerates a family that cannot be asked at all" without saying that the two are indistinguishable to the caller. Half a sentence. **LOW — landing order.** This branch and #16 collide on exactly one hunk of `tests/test-init-fence.bats`: the bare `!` before `printf ... | grep -q WARNING`. This branch's replacement is a `grep -c` that is compared, which is what #16 is after and does not need `run !` to get it, so this should land first and #16 drop that hunk — it still has four other bare negations in the same file that this branch does not touch. Same answer for `2.3.6+keel6`: this one first, #14 bumps. --- The reasoning for not reverting #3 is correct, and closing what cannot be redirected is the right shape. The findings are about what the insertion position actually guarantees, what the fatal path leaves behind, and saying both out loud where the next reader will look. **Warning**

marcos-mendez commented

2026-09-29 01:30:06 -03:00

(Migrated from github.com)

Review addressed in 36663c5 and 7cc4139, then master merged in (d24ca3e). The fence is now measured against real iptables in a network namespace. The unit starts after the services that restore a saved firewall at boot, and the comment says an on-demand restore unfences every port. The fatal path is documented as open. The refusal skips loopback and is now a tcp-reset, because the default ICMP error left IPv6 clients hanging until they timed out.

Review addressed in 36663c5 and 7cc4139, then master merged in (d24ca3e). The fence is now measured against real iptables in a network namespace. The unit starts after the services that restore a saved firewall at boot, and the comment says an on-demand restore unfences every port. The fatal path is documented as open. The refusal skips loopback and is now a tcp-reset, because the default ICMP error left IPv6 clients hanging until they timed out.