# fw: open items Most of this came from a code review after the first commit. Where a finding has been fixed it says so and how it was checked; where it has not, it says what is actually true. ## Fixed since the review 1. **The ctl filter was a blacklist.** `headers` on a udp conversation passed straight through, and the write queue is live from clone, so three writes sent a datagram anywhere with no `connect` for a rule to match. Now a whitelist: `bind`, `ttl`, `tos`, `ignoreadvice`, `close`, `hangup`, `keepalive` pass; `connect` and `announce` are rule-checked; everything else is refused by name. Verified — `headers` and gre `raw` are refused, `ttl` still works, and `none.ndb` now means what it says. 2. **`%M` was never installed**, so `fmtrules` emitted `ipmask=%M%` and every ctl edit on a rule set containing `ip=` failed. Reproduced, then fixed with `fmtinstall('M', eipfmt)`. Verified round-tripping. 3. **`parserules` built the new list in the globals** with no lock, so relays walked an empty then half-built list during any reload. Now built in locals. Verified: load, reload, a rejected file leaving the old rules, and edits. 4. **A 64KB frame on a 32KB proc stack** in `etherwriteip` — the same bug already fixed in `relay()` and left here. Now `Maxframe`. 5. **`putback`/`notehandler` were dead code** — `atexit` matches on the registering pid and `_exits` never runs the handlers. Removed, and every doc that claimed the card was put back is corrected. Nothing puts it back; see item 1 below. 6. **Expired flows kept matching.** `flowlook` refreshed `last` without checking the timeout, and reaping only happens on insert, so on an idle firewall a dead flow passed traffic forever. 7. **The pkt interface claimed a 4096 MTU** from a 1514-byte card. Now `mtu 1500`. Verified: `pkt0 maxtu 1500`. 8. **Ports were read out of non-first fragments**, so fragmented traffic was denied and a crafted fragment could pass. Later fragments now match on addresses and protocol only. 9. **Four holes in the filtered `/net`**: `/net/ndb` and `/net/log` were writable (mode 0666 — reconfigure name service, or trace every connection on the machine), and `ipifc/*/data` was readable, which on a machine also running `fw -e` is the packet stream itself. All closed, reads still work. Verified. 10. **Control files were world-writable.** Now 0600/0440 and owned by the user running `fw` — testing caught that owning them as a nonexistent user "fw" locked out the administrator too. 11. **`delete 0` and `delete foo`** appended a rule reading ``. Now refused. 12. **A dead relay left half a firewall** — one direction unfiltered, nothing to notice. Now `threadexitsall`. 13. **IPv6 unicast under `-e`** was dropped in silence. It is still not implemented — neighbour discovery is missing — but it now says so once instead of pretending. ## Still open ### 1. Nothing puts the card back Taking a card is destructive and is not undone. The pkt interface is `unbindonclose`, so when `fw` stops the interface and the address both go, leaving the card bound to nothing and the machine with no network. `fw` cannot restart unaided either: the address it would read off the card is the address that just vanished. The cleanup that claimed to handle this was dead code and has been removed. The answer is not more note handling — it has to be something that outlives `fw`, which means the supervisor, with the address stored where `fw` does not own it. This also blocks `svc` supervision. ### deny=in ip=... silently never matches in namespace mode An `announce` sets `anyip`, and `matchrule` skips rules naming an address, so a peer-address inbound rule is a no-op there while working at the packet layer. It is a reasoned decision, but by this project's own standard it should either be enforced at listen time (check `remote`, hang up) or refused at startup in that mode. ## Worth fixing - **`promiscuous` injects neighbours' unicast into the protected stack.** `ethermux` already delivers frames addressed to us plus multicast and broadcast; promiscuous only adds other machines' traffic, into the rules, the stats and the flow table. It is needed for multicast reception, so keep it and filter on the destination MAC in `etherin`: accept `ourmac` or `d[0]&1`. - **Control files are world-writable.** `ctl` and `rules` at 0666 means any user can rewrite the firewall. 0660 or 0600. - **Use-after-free of the matched rule.** `matchrule` returns `*rp` after dropping `rulelock` and callers read `rule->log` outside it. Copy the flag out under the lock. - **`learnaddr` truncates the routing table** — `buf[1024]`, `lines[8]`, against a table that routinely exceeds both, so the default route can be missed. - **`fmtrules` truncates silently at 64K**, so a large rule set loses rules on every ctl edit and on save. Detect and fail. - **`delete 0` / `delete foo`** falls into the append branch with a nil argument and appends a rule reading ``. - **A dead relay proc leaves a half-firewall** — the proc returns, the others carry on, one direction is permanently dead with nothing to notice. `threadexitsall` is the fail-closed answer. - **Static error buffers race under `srvrelease`** — `netfs.c` and `rules.c` both return `static char err[128]` while running multi-proc. Worst case is a wrong diagnostic, which is the thing namespace mode exists to produce. - **`revalidate` inflates hit counts** — it calls `matchrule`, so `stats` counts rule-change re-checks as decisions about traffic. ## Smaller - `relay()` and `permitted()` are the same decision logic twice with divergent log text; the gateway path should call `permitted()`. - The `reclaim` comment is orphaned above `putback`. - `putback`'s "the ctl fd must stay open" is wrong — `ethermedium.unbindonclose` is 0. - `fswalk1` does not reject names containing `/`. - `fwstart`'s `mntgen` inherits the caller's fds — the trap CLAUDE.md documents. ## Still open from before - **Broadcast handling is written but never observed.** The mapping is standard and normal traffic is unaffected, but no broadcast has been seen crossing `fw`. - **fw daemonizes**, so `svc` cannot supervise it. Needs a foreground mode; `ready=srv:name` fits, since `-s` already posts to `/srv`. `fwstart` is the wrong shape and should probably go. - **One card per fw, and rules cannot name a card.** Wants repeatable `-e` and an `ifc=` attribute together. - **Tflush is not implemented.** Only affects namespace mode. ~80 lines, with a race that cannot be fully closed. - **Positional `delete n` renumbers.** - **Never tested**: the gateway with two real VMs, any IPv6 traffic, a real second NIC carrying traffic. ## Docs that are now wrong - `design.md`, `todo.md` and `man/fw` all said `fw` "tries to put the card back as it exits, which covers an orderly stop". It covers nothing — see 5. - `man/fw` SYNOPSIS shows `-a` as required with `-e`; it is optional. - `man/fw` says an empty rule file means no network at all; see 1. - `design.md` presents "cs and dns come along for free" as a benefit. It is also why DNS cannot be blocked in namespace mode, which belongs next to it. ## Deliberately not doing **NAT**, rate limiting, fragment reassembly, ICMP type matching, deep IPv6. Scope, not difficulty.