summaryrefslogtreecommitdiff
path: root/fw/src/ether.c
diff options
context:
space:
mode:
authorCalvin Morrison <calvin@pobox.com>2026-08-18 19:32:56 -0400
committerCalvin Morrison <calvin@pobox.com>2026-08-18 19:32:56 -0400
commitb758d92ca80b25c0391dce4c7df73ef93aeeec99 (patch)
tree35f82e0c497aacba9d2aba558d7d596fcf7c3af9 /fw/src/ether.c
parent0f922552ad8cc73c0c3c3674d484c3d78dd8c557 (diff)
fw: fix a bypass, a broken round-trip, and eleven others from review
The serious one is that the ctl filter was a blacklist. checkctl looked at connect and announce and passed everything else, but udpctl takes "headers", and udpcreate gives a conversation a live write queue at clone time: c->wq = qbypass(udpkick, c); So three writes -- clone, "headers", a header-prefixed datagram to data -- sent a packet anywhere, with no connect for a rule to match. rudp and icmpv6 have the same verb, gre has raw and forward. none.ndb did not mean "no network at all", though the manual said it did. It is now a whitelist of control messages that cannot reach the network by themselves, which is the argument this code already made about ndb attributes it does not recognise, applied where it was not. Also blocking: %M was never installed, so fmtrules emitted ipmask=%M% and every ctl edit on a rule set containing ip= failed, while save wrote a file reload would reject. Tests had exercised the ctl path and the ip= path but never together. parserules built the new list in the globals with no lock, so for the length of a reload the relay procs walked a list that was empty and then half built -- exactly what installrules' comment promised could not happen. etherwriteip put a 64KB frame on a 32KB proc stack, the same bug design.md records learning and fixing in relay(). putback and notehandler were dead code: atexit matches on the registering pid and _exits never runs the handlers, which is why the cleanup "did not fire" rather than being flaky. Nothing puts the card back, and the docs that said otherwise are corrected. The rest: expired flows kept matching and refreshing themselves; the pkt interface claimed a 4096 MTU from a 1514-byte card; ports were read out of non-first fragments; /net/ndb and /net/log were writable and ipifc/*/data readable through the filter; control files were world-writable, and owning them as a user called "fw" locked out the administrator instead; delete 0 appended a rule reading <nil>; a dead relay left one direction unfiltered with nothing to notice; and IPv6 unicast under -e was dropped in silence when it is simply not implemented. All three modes regression tested after: a namespace refusing headers and port 22 while allowing 443, a card passing https and then blocking it live, and the machine's network restored afterwards. doc/todo.md says which of these were reproduced and which were read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'fw/src/ether.c')
-rw-r--r--fw/src/ether.c24
1 files changed, 22 insertions, 2 deletions
diff --git a/fw/src/ether.c b/fw/src/ether.c
index 54f5068..dd20971 100644
--- a/fw/src/ether.c
+++ b/fw/src/ether.c
@@ -38,6 +38,7 @@ enum
Arplen = 28,
Eminlen = 60, /* ethernet minimum frame, devether enforces it */
+ Maxframe = Ehdrlen + 16*1024,
Arpreq = 1,
Arpreply = 2,
@@ -323,11 +324,16 @@ etherisip(uchar *f, int n)
int
etherwriteip(uchar *p, int n, uchar *mask)
{
- uchar f[Ehdrlen + 64*1024], dst[IPaddrlen], hop[IPaddrlen];
+ /*
+ * On a proc stack, which libthread keeps small: 64K here
+ * overran it and corrupted the data segment. pktmedium's maxtu
+ * is 4096, so this is already generous.
+ */
+ uchar f[Maxframe], dst[IPaddrlen], hop[IPaddrlen];
uchar net[IPaddrlen], ournet[IPaddrlen], mac[Eaddrlen];
int type, len;
- if(n < 20 || n > 64*1024 - Ehdrlen)
+ if(n < 20 || n > Maxframe - Ehdrlen)
return -1;
switch(p[0] >> 4){
case 4:
@@ -355,6 +361,20 @@ etherwriteip(uchar *p, int n, uchar *mask)
ipmove(hop, dst);
if(!arpget(hop, mac)){
+ /*
+ * v6 has no ARP; resolving a neighbour needs ICMPv6
+ * solicitation, which is not implemented. Say so once,
+ * rather than dropping every v6 unicast in silence.
+ */
+ if(!isv4(hop)){
+ static int said;
+
+ if(!said++)
+ fprint(2, "fw: cannot resolve %I: "
+ "IPv6 neighbour discovery is not implemented, "
+ "so v6 unicast is dropped\n", hop);
+ return -1;
+ }
if(etherdebug)
fprint(2, "arp: no entry for %I, dropping and asking\n", hop);
arpask(hop);