Skip to content

Fix: Select iptables backend by kernel module, fail loud on misprogram #518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
huang195 merged 2 commits into from
Jun 16, 2026
Merged

Fix: Select iptables backend by kernel module, fail loud on misprogram #518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c609254
Fix: Select iptables backend by kernel module, fail loud on misprogram
huang195 Jun 16, 2026
d6e1f91
Fix: Guard IPv6 enforce-redirect jumps with require_jump (review #518)
huang195 Jun 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion authbridge/proxy-init/Dockerfile.init
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
FROM docker.io/library/alpine:3.23

# Install both iptables-nft and iptables-legacy; init-iptables.sh auto-detects
# the correct backend at runtime (prefers legacy for Kind/kubeadm compatibility).
# the correct backend at runtime by reading /proc/modules (iptable_nat loaded =>
# legacy, as on Kind/kubeadm; absent => nft, as on OpenShift/ROSA).
# The separate 'iptables-legacy' package is required — Alpine's base 'iptables'
# only provides nft binaries since Alpine 3.20+.
RUN apk add --no-cache iptables iptables-legacy
Expand Down
50 changes: 43 additions & 7 deletions authbridge/proxy-init/init-iptables.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,11 @@
# but the cluster's networking stack uses legacy tables, our PREROUTING rules
# have no effect — traffic bypasses Envoy's inbound listener entirely.
#
# This script auto-detects the correct backend by checking if iptables-legacy
# is available and can manipulate the nat table in this network namespace.
# Override with IPTABLES_CMD env var if needed.
# This script auto-detects the correct backend by reading the kernel module
# table (/proc/modules): the legacy nat path needs the `iptable_nat` module,
# which is loaded on legacy clusters (Kind, kubeadm — kube-proxy uses it) and
# ABSENT on nft-only platforms (OpenShift/ROSA expose only nf_tables +
# nft_compat). Override with IPTABLES_CMD env var if needed.
Comment on lines +121 to +125

@coderabbitai coderabbitai Bot Jun 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align the debugging tip with backend auto-detection behavior.

Line 109 still says both components use iptables-legacy, but this file now supports nft backend selection. That contradiction can mislead runtime triage on nft-only clusters.

📝 Proposed doc fix 
-#   iptables-legacy-save       — both Istio and AuthProxy use iptables-legacy
+#   iptables-save / iptables-legacy-save
+#                               — inspect whichever backend was selected at startup
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@authbridge/proxy-init/init-iptables.sh` around lines 121 - 125, The debugging
tip comment that states both components use iptables-legacy contradicts the
newly documented backend auto-detection behavior. Update the outdated comment to
reflect that the script now intelligently auto-detects and supports both the
legacy nat backend (using iptable_nat kernel module on legacy clusters like Kind
and kubeadm) and the nft-only backend (on platforms like OpenShift/ROSA), as
described in the adjacent documentation. This ensures the debugging guidance
aligns with the actual auto-detection capability explained in the kernel module
detection logic.

#
# ─────────────────────────────────────────────────────────────────────────────

Expand Down Expand Up @@ -148,16 +150,26 @@ case "${MODE}" in
esac

# --- Auto-detect iptables backend ---
# Prefer iptables-legacy for maximum compatibility with Kubernetes networking.
# The nft backend sets rules in a different netfilter table that may not be
# processed for PREROUTING when the cluster uses legacy iptables.
# Select the backend that is actually FUNCTIONAL in this kernel, not merely
# listable. A readability probe (`iptables-legacy -t nat -L`) mis-selects legacy
# on nft-only nodes: the empty legacy nat table still lists fine, so legacy gets
# chosen and our rules land in a table nothing consults (silent fail-open). A
# write-probe is unreliable too — individual legacy ops succeed via nft_compat
# even where the full nat pipeline later fails. Instead we check whether the
# `iptable_nat` kernel module is loaded: present => legacy nat is in use, match
# it (Kind, kubeadm); absent => nf_tables is the only working path (OpenShift/
# ROSA, EKS-nft). /proc/modules is host-kernel-wide, readable from the pod netns,
# timing-independent (unlike matching Istio's not-yet-installed rules), and not
# foolable by nft_compat. Override with IPTABLES_CMD (the operator can set it
# per-platform). PROC_MODULES is overridable for tests.
PROC_MODULES="${PROC_MODULES:-/proc/modules}"
detect_iptables_cmd() {
if [ -n "${IPTABLES_CMD:-}" ]; then
echo "${IPTABLES_CMD}"
return
fi
if command -v iptables-legacy >/dev/null 2>&1 && \
iptables-legacy -t nat -L -n >/dev/null 2>&1; then
grep -q '^iptable_nat ' "${PROC_MODULES}" 2>/dev/null; then
echo "iptables-legacy"
else
echo "iptables"
Expand All @@ -167,6 +179,24 @@ detect_iptables_cmd() {
IPT=$(detect_iptables_cmd)
echo "Using iptables command: ${IPT} ($(${IPT} --version 2>/dev/null || echo 'unknown version'))"

# Fail loud if a jump rule we just installed is not actually present in the
# chosen backend. `set -e` already aborts on hard programming errors; this also
# catches the subtler case where a command returned 0 but the rule did not land
# (e.g. rules written into a backend that is not the live datapath), so a

@mrsabath mrsabath Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit — the parenthetical “(e.g. rules written into a backend that is not the live datapath)” slightly overstates what require_jump catches. The -C check runs against the chosen backend (${_rj_cmd}), so if detection mis-selects a backend, the rule is both written to and found in that same backend and require_jump passes — it can't detect mis-selection. What it genuinely guards is the “insert returned 0 but the rule didn't persist” case. Consider trimming the e.g. so the comment doesn't read as datapath-vs-backend validation. Mechanism is sound and strictly additive — no functional concern.

# fail-closed traffic guard can never silently no-op. Usage:
# require_jump <cmd> <table> <parent-chain> <our-chain> [extra match tokens...]
require_jump() {
_rj_cmd="$1"; _rj_tbl="$2"; _rj_parent="$3"; _rj_chain="$4"; shift 4
if ! ${_rj_cmd} -t "${_rj_tbl}" -C "${_rj_parent}" "$@" -j "${_rj_chain}" 2>/dev/null; then
echo "ERROR: ${_rj_cmd}: '${_rj_chain}' is not installed in ${_rj_tbl} ${_rj_parent}." >&2
echo "ERROR: traffic interception is NOT active — refusing to start with enforcement disabled." >&2
echo "ERROR: the selected iptables backend could not program rules. On nft-only or managed" >&2
echo "ERROR: platforms set IPTABLES_CMD=iptables and ensure NET_ADMIN/NET_RAW are granted" >&2
echo "ERROR: (and, on OpenShift, an SELinux context that permits nf_tables access)." >&2
exit 1
fi
}

PROXY_PORT="${PROXY_PORT:-15123}"
INBOUND_PROXY_PORT="${INBOUND_PROXY_PORT:-15124}"
# enforce-redirect mode: the forward proxy's transparent listener port, the
Expand Down Expand Up @@ -307,6 +337,7 @@ setup_enforce_redirect() {
if ! ${IPT} -t nat -C OUTPUT -j "${REDIR_CHAIN}" 2>/dev/null; then
${IPT} -t nat -I OUTPUT 1 -j "${REDIR_CHAIN}"
fi
require_jump "${IPT}" nat OUTPUT "${REDIR_CHAIN}"

# --- IPv4: mangle DROP for non-TCP ---
${IPT} -t mangle -N "${NOTCP_CHAIN}" 2>/dev/null || true
Expand All @@ -332,6 +363,7 @@ setup_enforce_redirect() {
if ! ${IPT} -t mangle -C OUTPUT -j "${NOTCP_CHAIN}" 2>/dev/null; then
${IPT} -t mangle -I OUTPUT 1 -j "${NOTCP_CHAIN}"
fi
require_jump "${IPT}" mangle OUTPUT "${NOTCP_CHAIN}"
echo "enforce-redirect: IPv4 egress capture configured"

# --- IPv6 ---
Expand All @@ -357,6 +389,7 @@ setup_enforce_redirect() {
if ! ${IP6T} -t nat -C OUTPUT -j "${REDIR_CHAIN}" 2>/dev/null; then
${IP6T} -t nat -I OUTPUT 1 -j "${REDIR_CHAIN}"
fi
require_jump "${IP6T}" nat OUTPUT "${REDIR_CHAIN}"

${IP6T} -t mangle -N "${NOTCP_CHAIN}" 2>/dev/null || true
${IP6T} -t mangle -F "${NOTCP_CHAIN}"
Expand All @@ -377,6 +410,7 @@ setup_enforce_redirect() {
if ! ${IP6T} -t mangle -C OUTPUT -j "${NOTCP_CHAIN}" 2>/dev/null; then
${IP6T} -t mangle -I OUTPUT 1 -j "${NOTCP_CHAIN}"
fi
require_jump "${IP6T}" mangle OUTPUT "${NOTCP_CHAIN}"
echo "enforce-redirect: IPv6 egress capture configured"
else
echo "enforce-redirect: ip6tables unavailable — skipping IPv6 egress capture"
Expand Down Expand Up @@ -483,6 +517,7 @@ ${IPT} -t nat -A PROXY_OUTPUT -p tcp -j REDIRECT --to-port "${PROXY_PORT}"
if ! ${IPT} -t nat -C OUTPUT -p tcp -j PROXY_OUTPUT 2>/dev/null; then
${IPT} -t nat -I OUTPUT 1 -p tcp -j PROXY_OUTPUT
fi
require_jump "${IPT}" nat OUTPUT PROXY_OUTPUT -p tcp

# --- Mangle rule: prevent Envoy→app loop through ISTIO_OUTPUT ---
# After inbound JWT validation, Envoy (UID 1337) connects to the local app.
Expand Down Expand Up @@ -566,6 +601,7 @@ ${IPT} -t nat -A PROXY_INBOUND -p tcp -j REDIRECT --to-port "${INBOUND_PROXY_POR
if ! ${IPT} -t nat -C PREROUTING -p tcp -j PROXY_INBOUND 2>/dev/null; then
${IPT} -t nat -I PREROUTING 1 -p tcp -j PROXY_INBOUND
fi
require_jump "${IPT}" nat PREROUTING PROXY_INBOUND -p tcp

echo "Inbound iptables rules configured successfully"
echo "Inbound traffic will be redirected to port ${INBOUND_PROXY_PORT}"
Expand Down
23 changes: 23 additions & 0 deletions authbridge/proxy-init/test-enforce-redirect.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,29 @@ else
echo "PASS: init aborts fail-closed when resolv.conf has no nameservers"
fi

echo "### Backend detection unit test (/proc/modules seam)"
# Pull detect_iptables_cmd (and its PROC_MODULES default) out of the script and
# exercise it against fixture module tables — no real kernel needed. The legacy
# branch also requires the iptables-legacy binary, so skip the legacy-positive
# case when it is not installed on the host.
eval "$(sed -n '/^PROC_MODULES=/,/^}/p' "${INIT}")"

@mrsabath mrsabath Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must-fix — this detection unit test is short-circuited by the harness's own IPTABLES_CMD, so it doesn't test what it claims to.

The re-exec at L39–40 injects IPTABLES_CMD="${IPT}" (= iptables-nft) into the environment for the entire netns run, and detect_iptables_cmd checks IPTABLES_CMD first and returns it immediately. So:

  • L172 (mods_legacy) returns iptables-nft, expected iptables-legacy → FAIL
  • L178 (mods_nft) returns iptables-nft, expected iptables → FAIL

Reproduced by extracting the function and exporting IPTABLES_CMD=iptables-nft: both cases return iptables-nft. This stays green in CI only because no workflow invokes this harness (CI = sh -n/shellcheck + the proxy-init Docker build). The override case (L181) passes for the wrong reason — it sets its own override, so it'd pass even if detection were completely broken.

Fix: clear the inherited var for the non-override subshells, e.g. wrap the block with env -u IPTABLES_CMD or unset IPTABLES_CMD before L172/L178 (the L181 override case re-sets it explicitly, so it's unaffected).

mods_legacy=$(mktemp); printf 'ip_tables 28672 4 - Live 0x0\niptable_nat 12288 19 - Live 0x0\n' > "${mods_legacy}"
mods_nft=$(mktemp); printf 'nf_tables 315392 344 nft_compat - Live 0x0\nnft_compat 20480 0 - Live 0x0\n' > "${mods_nft}"
if command -v iptables-legacy >/dev/null 2>&1; then
got=$(PROC_MODULES="${mods_legacy}" detect_iptables_cmd)
[ "${got}" = "iptables-legacy" ] && echo "PASS: iptable_nat loaded => iptables-legacy" \
|| { echo "FAIL: expected iptables-legacy, got '${got}'"; fail=1; }
else
echo "SKIP: iptables-legacy not installed on host — legacy-positive case skipped"
fi
got=$(PROC_MODULES="${mods_nft}" detect_iptables_cmd)
[ "${got}" = "iptables" ] && echo "PASS: iptable_nat absent => iptables (nft)" \
|| { echo "FAIL: expected iptables, got '${got}'"; fail=1; }
got=$(IPTABLES_CMD=iptables-legacy PROC_MODULES="${mods_nft}" detect_iptables_cmd)
[ "${got}" = "iptables-legacy" ] && echo "PASS: IPTABLES_CMD override wins over detection" \
|| { echo "FAIL: override ignored, got '${got}'"; fail=1; }
rm -f "${mods_legacy}" "${mods_nft}"

echo
[ "${fail}" -eq 0 ] && echo "ALL TESTS PASSED" || echo "SOME TESTS FAILED"
exit "${fail}"
Loading