Skip to content

fix(packaging): the wired LAN ports get a bridge of their own (br-mgmt) — rebased - #856

Open
Amperstrand wants to merge 1 commit into
mainfrom
pr/lan-mgmt-bridge
Open

Amperstrand wants to merge 1 commit into
mainfrom
pr/lan-mgmt-bridge

Conversation

@Amperstrand

Copy link
Copy Markdown
Collaborator

fix(packaging): the wired LAN ports get a bridge of their own, scoped to the admin surfaces

Implements the bridge half of
docs/architecture/lan-port-management-bridge-decision.md
(#599, D1-D8): the wired LAN ports move off the captive bridge onto br-mgmt, so
a cabled operator reaches the administration surfaces before paying anything, and
the port stops sharing a layer-2 domain with the strangers on the open guest
SSID.

The requirement this was written for has two halves, and this PR delivers one
of them.
The operator asked for a br-mgmt "where the connected devices can
access luci and the luci alternative dashboard, but they still need to pay". The
first half is here. The second half — a paywalled wired bridge — is not
delivered, and cannot be with the shipped stack: #599 measured that nodogsplash
5.0.2 gates exactly one interface and that a second instance is the dangerous
wrong answer (fixed nds* chain names in one namespace, teardown by name, one
control socket), so br-mgmt has no captive gate at all. A bridge this module
cannot gate must not take money it cannot deliver, so it is built as a
management network: admin surfaces reachable, no internet, and the payment
surfaces unreachable
. D8 in the decision record says exactly this; F1/F2 is
the follow-up if a wired customer network is still wanted.

The three writers

1. setup_mgmt_bridge in packaging/files/etc/uci-defaults/99-tollgate-setup
(+206) — MOVES the port list the base image writes on the captive bridge onto
br-mgmt:

  • it never names a port (eth1 on the MT3000, lan1…lan5 elsewhere — it moves
    what it finds on network.lan.device's device section, located by NAME because
    the device sections are anonymous in a shipped config);
  • a router with no port list on the captive bridge and none on br-mgmt is
    refused: ERROR, return 1, and no uci set at all — a portless br-mgmt
    would report a management bridge the operator has no way into;
  • an existing network.mgmt.ipaddr is kept (a reinstall must not move an address
    the operator chose); otherwise it takes the first free /24 next to the LAN's,
    skipping the private network's and any upstream collision, reusing the existing
    prefix24_of/octet_of/subnet_conflicts helpers;
  • dhcp.mgmt so the laptop has a lease (and therefore so its MAC still resolves,
    which is what keeps a future F1 possible without redesigning identity);
  • its own firewall.mgmt_zone with forward 'REJECT' and no config forwarding — deliberately not firewall.private_zone, whose forward 'ACCEPT' plus private → wan forwarding would hand every wired client free
    internet, which is the hole this change exists not to create.

The writer is re-asserted on the same-version (reinstall/upgrade) path as well
as full setup, because the port list is image-owned: sysupgrade -n or a factory
reset puts the wired port back on the captive bridge, and nothing else in this
repository would notice. Each of the three configs it touches is committed only
when its export actually differs, so a converged router is a no-op.

2. packaging/files/etc/nftables.d/33-mgmt-bridge-scope.nft (new, +86) — an
input fragment, same shape as its five siblings so fw4 reload loads it and a
nodogsplash restart does not remove it:

  • allow: DHCP/DNS (53/67/68), SSH (22), and the four administration surfaces
    443, 8080, 8090, 8443;
  • IPv6 dropped outright (the bridge has ip6assign '0' and no RA, the same
    posture setup_disable_ipv6_lan takes);
  • a catch-all counter drop for everything else, which is what makes

Replacement for #601: that PR's head branch on the felixfelix-bot fork was deleted, which froze its tracking — a recreated same-named branch cannot re-attach. This PR carries the identical change rebased onto current main (content ported via three-way patch; conflicts resolved: SSID fence + pre-commit fences unioned, rc-tester-guide kept at main's rc1 state, changelog entry re-spliced).

…LAN ports get a bridge of their own, scoped to the admin surfaces
@c03rad0r c03rad0r added the release:post-0.6.0 Deferred until after the v0.6.0 release label Oct 11, 2026
c03rad0r added a commit that referenced this pull request Oct 11, 2026
c03rad0r added a commit that referenced this pull request Oct 11, 2026
@c03rad0r

Copy link
Copy Markdown
Contributor

🔎 Integration finding — 33-mgmt-bridge-scope.nft reintroduces the #757 literal class

Merging this branch into the combined sandbox integration turns check-nft-interface-defs.sh red:

FAIL: a rule line still carries an interface literal
check-nft-interface-defs: FAIL (1)

The new fragment hardcodes iifname "br-mgmt" in its rule lines (6 rule lines). That is exactly the class #784's fence forbids — AGENTS.md's firewall rules of engagement: "No interface-name literals in rule lines — shipped or GENERATED", and it names this fragment specifically: "#601's planned static 33- fragment must follow the same rule." The fragment's own header already admits the caveat ("this assumes the wired management bridge is named br-mgmt").

Suggested fix (I implemented this on the sandbox integration branch to make the combined tree green — happy to have it land here instead):

  1. add define tg_mgmt_if = "br-mgmt" to the shipped packaging/files/etc/nftables.d/00-tollgate-defs.nft and to tollgate-nft-interfaces-render's output (resolved from network.mgmt.device, default br-mgmt, run through valid_ifname);
  2. replace iifname "br-mgmt" with iifname $tg_mgmt_if in 33-mgmt-bridge-scope.nft;
  3. have tests/packaging/mgmt-bridge-scope_test.sh assert the define form (and check-nft-interface-defs.sh cover the third define).

This keeps the fragment correct for a renamed management bridge too — the header's "assumes the name" caveat then goes away, which is the whole point of the #757 fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:post-0.6.0 Deferred until after the v0.6.0 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants