fw: the gateway relay decides the same way the card does

relay() and permitted() were the same twenty lines twice: parse, work
out which end is the peer, consult the flow table, consult the rules,
log, count, remember.  Two copies that had already drifted -- relay
printed "(state)" and "(new)" under -d and permitted printed neither,
and the two spelled the drop reason differently -- and every fix since
has had to be made in both, which is how the copies drift further.

Now relay reads a packet, asks permitted, and writes it or does not.
The debug line permitted was missing is added rather than dropped, so
-d still distinguishes a packet the flow table let through from one the
rules did.

No new checks: the point is that nothing changes.  The gateway tests
already cover this path -- a connection crossing, one rule serving both
directions, a rule change killing a live flow -- and all 60 pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

1 files changed, 68 insertions, 126 deletions

diff --git a/fw/src/fw.c b/fw/src/fw.c
index ef4aff0..294046d 100644
--- a/fw/src/fw.c
+++ b/fw/src/fw.c
@@ -391,9 +391,73 @@ parsepkt(uchar *b, int n, Pkt *p)
 }
 
 /*
- * One direction.  "verb" says which way this is, and the rule's ip and
- * port always refer to the peer - the far end - so a rule reads the
- * same whichever direction it governs.
+ * Decide on one packet.  Shared by every relay: the wire it came from
+ * only changes which way "in" and "out" mean.
+ */
+static int
+permitted(uchar *buf, int n, int verb, Pkt *p)
+{
+	Match m;
+
+	parsepkt(buf, n, p);
+	if(!p->ok){
+		ndeny++;
+		if(debug)
+			fprint(2, "drop %s: unparseable, %d bytes\n",
+				verb == Vout ? "out" : "in", n);
+		return 0;
+	}
+	p->verb = verb;
+	memset(&m, 0, sizeof m);
+	m.count = 1;
+	m.verb = verb;
+	m.proto = protonum2name(p->proto);
+	if(verb == Vout){
+		m.ip = p->dst;
+		m.port = p->dport;
+		m.lport = p->sport;
+	}else{
+		m.ip = p->src;
+		m.port = p->sport;
+		m.lport = p->dport;
+	}
+	if(flowseen(p)){
+		nallow++;
+		if(debug)
+			fprint(2, "pass %s %s %I!%d -> %I!%d (state)\n",
+				verb == Vout ? "out" : "in", protonum2name(p->proto),
+				p->src, p->sport, p->dst, p->dport);
+		return 1;
+	}
+	if(!matchrule(&m)){
+		ndeny++;
+		if(m.log)
+			syslog(0, "fw", "drop %s %s %I!%d -> %I!%d: %s",
+				verb == Vout ? "out" : "in", protonum2name(p->proto),
+				p->src, p->sport, p->dst, p->dport, m.err);
+		if(debug)
+			fprint(2, "drop %s %s %I!%d -> %I!%d: %s\n",
+				verb == Vout ? "out" : "in", protonum2name(p->proto),
+				p->src, p->sport, p->dst, p->dport, m.err);
+		return 0;
+	}
+	nallow++;
+	if(m.log)
+		syslog(0, "fw", "pass %s %s %I!%d -> %I!%d",
+			verb == Vout ? "out" : "in", protonum2name(p->proto),
+			p->src, p->sport, p->dst, p->dport);
+	if(debug)
+		fprint(2, "pass %s %s %I!%d -> %I!%d\n",
+			verb == Vout ? "out" : "in", protonum2name(p->proto),
+			p->src, p->sport, p->dst, p->dport);
+	flowadd(p);
+	return 1;
+}
+
+/*
+ * One direction.  "verb" says which way this is; permitted() takes it
+ * from there, so the wire the packet came from only decides which way
+ * "in" and "out" mean and where the packet goes if it may go at all.
  */
 int revalidate(void);
 
@@ -401,7 +465,6 @@ static void
 relay(Wire *from, Wire *to, int verb)
 {
 	uchar *buf;
-	Match m;
 	Pkt p;
 	int n;
 
@@ -416,69 +479,8 @@ relay(Wire *from, Wire *to, int verb)
 			syslog(0, "fw", "%s: read failed: %r", from->side);
 			threadexitsall("wire");
 		}
-
-		parsepkt(buf, n, &p);
-		if(!p.ok){
-			ndeny++;
-			if(debug)
-				fprint(2, "drop %s: unparseable, %d bytes\n", from->side, n);
-			continue;
-		}
-
-		p.verb = verb;
-		memset(&m, 0, sizeof m);
-		m.count = 1;
-		m.verb = verb;
-		m.proto = protonum2name(p.proto);
-		if(verb == Vout){
-			m.ip = p.dst;
-			m.port = p.dport;
-			m.lport = p.sport;
-		}else{
-			m.ip = p.src;
-			m.port = p.sport;
-			m.lport = p.dport;
-		}
-
-		if(flowseen(&p)){
-			nallow++;
-			if(debug)
-				fprint(2, "pass %s %s %I!%d -> %I!%d (state)\n",
-					verb == Vout ? "out" : "in",
-					protonum2name(p.proto),
-					p.src, p.sport, p.dst, p.dport);
-			if(write(to->dfd, buf, n) != n)
-				fprint(2, "fw: write %s wire: %r\n", to->side);
+		if(!permitted(buf, n, verb, &p))
 			continue;
-		}
-
-		if(!matchrule(&m)){
-			if(m.log)
-				syslog(0, "fw", "drop %s %s %I!%d -> %I!%d: %s",
-					verb == Vout ? "out" : "in",
-					protonum2name(p.proto),
-					p.src, p.sport, p.dst, p.dport, m.err);
-			ndeny++;
-			if(debug)
-				fprint(2, "drop %s %s %I!%d -> %I!%d: %s\n",
-					verb == Vout ? "out" : "in",
-					protonum2name(p.proto),
-					p.src, p.sport, p.dst, p.dport, m.err);
-			continue;
-		}
-
-		nallow++;
-		if(m.log)
-			syslog(0, "fw", "pass %s %s %I!%d -> %I!%d",
-				verb == Vout ? "out" : "in",
-				protonum2name(p.proto),
-				p.src, p.sport, p.dst, p.dport);
-		flowadd(&p);
-		if(debug)
-			fprint(2, "pass %s %s %I!%d -> %I!%d (new)\n",
-				verb == Vout ? "out" : "in",
-				protonum2name(p.proto),
-				p.src, p.sport, p.dst, p.dport);
 		if(write(to->dfd, buf, n) != n)
 			fprint(2, "fw: write %s wire: %r\n", to->side);
 	}
@@ -928,66 +930,6 @@ relayproc(void *a)
 	relay(w[0], w[1], (int)(uintptr)w[2]);
 }
 
-/*
- * Decide on one packet.  Shared by every relay: the wire it came from
- * only changes which way "in" and "out" mean.
- */
-static int
-permitted(uchar *buf, int n, int verb, Pkt *p)
-{
-	Match m;
-
-	parsepkt(buf, n, p);
-	if(!p->ok){
-		ndeny++;
-		if(debug)
-			fprint(2, "drop %s: unparseable, %d bytes\n",
-				verb == Vout ? "out" : "in", n);
-		return 0;
-	}
-	p->verb = verb;
-	memset(&m, 0, sizeof m);
-	m.count = 1;
-	m.verb = verb;
-	m.proto = protonum2name(p->proto);
-	if(verb == Vout){
-		m.ip = p->dst;
-		m.port = p->dport;
-		m.lport = p->sport;
-	}else{
-		m.ip = p->src;
-		m.port = p->sport;
-		m.lport = p->dport;
-	}
-	if(flowseen(p)){
-		nallow++;
-		return 1;
-	}
-	if(!matchrule(&m)){
-		ndeny++;
-		if(m.log)
-			syslog(0, "fw", "drop %s %s %I!%d -> %I!%d: %s",
-				verb == Vout ? "out" : "in", protonum2name(p->proto),
-				p->src, p->sport, p->dst, p->dport, m.err);
-		if(debug)
-			fprint(2, "drop %s %s %I!%d -> %I!%d: %s\n",
-				verb == Vout ? "out" : "in", protonum2name(p->proto),
-				p->src, p->sport, p->dst, p->dport, m.err);
-		return 0;
-	}
-	nallow++;
-	if(m.log)
-		syslog(0, "fw", "pass %s %s %I!%d -> %I!%d",
-			verb == Vout ? "out" : "in", protonum2name(p->proto),
-			p->src, p->sport, p->dst, p->dport);
-	if(debug)
-		fprint(2, "pass %s %s %I!%d -> %I!%d\n",
-			verb == Vout ? "out" : "in", protonum2name(p->proto),
-			p->src, p->sport, p->dst, p->dport);
-	flowadd(p);
-	return 1;
-}
-
 /* the wire -> the protected stack */
 static void
 etherin(void *a)