Skip to content

Commit 26a5904

Browse files
committed
fix(router): publishing the panel on the uplink only opened half the path
Reported from the box: with both uplink toggles on, SSH answered from the WAN and the panel did not. Opening a port in INPUT is only half of it. SSH is a host service and is delivered locally, so it hits INPUT and the allow rule worked. The panel is an nginx container with published ports: a request to the box's address is DNAT'd and then FORWARDED to the container, never reaching INPUT, and the forward policy is drop. The LAN side already had a `ct status dnat` rule for exactly this reason — the WAN side, added later, did not. Worse than a missing feature: the operator turns the toggle on precisely so they can reach the panel after the switch, then cannot, and so cannot press confirm — the watchdog reverts a router that was working. That is what happened. Scoped to `ct status dnat` and the allowed ports rather than a blanket accept, so it publishes what was asked for and not every other container port that happens to be mapped. Verified on hardware: panel and SSH both answer from the uplink while router mode is up, with the LAN bridged across a wired port and the radio.
1 parent bbb7571 commit 26a5904

2 files changed

Lines changed: 80 additions & 1 deletion

File tree

backend/app/core/nftables.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,31 @@ async def apply_router_nat(
552552
]
553553
wan_service_rules = "\n".join(rules)
554554

555+
# The same ports again, in FORWARD — because opening them in INPUT is only
556+
# half the story. Services on the host (SSH) are delivered locally and hit
557+
# INPUT. The panel is an nginx container with published ports, so a request
558+
# to this box's address is DNAT'd and then FORWARDED to the container: it
559+
# never reaches INPUT at all, and the forward policy is drop.
560+
#
561+
# Observed exactly that: with both toggles on, SSH answered from the uplink
562+
# and the panel did not.
563+
#
564+
# Scoped to `ct status dnat` AND the allowed ports, so this admits the
565+
# services the operator published and not every other container port that
566+
# happens to be mapped.
567+
fwd = []
568+
if tcp_ports:
569+
fwd.append(
570+
f' iifname "{wan}" ct status dnat tcp dport '
571+
f'{{ {", ".join(map(str, tcp_ports))} }} accept'
572+
)
573+
if udp_ports:
574+
fwd.append(
575+
f' iifname "{wan}" ct status dnat udp dport '
576+
f'{{ {", ".join(map(str, udp_ports))} }} accept'
577+
)
578+
wan_published_rules = ("\n" + "\n".join(fwd)) if fwd else ""
579+
555580
# One transaction, not two. `delete table` followed by a separate load
556581
# meant a script that failed to parse left NO ruleset behind — the box
557582
# ends up with neither the old firewall nor the new one. Declaring the
@@ -621,7 +646,7 @@ async def apply_router_nat(
621646
# first apply cuts every LAN device off the panel, which also means
622647
# nobody can press the confirm button and the watchdog reverts a
623648
# working router purely because we blocked the way to its own UI.
624-
iifname "{lan}" ct status dnat accept
649+
iifname "{lan}" ct status dnat accept{wan_published_rules}
625650
# Container-to-container (the reverse proxy reaching the SPA) once
626651
# bridge-nf-call-iptables puts bridged traffic through this hook.
627652
iifname "br-*" oifname "br-*" accept

backend/tests/test_router_mode.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2357,3 +2357,57 @@ def _async(value):
23572357
async def _fn(*a, **k):
23582358
return value
23592359
return _fn
2360+
2361+
2362+
class TestPublishingThePanelOnTheUplink:
2363+
"""Opening a port in INPUT is only half the story. Services on the host —
2364+
SSH — are delivered locally and hit INPUT. The panel is an nginx container
2365+
with published ports, so a request to the box's address is DNAT'd and then
2366+
FORWARDED to the container, never reaching INPUT at all, and the forward
2367+
policy is drop. Observed on hardware: with both toggles on, SSH answered
2368+
from the uplink and the panel did not."""
2369+
2370+
def _chains(self, **kw):
2371+
import asyncio
2372+
from app.core import nftables as nft
2373+
cap = {}
2374+
2375+
async def fake(script):
2376+
cap["s"] = script
2377+
return True
2378+
2379+
original, nft._nft = nft._nft, fake
2380+
try:
2381+
asyncio.run(nft.apply_router_nat("eth0", "br-lan", **kw))
2382+
finally:
2383+
nft._nft = original
2384+
s = cap["s"]
2385+
return (s.split("chain input")[1].split("chain forward")[0],
2386+
s.split("chain forward")[1].split("chain ")[0])
2387+
2388+
def test_published_ports_are_allowed_in_forward_too(self):
2389+
_, fwd = self._chains(wan_allow_tcp=[22, 80, 443])
2390+
rule = [ln.strip() for ln in fwd.splitlines()
2391+
if "ct status dnat" in ln and "eth0" in ln]
2392+
assert len(rule) == 1
2393+
assert "tcp dport { 22, 80, 443 }" in rule[0]
2394+
2395+
def test_the_forward_allowance_is_scoped_to_the_published_ports(self):
2396+
"""Not a blanket `ct status dnat accept` on the uplink: that would
2397+
expose every other container port that happens to be mapped."""
2398+
_, fwd = self._chains(wan_allow_tcp=[80])
2399+
rule = next(ln.strip() for ln in fwd.splitlines()
2400+
if "ct status dnat" in ln and "eth0" in ln)
2401+
assert "dport" in rule
2402+
assert rule != 'iifname "eth0" ct status dnat accept'
2403+
2404+
def test_nothing_is_published_when_no_ports_are_opened(self):
2405+
_, fwd = self._chains()
2406+
assert not [ln for ln in fwd.splitlines()
2407+
if "ct status dnat" in ln and "eth0" in ln]
2408+
2409+
def test_udp_ports_get_the_same_treatment(self):
2410+
_, fwd = self._chains(wan_allow_udp=[51820])
2411+
rule = [ln.strip() for ln in fwd.splitlines()
2412+
if "ct status dnat" in ln and "udp dport" in ln]
2413+
assert len(rule) == 1 and "51820" in rule[0]

0 commit comments

Comments
 (0)