diff --git a/README.md b/README.md index 78e9621..85af015 100644 --- a/README.md +++ b/README.md @@ -390,7 +390,10 @@ Six things, all recorded in `.install-stamp` and all undone by the uninstaller. 4. The two systemd units, if you let the installer add them. 5. The device-mapper pool `brig-devpool` and its loop devices, which are kernel-global objects. -6. The `brig0` CNI bridge. +6. The `brig0` CNI bridge, and the two rules the CNI firewall plugin adds for + it to the `CNI-ISOLATION-STAGE-1` and `-2` chains. Those chains are shared + with other CNI networks, so the uninstaller removes only the rules that + name `brig0`. Nothing else. The host's `/etc/containerd/config.toml` is not read or written, and the host containerd is never restarted. diff --git a/scripts/build-bundle.sh b/scripts/build-bundle.sh index 55d5f12..126daa6 100755 --- a/scripts/build-bundle.sh +++ b/scripts/build-bundle.sh @@ -877,11 +877,43 @@ for l in "${BRIG_LAUNCHER:-}" "${BRIGD_LAUNCHER:-}"; do fi done -if [ "${INSTALL_MODE:-system}" != "user" ] \ - && [ -n "${BRIDGE_NAME:-}" ] && ip link show "$BRIDGE_NAME" >/dev/null 2>&1; then - ip link set "$BRIDGE_NAME" down 2>/dev/null || true - ip link delete "$BRIDGE_NAME" 2>/dev/null || true - info "removed the CNI bridge $BRIDGE_NAME" +# drop_bridge_rules : delete the rules naming with -i or -o +# from the CNI-* chains (filter and nat, IPv4 and IPv6). The CNI firewall +# plugin adds two such rules per bridge and never removes them. The chains are +# shared with other CNI networks, so they stay, and so do other bridges' rules. +# Each rule goes to iptables-restore as iptables-save printed it, so the shell +# never parses its quoted comment. A failed delete only warns, with iptables' +# own reason. The xtables lock is waited on for 5 seconds at most, so a lock +# another process holds cannot hang the uninstall. +drop_bridge_rules() { + for ipt in iptables ip6tables; do + command -v "$ipt-save" >/dev/null 2>&1 || continue + command -v "$ipt-restore" >/dev/null 2>&1 || continue + for table in filter nat; do + "$ipt-save" -t "$table" 2>/dev/null | awk -v br="$1" ' + $1 == "-A" && $2 ~ /^CNI-/ { + for (i = 3; i < NF; i++) + if (($i == "-i" || $i == "-o") && $(i + 1) == br) { print; next } + }' | while IFS= read -r rule; do + if err="$(printf '*%s\n-D %s\nCOMMIT\n' "$table" "${rule#-A }" \ + | "$ipt-restore" -w 5 --noflush 2>&1)"; then + info "removed $ipt -t $table ${rule#-A }" + else + warn "could not remove $ipt -t $table ${rule#-A }: $err" + fi + done + done + done +} + +if [ "${INSTALL_MODE:-system}" != "user" ] && [ -n "${BRIDGE_NAME:-}" ]; then + if ip link show "$BRIDGE_NAME" >/dev/null 2>&1; then + ip link set "$BRIDGE_NAME" down 2>/dev/null || true + ip link delete "$BRIDGE_NAME" 2>/dev/null || true + info "removed the CNI bridge $BRIDGE_NAME" + fi + # The rules outlive the bridge, so they go whether or not it is still up. + drop_bridge_rules "$BRIDGE_NAME" fi if [ "${GROUP_CREATED:-false}" = "true" ] && [ -n "${BRIG_GROUP:-}" ]; then diff --git a/tests/uninstall-cni-rules.sh b/tests/uninstall-cni-rules.sh new file mode 100644 index 0000000..304fa57 --- /dev/null +++ b/tests/uninstall-cni-rules.sh @@ -0,0 +1,231 @@ +#!/bin/sh +# +# Run the generated brig-uninstall.sh on a fake system install and check what it +# does to the CNI firewall rules. iptables-save and iptables-restore are stubs +# that answer from, and apply -D lines to, per-table rule files in a scratch +# directory. The uninstall must delete the rules that name brig0 in the CNI-* +# chains and leave the shared chains and other bridges' rules alone. + +set -eu + +here="$(cd "$(dirname "$0")/.." && pwd)" + +fail() { echo "FAIL: $*" >&2; exit 1; } +ok() { echo "ok - $*"; } + +T="$(mktemp -d)" +trap 'rm -rf "$T"' EXIT +mkdir -p "$T/stub" "$T/bare" "$T/state" "$T/want" "$T/home" + +# Generate the uninstaller with the build's own function. +STAGE="$T/stage" +mkdir -p "$STAGE/bin" +eval "$(awk '/^write_uninstaller\(\) \{/ {f=1} /^fetch\(\) \{/ {f=0} f' \ + "$here/scripts/build-bundle.sh")" +write_uninstaller + +# The commands the uninstaller reaches for on the host. id answers FAKE_UID, ip +# answers whether the bridge exists, and the rest only log. +cat > "$T/bare/id" <<'STUB' +#!/bin/sh +echo "${FAKE_UID:-0}" +STUB +cat > "$T/bare/ip" <> "$T/log" +case "\$*" in "link show"*) [ "\${FAKE_BRIDGE:-up}" = up ] ;; esac +STUB +for c in systemctl pkill sleep umount dmsetup losetup groupdel; do + printf '#!/bin/sh\necho "%s $*" >> "%s/log"\n' "$c" "$T" > "$T/bare/$c" +done +chmod 0755 "$T/bare"/* +cp "$T/bare"/* "$T/stub/" + +# -save -t prints $T/state/.
. -restore takes +# "*
", "-D " and "COMMIT" lines and drops the first "-A " +# from that file, or fails as iptables does when the rule is not there. It +# refuses to run without --noflush, which would flush the whole table. +cat > "$T/stub/iptables-save" <> "$T/log" +[ "\$1" = -t ] || exit 2 +cat "$T/state/\$ipt.\$2" 2>/dev/null || true +STUB +cat > "$T/stub/iptables-restore" <> "$T/log" +case " \$* " in *" --noflush "*) ;; *) exit 3 ;; esac +[ "\${FAKE_RESTORE_FAIL:-}" != 1 ] || { echo "\$ipt-restore: line 2 failed: Bad rule" >&2; exit 1; } +table="" +while IFS= read -r l; do + case "\$l" in + "*"*) table="\${l#\*}" ;; + COMMIT) ;; + "-D "*) + f="$T/state/\$ipt.\$table" + grep -qxF -- "-A \${l#-D }" "\$f" 2>/dev/null || exit 1 + awk -v r="-A \${l#-D }" '\$0 == r && !d {d = 1; next} {print}' "\$f" > "\$f.new" + mv "\$f.new" "\$f" ;; + *) exit 4 ;; + esac +done +STUB +chmod 0755 "$T/stub"/iptables-* +ln -s iptables-save "$T/stub/ip6tables-save" +ln -s iptables-restore "$T/stub/ip6tables-restore" + +# The filter table as nuc8 had it after a system install, with brig0 next to a +# Docker bridge, plus two rules naming brig0 that are not ours to remove: one in +# FORWARD, outside the CNI-* chains, and one for brig0x. +cat > "$T/filter" <<'RULES' +*filter +:INPUT ACCEPT [0:0] +:FORWARD DROP [0:0] +:OUTPUT ACCEPT [0:0] +:CNI-ADMIN - [0:0] +:CNI-FORWARD - [0:0] +:CNI-ISOLATION-STAGE-1 - [0:0] +:CNI-ISOLATION-STAGE-2 - [0:0] +-A FORWARD -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge|isolated)" -j CNI-ISOLATION-STAGE-1 +-A FORWARD -m comment --comment "CNI firewall plugin rules" -j CNI-FORWARD +-A FORWARD -i brig0 -j ACCEPT +-A CNI-FORWARD -m comment --comment "CNI firewall plugin admin overrides" -j CNI-ADMIN +-A CNI-ISOLATION-STAGE-1 -i br-177cd0907bb6 ! -o br-177cd0907bb6 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge)" -j CNI-ISOLATION-STAGE-2 +-A CNI-ISOLATION-STAGE-1 -i brig0 ! -o brig0 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge)" -j CNI-ISOLATION-STAGE-2 +-A CNI-ISOLATION-STAGE-1 -i brig0x ! -o brig0x -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge)" -j CNI-ISOLATION-STAGE-2 +-A CNI-ISOLATION-STAGE-1 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge|isolated)" -j RETURN +-A CNI-ISOLATION-STAGE-2 -o br-177cd0907bb6 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge|isolated)" -j DROP +-A CNI-ISOLATION-STAGE-2 -o brig0 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge|isolated)" -j DROP +-A CNI-ISOLATION-STAGE-2 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge|isolated)" -j RETURN +COMMIT +RULES +grep -vxF \ + -e '-A CNI-ISOLATION-STAGE-1 -i brig0 ! -o brig0 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge)" -j CNI-ISOLATION-STAGE-2' \ + -e '-A CNI-ISOLATION-STAGE-2 -o brig0 -m comment --comment "CNI firewall plugin rules (ingressPolicy: same-bridge|isolated)" -j DROP' \ + "$T/filter" > "$T/want/iptables.filter" + +# The nat and IPv6 tables carry no brig0 rule on a real host. These are made up +# to cover the other tables the uninstaller reads. +cat > "$T/nat" <<'RULES' +*nat +:POSTROUTING ACCEPT [0:0] +:CNI-HOSTPORT-MASQ - [0:0] +-A POSTROUTING -m comment --comment "CNI portfwd requiring masquerade" -j CNI-HOSTPORT-MASQ +-A CNI-HOSTPORT-MASQ -o brig0 -m comment --comment "made up" -j MASQUERADE +-A CNI-HOSTPORT-MASQ -o br-177cd0907bb6 -m comment --comment "made up" -j MASQUERADE +COMMIT +RULES +grep -v -- '-o brig0 ' "$T/nat" > "$T/want/iptables.nat" +grep -v 'brig0x\|br-177' "$T/filter" > "$T/filter6" +grep -v -- '^-A CNI-.*brig0' "$T/filter6" > "$T/want/ip6tables.filter" + +reset_state() { + rm -f "$T/state"/* "$T/log" + cp "$T/filter" "$T/state/iptables.filter" + cp "$T/nat" "$T/state/iptables.nat" + cp "$T/filter6" "$T/state/ip6tables.filter" +} + +# install : a fresh install tree for the uninstaller to remove, with the +# stamp install.sh writes. +install() { + rm -rf "$T/inst" + mkdir -p "$T/inst/data/bin" "$T/inst/agent" "$T/run" + cp "$STAGE/bin/brig-uninstall.sh" "$T/inst/data/bin/" + cat > "$T/inst/data/.install-stamp" <: run the uninstaller with that PATH, keep its output. The +# FAKE_* settings are passed on by hand: whether an assignment in front of a +# function call reaches the commands it runs differs between shells. +uninstall() { + FAKE_UID="${FAKE_UID:-0}" FAKE_BRIDGE="${FAKE_BRIDGE:-up}" \ + FAKE_RESTORE_FAIL="${FAKE_RESTORE_FAIL:-}" \ + HOME="$T/home" XDG_CONFIG_HOME="$T/home/.config" PATH="$1" \ + "$T/inst/data/bin/brig-uninstall.sh" --yes > "$T/out" 2>&1 \ + || { cat "$T/out" >&2; fail "the uninstall failed"; } + grep -q '^\[brig-uninstall\] done$' "$T/out" || { cat "$T/out" >&2; fail "the uninstall did not finish"; } + [ ! -e "$T/inst/data" ] || fail "the uninstall left $T/inst/data" +} + +same_state() { + for f in iptables.filter iptables.nat ip6tables.filter; do + cmp -s "$T/state/$f" "$1/$f" || { diff "$1/$f" "$T/state/$f" >&2 || true; fail "$2: $f"; } + done +} + +mkdir -p "$T/orig" +cp "$T/filter" "$T/orig/iptables.filter" +cp "$T/nat" "$T/orig/iptables.nat" +cp "$T/filter6" "$T/orig/ip6tables.filter" + +# A system install with the bridge still up. +reset_state; install system +FAKE_UID=0 FAKE_BRIDGE=up uninstall "$T/stub:/usr/bin:/bin" +grep -q '^ip link delete brig0$' "$T/log" || fail "the bridge was not deleted" +same_state "$T/want" "the brig0 rules are not exactly the ones removed" +[ "$(grep -c '^\[brig-uninstall\] removed ip6\{0,1\}tables ' "$T/out")" -eq 5 ] \ + || { cat "$T/out" >&2; fail "the uninstall did not report five removed rules"; } +grep -q ':CNI-ISOLATION-STAGE-2 - ' "$T/state/iptables.filter" || fail "a shared chain went" +ok "a system uninstall removes the brig0 rules and keeps the rest" + +# Again on what is left: nothing matches, so nothing is deleted or said. +rm -f "$T/log"; install system +FAKE_UID=0 FAKE_BRIDGE=up uninstall "$T/stub:/usr/bin:/bin" +! grep -q 'restore' "$T/log" || fail "a second uninstall still deleted rules" +! grep -q 'tables\|WARNING' "$T/out" || { cat "$T/out" >&2; fail "a second uninstall was not quiet"; } +same_state "$T/want" "a second uninstall changed the rules" +ok "a second uninstall finds nothing and says nothing" + +# nuc8: the bridge was already gone by the time the uninstall ran. +reset_state; install system +FAKE_UID=0 FAKE_BRIDGE=absent uninstall "$T/stub:/usr/bin:/bin" +! grep -q '^ip link delete' "$T/log" || fail "an absent bridge was deleted" +same_state "$T/want" "the brig0 rules stayed with the bridge gone" +ok "the rules go when the bridge is already gone" + +# iptables fails every delete: the uninstall warns and finishes. +reset_state; install system +FAKE_UID=0 FAKE_RESTORE_FAIL=1 uninstall "$T/stub:/usr/bin:/bin" +[ "$(grep -c 'WARNING: could not remove ip6\{0,1\}tables ' "$T/out")" -eq 5 ] \ + || { cat "$T/out" >&2; fail "the failed deletes were not reported"; } +[ "$(grep -c 'WARNING: could not remove ip6\{0,1\}tables .*: ip6\{0,1\}tables-restore: line 2 failed: Bad rule' "$T/out")" -eq 5 ] \ + || { cat "$T/out" >&2; fail "a failed delete did not say why"; } +grep -q -- '-restore -w 5 --noflush' "$T/log" || fail "the xtables lock wait is not bounded" +same_state "$T/orig" "a failed delete changed the rules" +ok "a failing iptables does not stop the uninstall" +unset FAKE_RESTORE_FAIL FAKE_BRIDGE + +# A user install owns no host firewall rules. +reset_state; install user +FAKE_UID=1000 uninstall "$T/stub:/usr/bin:/bin" +! grep -q 'tables' "$T/log" || fail "a user uninstall ran iptables" +same_state "$T/orig" "a user uninstall changed the rules" +ok "a user uninstall leaves iptables alone" + +# A host without iptables at all. +if PATH="$T/bare:/usr/bin:/bin" command -v iptables-save >/dev/null 2>&1; then + echo "SKIP: iptables-save is in /usr/bin or /bin here" +else + rm -f "$T/log"; install system + FAKE_UID=0 uninstall "$T/bare:/usr/bin:/bin" + ok "a system uninstall without iptables finishes" +fi