1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
|
# 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 — 91 of them
now. Run it twice in a row after touching anything, and boot with
`run.sh -gw` or fourteen of them skip. `test/wire.md` is the part that
needs a second machine.
## 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. **`/mnt/fw/ether0` was a name nothing made.** mntgen invents names
one level deep, so the mntgen over `/mnt` gives `/mnt/fw` and stops.
`fwstart` did it right by accident of being imperative; the service
file needed a second mntgen of its own, and now has one.
12. **"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.
13. **"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.
|
