diff options
Diffstat (limited to 'fw/src')
| -rw-r--r-- | fw/src/ether.c | 24 | ||||
| -rw-r--r-- | fw/src/fw.c | 89 | ||||
| -rw-r--r-- | fw/src/netfs.c | 67 | ||||
| -rw-r--r-- | fw/src/rules.c | 36 |
4 files changed, 173 insertions, 43 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); diff --git a/fw/src/fw.c b/fw/src/fw.c index 4f94090..a136194 100644 --- a/fw/src/fw.c +++ b/fw/src/fw.c @@ -48,6 +48,7 @@ struct Pkt uchar dst[IPaddrlen]; int sport; int dport; + int frag; /* a later fragment: no ports in it */ int verb; }; @@ -130,6 +131,15 @@ flowlook(int proto, uchar *src, uchar *dst, int sport, int dport, long now) h = flowhash(proto, src, dst, sport, dport); for(f = flowtab[h]; f != nil; f = f->next) if(flowis(f, proto, src, dst, sport, dport)){ + /* + * An expired flow must not match. Reaping only + * happens when a flow is added, so on a quiet + * firewall nothing is ever reaped and a flow that + * timed out long ago would keep passing traffic, + * refreshing itself on every packet. + */ + if(now - f->last > flowtimeout(f->proto)) + return 0; f->last = now; return 1; } @@ -295,6 +305,13 @@ wireup(Wire *w) if(fprint(w->cfd, "bind pkt") < 0) sysfatal("%s: bind pkt: %r", w->net); + /* + * pktmedium claims 4096 with no link header; a card is 1514 with + * 14. Left alone the protected stack emits packets the card + * refuses, and only remote peers capping the MSS hide it. + */ + if(fprint(w->cfd, "mtu 1500") < 0) + fprint(2, "fw: %s: cannot set mtu: %r\n", w->net); if(fprint(w->cfd, "add %s %s", w->addr, w->mask) < 0) sysfatal("%s: add %s %s: %r", w->net, w->addr, w->mask); @@ -333,6 +350,15 @@ parsepkt(uchar *b, int n, Pkt *p) p->proto = b[9]; v4tov6(p->src, b + 12); v4tov6(p->dst, b + 16); + /* + * Only the first fragment carries the transport header. + * Reading one out of a later fragment gives payload bytes + * as ports, which both loses the traffic - it matches no + * flow - and lets a crafted fragment whose bytes happen to + * match an open flow through. + */ + if((nhgets(b + 6) & 0x1FFF) != 0) + p->frag = 1; t = b + hl; n -= hl; break; @@ -349,7 +375,7 @@ parsepkt(uchar *b, int n, Pkt *p) return; } - if((p->proto == 6 || p->proto == 17) && n >= 4){ + if(!p->frag && (p->proto == 6 || p->proto == 17) && n >= 4){ p->sport = nhgets(t); p->dport = nhgets(t + 2); } @@ -382,7 +408,7 @@ relay(Wire *from, Wire *to, int verb) if((n = read(from->dfd, buf, Maxpkt)) <= 0){ fprint(2, "fw: %s: read failed: %r\n", from->side); syslog(0, "fw", "%s: read failed: %r", from->side); - return; + threadexitsall("wire"); } parsepkt(buf, n, &p); @@ -818,8 +844,15 @@ fswrite(Req *r) err = editrules(arg, 1, 0); else if(strcmp(buf, "append") == 0) err = editrules(arg, 0, 0); - else if(strcmp(buf, "delete") == 0) - err = editrules(nil, 0, atoi(arg)); + else if(strcmp(buf, "delete") == 0){ + int nr; + + nr = atoi(arg); + if(nr < 1) + err = "delete wants a rule number, from 1"; + else + err = editrules(nil, 0, nr); + } else err = "unknown command; read ctl for the list"; @@ -854,14 +887,21 @@ rulesdidchange(void) static void servectl(char *mtpt, char *srvname) { + char *user; File *root; - fs.tree = alloctree("fw", "fw", DMDIR|0555, nil); + /* + * Owned by whoever is running fw, not by a user called "fw" that + * does not exist: with 0600 that locked out the administrator as + * effectively as everyone else. + */ + user = getuser(); + fs.tree = alloctree(user, user, DMDIR|0555, nil); root = fs.tree->root; - closefile(createfile(root, "ctl", "fw", 0666, (void*)Qctl)); - closefile(createfile(root, "rules", "fw", 0666, (void*)Qrules)); - closefile(createfile(root, "flows", "fw", 0444, (void*)Qflows)); - closefile(createfile(root, "stats", "fw", 0444, (void*)Qstats)); + closefile(createfile(root, "ctl", user, 0600, (void*)Qctl)); + closefile(createfile(root, "rules", user, 0600, (void*)Qrules)); + closefile(createfile(root, "flows", user, 0440, (void*)Qflows)); + closefile(createfile(root, "stats", user, 0440, (void*)Qstats)); threadpostmountsrv(&fs, srvname, mtpt, MREPL); } @@ -947,9 +987,14 @@ etherin(void *a) buf = emalloc(Maxpkt); for(;;){ if((n = read(efd, buf, Maxpkt)) <= 0){ + /* + * One relay stopping would leave the other running + * and that direction unfiltered, with nothing to + * notice. Take the whole firewall down instead. + */ fprint(2, "fw: %s: read the card: %r\n", w->side); syslog(0, "fw", "stopped reading the card: %r"); - return; + threadexitsall("card"); } if(debug > 1) fprint(2, "wire: %d bytes type %.4ux\n", n, (buf[12]<<8)|buf[13]); @@ -977,7 +1022,7 @@ etherout(void *a) if((n = read(w->dfd, buf, Maxpkt)) <= 0){ fprint(2, "fw: %s: read the stack: %r\n", w->side); syslog(0, "fw", "stopped reading the stack: %r"); - return; + threadexitsall("stack"); } if(permitted(buf, n, Vout, &p)) etherwriteip(buf, n, ethermask); @@ -1057,6 +1102,19 @@ learnaddr(char *net, char *dev, char *addr, int naddr, char *gw, int ngw) * it: a live fw would still be holding the card we are about to take. */ /* + * Undo a previous fw that died. + * + * An IP stack outlives the program that configured it, so a fw that is + * killed leaves its pkt interface behind, holding the address, with + * nothing on the other end of it. The machine has no network until + * someone unpicks that by hand, and the next fw to start makes a second + * interface with the same address and routes that could go to either. + * + * So before taking anything, throw away any pkt interface already + * carrying the address we are about to use. Nothing else can have made + * it: a live fw would still be holding the card we are about to take. + */ +/* * Put the card back. * * Taking a card is destructive: the stack loses it, and the pkt @@ -1248,6 +1306,7 @@ threadmain(int argc, char **argv) fmtinstall('I', eipfmt); fmtinstall('V', eipfmt); fmtinstall('E', eipfmt); + fmtinstall('M', eipfmt); /* fmtrules prints masks with it */ rulepath = argv[0]; readrules(rulepath); @@ -1317,14 +1376,6 @@ threadmain(int argc, char **argv) ethersetaddr(ip, gw, haveg); reclaim(netmtpt, abuf); - backdev = etherdev; - backaddr = abuf; - backmask = m; - backnet = netmtpt; - backgw = ethergw; - atexit(putback); - threadnotify(notehandler, 1); - takecard(netmtpt, etherdev); prot = emalloc(sizeof *prot); diff --git a/fw/src/netfs.c b/fw/src/netfs.c index c87412d..f28485f 100644 --- a/fw/src/netfs.c +++ b/fw/src/netfs.c @@ -121,11 +121,31 @@ hidden(char *path) static char* protect(char *path, int mode) { + char *p; + if(hidden(path)) return "fw: does not exist"; if(strcmp(path, "ipifc/clone") == 0) return "fw: interface creation denied"; - if(under(path, "ipifc") || under(path, "iproute") || under(path, "arp")) + + /* + * An interface's data file is a wire. On a machine that is also + * running fw -e it is *the* wire, so reading it hands a filtered + * program the packet stream the filter exists to control. Not + * read-only: not at all. + */ + if(under(path, "ipifc")) + if((p = strrchr(path, '/')) != nil && strcmp(p+1, "data") == 0) + return "fw: denied"; + + /* + * ndb is the machine's configuration and log is a trace of every + * connection on it, both mode 0666. Writing the first + * reconfigures name service for everyone; reading the second + * after enabling it watches the whole machine. + */ + if(under(path, "ipifc") || under(path, "iproute") || under(path, "arp") + || under(path, "ndb") || under(path, "log")) if((mode & 3) != OREAD) return "fw: read-only under fw"; return nil; @@ -181,6 +201,22 @@ ctlproto(char *path) * connect takes addr!port with optional trailing fields; announce takes * a bare port, or addr!port with addr often "*". */ +/* + * Control messages that only change how this one conversation behaves, + * and so cannot reach the network by themselves. Everything outside + * this list is refused: see checkctl. + */ +static char *okverbs[] = { + "bind", /* the local address; announce is what opens */ + "ttl", + "tos", + "ignoreadvice", + "close", + "hangup", + "keepalive", + nil, +}; + static char* checkctl(char *proto, char *msg, long n) { @@ -188,7 +224,7 @@ checkctl(char *proto, char *msg, long n) static char err[128]; uchar ip[IPaddrlen], mask[IPaddrlen]; Rule *rule; - int nf, na, verb, anyip, port, lport; + int nf, na, verb, anyip, port, lport, i; if(n <= 0) return nil; @@ -203,8 +239,31 @@ checkctl(char *proto, char *msg, long n) verb = Vout; else if(strcmp(f[0], "announce") == 0) verb = Vin; - else - return nil; /* hangup, ttl, keepalive: not policy */ + else{ + /* + * Anything else is refused unless it is known to be + * harmless. Letting unknown control messages through was + * a hole, not a convenience: "headers" on a udp + * conversation turns it into one that carries its own + * destination, and the write queue is live from the + * moment it is cloned, so three writes send a datagram + * anywhere with no connect for a rule to match. gre has + * "raw" and "forward"; rudp and icmpv6 have "headers" + * too. + * + * This is the same argument the rule parser already makes + * about attributes it does not recognise, applied to the + * place it was not. + */ + for(i = 0; okverbs[i] != nil; i++) + if(strcmp(f[0], okverbs[i]) == 0) + return nil; + syslog(0, "fw", "deny %s %s: control message not permitted", + proto, f[0]); + snprint(err, sizeof err, + "fw: %s: not a permitted control message", f[0]); + return err; + } if(nf < 2) return nil; /* malformed; let the kernel say so */ diff --git a/fw/src/rules.c b/fw/src/rules.c index c0df82d..55de9cb 100644 --- a/fw/src/rules.c +++ b/fw/src/rules.c @@ -30,7 +30,6 @@ Rule *rules; void (*rulechanged)(void); static Lock rulelock; -static Rule *lastrule; static char *rulefile; static char *parseerr; static jmp_buf parsejmp; @@ -135,7 +134,7 @@ verbof(char *s) * a firewall gets to do. */ static void -addrule(Ndbtuple *t, int nr) +addrule(Ndbtuple *t, int nr, Rule **head, Rule **tail) { char *ip, *mask, *p, abuf[64]; Rule *r; @@ -206,28 +205,34 @@ addrule(Ndbtuple *t, int nr) r->anyip = 0; } - if(lastrule == nil) - rules = r; + if(*head == nil) + *head = r; else - lastrule->next = r; - lastrule = r; + (*tail)->next = r; + *tail = r; } /* * Parse without installing. Returns the new list, or nil with *err set. * An empty file is a valid rule set: it denies everything. */ +/* + * Build the new list in locals. Earlier this borrowed the globals and + * put them back afterwards, which meant that for the length of a reload + * the relay procs - which take rulelock, a lock this never held - were + * walking a list that was first empty and then half built. Every packet + * in that window was judged against a partial rule set, and a parse + * failure freed nodes a relay might still have been holding. + */ Rule* parserules(char *file, char **err) { - Rule *new, *save, *savelast; + Rule *head, *tail; Ndbtuple *t; Ndb *db; int nr; - save = rules; - savelast = lastrule; - rules = lastrule = nil; + head = tail = nil; rulefile = file; parseerr = nil; @@ -236,30 +241,25 @@ parserules(char *file, char **err) snprint(eb, sizeof eb, "%s: %r", file); *err = eb; - rules = save; - lastrule = savelast; return nil; } parsing = 1; if(setjmp(parsejmp) == 0){ for(nr = 1; (t = ndbparse(db)) != nil; nr++){ - addrule(t, nr); + addrule(t, nr, &head, &tail); ndbfree(t); } } parsing = 0; ndbclose(db); - new = rules; - rules = save; - lastrule = savelast; if(parseerr != nil){ - freerules(new); + freerules(head); *err = parseerr; return nil; } *err = nil; - return new; + return head; } void |
