# fw: open items

Most of this came from code review. Where a finding has been fixed it
says so; where it has not, it says what is actually true. An item never
appears in both halves.

`test/fwtest.rc` is a check for every bug that has shipped here, and
for the properties that must not quietly stop being true — 87 of them
now. Run it twice in a row after touching anything.

## Fixed, first round

1. **The ctl filter was a blacklist.** `headers` on a udp conversation
   passed, and the write queue is live from clone, so three writes sent
   a datagram anywhere. Now a whitelist of control messages.
2. **`%M` was never installed**, so `fmtrules` emitted `ipmask=%M%` and
   every ctl edit on a rule set containing `ip=` failed.
3. **`parserules` built the new list in the globals** with no lock, so
   relays walked an empty then half-built list during any reload.
4. **A 64KB frame on a 32KB proc stack** in `etherwriteip`.
5. **`putback`/`notehandler` were dead code.** `atexit` matches on the
   registering pid and `_exits` never runs the handlers.
6. **Expired flows kept matching**: `flowlook` refreshed `last` without
   checking the timeout, and reaping only happens on insert.
7. **The pkt interface claimed a 4096 MTU** from a 1514-byte card.
8. **Ports were read out of non-first fragments.**
9. **`/net/ndb` and `/net/log` were writable and `ipifc/*/data`
   readable** through the filtered `/net`.
10. **Control files were world-writable.**
11. **`delete 0` and `delete foo`** appended a rule reading `<nil>`.
12. **A dead relay left half a firewall.** Now `threadexitsall`.
13. **IPv6 unicast under `-e`** was dropped in silence.

## Fixed, second round

1. **The path filter was still a blacklist, and still had a hole.**
   `/net/*/trans` installs kernel address translations, gated by
   `iseve()` — which through this server is `fw`'s identity, not the
   caller's. Replaced with a list of what is served. `/net/log` was
   half closed the same way, and `ipifc/*/snoop` is the same wire as
   `data`.
2. **The suite poisoned its own next run**, and a comment blamed
   timing. Six `fw` processes and their interfaces survived every run.
3. **Checks that passed for the wrong reason**: "refused" was returned
   for any failure, so a check for a hole passed on a kernel that never
   had the hole.
4. **The dead code from item 5 above was not actually deleted.**
5. **Two compiler warnings** had stood since the program was written.
6. **The frame buffer belongs to the caller**, not to a proc stack.

## Fixed, third round

1. **matchrule returned things whose lifetime it did not own** — a
   `Rule*` and a static error buffer, both read after it released the
   lock, so a rule set installed in between freed one and two procs
   deciding at once overwrote the other. Now a `Match` in the caller's
   frame. `netfs.c` had its own static buffer with the same problem.
2. **revalidate counted as traffic.** Re-checking live flows after an
   edit charged each one to the rule that matched, so `stats` answered
   a different question after every edit.
3. **A rule set was 64 kilobytes long.** `fmtrules` clipped, and since
   every ctl edit formats the set out and parses it back, editing a set
   past the limit truncated the *rules*: 2000 in, 1214 out, and the
   next edit failed. `flows` had the same cap.
4. **A fragmented datagram did not cross.** Later fragments matched no
   port and every rule set is written in ports: measured, one fragment
   passed and two dropped. The first fragment now decides and the train
   inherits.
5. **`deny=in ip=...` was a no-op in namespace mode.** An announce
   names no peer, so rules naming one were skipped and nothing asked
   again. The peer is now checked at listen time, and a refused caller
   is hung up rather than handed to the program.
6. **`fwstart` had never worked.** `mntgen /mnt/fw` was run only when
   `/mnt/fw` was missing, which is the one case where mount(2) fails,
   so the control directories never appeared. It also left its servers
   holding the console.
7. **The log named an address without its port** — `getfields` writes
   over the separators it splits on.
8. **relay() and permitted() were the same logic twice**, already
   drifted apart in their debug output.
9. **The interface tables were read into fixed buffers** and searched
   as blobs rather than parsed. Not the defect it was reported as — the
   default route sorts first, so it was always found — but the device
   name was matched against the whole status text, addresses included.
10. **Promiscuous mode had no filter behind it.** The card must be
    promiscuous for multicast, but every neighbour's unicast was then
    judged, counted and flow-tracked as if it were ours.
11. **"A dead fw takes the network with it" was the requirement, not
    the bug.** It had been item 1 since the beginning, on the grounds
    that a dead `fw` leaves the machine with no network. That is the
    only defensible thing for a firewall to do: the alternative is
    traffic flowing unfiltered, which is the moment the firewall exists
    for. All three modes do it, by mechanism rather than by care —
    `unbindonclose` on the card and the gateway, a hung-up mount plus a
    dropped `#I` in a namespace — and all three are now measured.
    `putback`, deleted earlier as dead code, was the fail-open version
    of this; it never ran, so it was never wrong, but repairing it
    would have been. What remained was that recovery did not work:
    `-a` and `-g` are now in the service file so a restart does not
    need the address it just lost, and `fw` clears the dead mount its
    own corpse left on the control mountpoint, which was refusing the
    restart by way of the check meant to protect the card.
12. **"fw daemonizes, so svc cannot supervise it" was wrong.** `svc`
    has had the detaching shape from the start: `ready=srv:` watches
    the `/srv` name, and for those services the process exiting "is
    normal and never causes a restart". Nothing needed a foreground
    mode. What was true underneath it: the control filesystem going
    away ended the server proc and left the relays filtering, so the
    `/srv` name could be gone while the firewall was still running.
    Now any part stopping stops all of them, which is what makes the
    name worth watching. `fwstart` is still the wrong shape; a service
    file per card is in `lib/svc`.

## Still open

### 1. One card per fw, and rules cannot name a card

Wants repeatable `-e` and an `ifc=` attribute, together.

### 2. Tflush is not implemented

A request `fw` is blocked on cannot be abandoned. Only affects
namespace mode, where waiting for an inbound connection is the one
operation that blocks indefinitely. ~80 lines, with a race that cannot
be fully closed: between the syscall returning and the handler clearing
its entry, a note may land on a worker that has moved on.

### 3. Positional `delete n` renumbers

Inherent to positional deletion; iptables has it too. Said out loud
here rather than fixed.

## Decided, and now written down

**A dead `fw` leaves the network down, not open.** See `design.md` and
FAILURE in `fw(8)`. The tests assert it in card and namespace mode,
because it is exactly the kind of property a later helpful change would
reverse without meaning to.

## Tested on a wire

Two VMs on one ethernet segment, `run.sh -gw` and `-lan`; the procedure
is `test/wire.md` and it is worth keeping, because none of this can be
seen from one machine.

- **ARP for an address whose card has been taken.** The peer's arp
  table says `10.9.9.1 → 52540087c8c1`, which is the firewall's card
  and not the peer's own. Nothing else could have answered.
- **The first packet to an unresolved hop is dropped**, exactly as
  fw(8) BUGS says: first ping ~1s while fw ARPs for the peer, the rest
  sub-millisecond.
- **Filtering rather than forwarding.** `allow=in proto=icmp` alone:
  ping works, TCP sits for 290 seconds and times out. Add the rule
  through `ctl` and the same connect is refused in 2 seconds — refused
  is the far stack's RST, so the packet arrived.
- **Frames addressed to somebody else are ignored.** Forged with
  `test/rawether.c`. With the destination check the counter does not
  move; without it, a frame for nobody is judged exactly like one for
  us.

## Never tested

- **Broadcast handling.** Written, reviewed, still never observed
  crossing `fw`. The rig above could now show it — a broadcast from the
  peer is one `rawether` call away — and it has not been done.
- **IPv6 traffic**, in any mode. Under `-e` it cannot work at all:
  there is no neighbour discovery, so v6 unicast is dropped. IPv6
  extension headers are not walked, so v6 fragments do not cross.
- **The gateway between two real machines.** The wire rig proves card
  mode; the gateway has still only been run between two synthetic
  stacks on one machine.
- **A real NIC on real hardware**, as against an emulated e1000.

## Deliberately not doing

**NAT**, rate limiting, fragment reassembly, ICMP type matching, deep
IPv6. Scope, not difficulty.