o Overhaul filtering, adding facilities to jump over rules and to

  negate the sense of rules.
o Remove the redundant (and undocumented) ``host'' and ``port''
  words (README.changes updated).
o Don't permit (and ignore) garbage instead of the protocol.

Mostly submitted by:  Peter Jeremy <jeremyp@gsmx07.alcatel.com.au>

1 files changed, 182 insertions, 131 deletions

diff --git a/usr.sbin/ppp/ip.c b/usr.sbin/ppp/ip.c
index a674e43..8ea2e73 100644
--- a/usr.sbin/ppp/ip.c
+++ b/usr.sbin/ppp/ip.c
@@ -17,7 +17,7 @@
  * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
  * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  *
- * $Id: ip.c,v 1.63 1999/06/02 15:59:00 brian Exp $
+ * $Id: ip.c,v 1.64 1999/06/23 16:48:23 brian Exp $
  *
  *	TODO:
  *		o Return ICMP message for filterd packet
@@ -79,11 +79,11 @@ static const u_short interactive_ports[32] = {
 
 static const char *TcpFlags[] = { "FIN", "SYN", "RST", "PSH", "ACK", "URG" };
 
-static int
+static __inline int
 PortMatch(int op, u_short pport, u_short rport)
 {
   switch (op) {
-    case OP_EQ:
+  case OP_EQ:
     return (pport == rport);
   case OP_GT:
     return (pport > rport);
@@ -96,142 +96,193 @@ PortMatch(int op, u_short pport, u_short rport)
 
 /*
  *  Check a packet against a defined filter
+ *  Returns 0 to accept the packet, non-zero to drop the packet
+ *
+ *  If filtering is enabled, the initial fragment of a datagram must
+ *  contain the complete protocol header, and subsequent fragments
+ *  must not attempt to over-write it.
  */
 static int
-FilterCheck(struct ip *pip, struct filter *filter)
+FilterCheck(const struct ip *pip, const struct filter *filter)
 {
-  int gotinfo, cproto, estab, syn, finrst, n, len, didname;
-  struct tcphdr *th;
-  struct udphdr *uh;
-  struct icmp *ih;
-  char *ptop;
-  u_short sport, dport;
-  struct filterent *fp = filter->rule;
+  int gotinfo;			/* true if IP payload decoded */
+  int cproto;			/* P_* protocol type if (gotinfo) */
+  int estab, syn, finrst;	/* TCP state flags if (gotinfo) */
+  u_short sport, dport;		/* src, dest port from packet if (gotinfo) */
+  int n;			/* filter rule to process */
+  int len;			/* bytes used in dbuff */
+  int didname;			/* true if filter header printed */
+  int match;			/* true if condition matched */
+  const struct filterent *fp = filter->rule;
   char dbuff[100];
 
-  if (fp->action) {
-    cproto = gotinfo = estab = syn = finrst = didname = 0;
-    sport = dport = 0;
-    for (n = 0; n < MAXFILTERS; n++) {
-      if (fp->action != A_NONE) {
-	/* permit fragments on in and out filter */
-        if (filter->fragok && (ntohs(pip->ip_off) & IP_OFFMASK) != 0)
-	  return (A_PERMIT);
-
-        if (!didname)
-          log_Printf(LogDEBUG, "%s filter:\n", filter->name);
-        didname = 1;
-
-	if ((pip->ip_src.s_addr & fp->src.mask.s_addr) ==
-	    (fp->src.ipaddr.s_addr & fp->src.mask.s_addr) &&
-	    (pip->ip_dst.s_addr & fp->dst.mask.s_addr) ==
-	    (fp->dst.ipaddr.s_addr & fp->dst.mask.s_addr)) {
-	  if (fp->proto) {
-	    if (!gotinfo) {
-	      ptop = (char *) pip + (pip->ip_hl << 2);
-
-	      switch (pip->ip_p) {
-	      case IPPROTO_ICMP:
-		cproto = P_ICMP;
-		ih = (struct icmp *) ptop;
-		sport = ih->icmp_type;
-		estab = syn = finrst = -1;
-                if (log_IsKept(LogDEBUG))
-		  snprintf(dbuff, sizeof dbuff, "sport = %d", sport);
-		break;
-	      case IPPROTO_IGMP:
-		cproto = P_IGMP;
-		estab = syn = finrst = -1;
-		sport = ntohs(0);
-		break;
-	      case IPPROTO_UDP:
-	      case IPPROTO_IPIP:
-		cproto = P_UDP;
-		uh = (struct udphdr *) ptop;
-		sport = ntohs(uh->uh_sport);
-		dport = ntohs(uh->uh_dport);
-		estab = syn = finrst = -1;
-                if (log_IsKept(LogDEBUG))
-		  snprintf(dbuff, sizeof dbuff, "sport = %d, dport = %d",
-                           sport, dport);
-		break;
-	      case IPPROTO_TCP:
-		cproto = P_TCP;
-		th = (struct tcphdr *) ptop;
-		sport = ntohs(th->th_sport);
-		dport = ntohs(th->th_dport);
-		estab = (th->th_flags & TH_ACK);
-		syn = (th->th_flags & TH_SYN);
-		finrst = (th->th_flags & (TH_FIN|TH_RST));
-                if (log_IsKept(LogDEBUG)) {
-                  if (!estab)
-		    snprintf(dbuff, sizeof dbuff,
-                             "flags = %02x, sport = %d, dport = %d",
-                             th->th_flags, sport, dport);
-                  else
-                    *dbuff = '\0';
-                }
-		break;
-	      default:
-		return (A_DENY);       /* We'll block unknown type of packet */
-	      }
-              if (log_IsKept(LogDEBUG)) {
-                if (estab != -1) {
-                  len = strlen(dbuff);
-                  snprintf(dbuff + len, sizeof dbuff - len,
-                           ", estab = %d, syn = %d, finrst = %d",
-                           estab, syn, finrst);
-                }
-	        log_Printf(LogDEBUG, " Filter: proto = %s, %s\n",
-                          filter_Proto2Nam(cproto), dbuff);
-              }
-	      gotinfo = 1;
+  if (fp->f_action == A_NONE)
+    return (0);		/* No rule is given. Permit this packet */
+
+  /* Deny any packet fragment that tries to over-write the header.
+   * Since we no longer have the real header available, punt on the
+   * largest normal header - 20 bytes for TCP without options, rounded
+   * up to the next possible fragment boundary.  Since the smallest
+   * `legal' MTU is 576, and the smallest recommended MTU is 296, any
+   * fragmentation within this range is dubious at best */
+  len = ntohs(pip->ip_off) & IP_OFFMASK;	/* fragment offset */ 
+  if (len > 0) {		/* Not first fragment within datagram */
+    if (len < (24 >> 3))	/* don't allow fragment to over-write header */
+      return (1);
+    /* permit fragments on in and out filter */
+    return (filter->fragok);
+  }
+  
+  cproto = gotinfo = estab = syn = finrst = didname = 0;
+  sport = dport = 0;
+  for (n = 0; n < MAXFILTERS; ) {
+    if (fp->f_action == A_NONE) {
+      n++;
+      fp++;
+      continue;
+    }
+
+    if (!didname) {
+      log_Printf(LogDEBUG, "%s filter:\n", filter->name);
+      didname = 1;
+    }
+
+    match = 0;
+    if (!((pip->ip_src.s_addr ^ fp->f_src.ipaddr.s_addr) &
+	  fp->f_src.mask.s_addr) &&
+	!((pip->ip_dst.s_addr ^ fp->f_dst.ipaddr.s_addr) &
+	  fp->f_dst.mask.s_addr)) {
+      if (fp->f_proto != P_NONE) {
+	if (!gotinfo) {
+	  const char *ptop = (const char *) pip + (pip->ip_hl << 2);
+	  const struct tcphdr *th;
+	  const struct udphdr *uh;
+	  const struct icmp *ih;
+	  int datalen;	/* IP datagram length */
+
+	  datalen = ntohs(pip->ip_len) - (pip->ip_hl << 2);
+	  switch (pip->ip_p) {
+	  case IPPROTO_ICMP:
+	    cproto = P_ICMP;
+	    if (datalen < 8)	/* ICMP must be at least 8 octets */
+	      return (1);
+	    ih = (const struct icmp *) ptop;
+	    sport = ih->icmp_type;
+	    estab = syn = finrst = -1;
+	    if (log_IsKept(LogDEBUG))
+	      snprintf(dbuff, sizeof dbuff, "sport = %d", sport);
+	    break;
+	  case IPPROTO_IGMP:
+	    cproto = P_IGMP;
+	    if (datalen < 8)	/* IGMP uses 8-octet messages */
+	      return (1);
+	    estab = syn = finrst = -1;
+	    sport = ntohs(0);
+	    break;
+	  case IPPROTO_UDP:
+	  case IPPROTO_IPIP:
+	    cproto = P_UDP;
+	    if (datalen < 8)	/* UDP header is 8 octets */
+	      return (1);
+	    uh = (const struct udphdr *) ptop;
+	    sport = ntohs(uh->uh_sport);
+	    dport = ntohs(uh->uh_dport);
+	    estab = syn = finrst = -1;
+	    if (log_IsKept(LogDEBUG))
+	      snprintf(dbuff, sizeof dbuff, "sport = %d, dport = %d",
+		       sport, dport);
+	    break;
+	  case IPPROTO_TCP:
+	    cproto = P_TCP;
+	    th = (const struct tcphdr *) ptop;
+	    /* TCP headers are variable length.  The following code
+	     * ensures that the TCP header length isn't de-referenced if
+	     * the datagram is too short
+	     */
+	    if (datalen < 20 || datalen < (th->th_off << 2))
+	      return (1);
+	    sport = ntohs(th->th_sport);
+	    dport = ntohs(th->th_dport);
+	    estab = (th->th_flags & TH_ACK);
+	    syn = (th->th_flags & TH_SYN);
+	    finrst = (th->th_flags & (TH_FIN|TH_RST));
+	    if (log_IsKept(LogDEBUG)) {
+	      if (!estab)
+		snprintf(dbuff, sizeof dbuff,
+			 "flags = %02x, sport = %d, dport = %d",
+			 th->th_flags, sport, dport);
+	      else
+		*dbuff = '\0';
 	    }
-            if (log_IsKept(LogDEBUG)) {
-	      if (fp->opt.srcop != OP_NONE) {
-                snprintf(dbuff, sizeof dbuff, ", src %s %d",
-                         filter_Op2Nam(fp->opt.srcop), fp->opt.srcport);
-                len = strlen(dbuff);
-              } else
-                len = 0;
-	      if (fp->opt.dstop != OP_NONE) {
-                snprintf(dbuff + len, sizeof dbuff - len,
-                         ", dst %s %d", filter_Op2Nam(fp->opt.dstop),
-                         fp->opt.dstport);
-              } else if (!len)
-                *dbuff = '\0';
-
-	      log_Printf(LogDEBUG, "  rule = %d: Address match, "
-                        "check against proto %s%s, action = %s\n",
-                        n, filter_Proto2Nam(fp->proto),
-                        dbuff, filter_Action2Nam(fp->action));
-            }
-
-	    if (cproto == fp->proto) {
-	      if ((fp->opt.srcop == OP_NONE ||
-		   PortMatch(fp->opt.srcop, sport, fp->opt.srcport)) &&
-		  (fp->opt.dstop == OP_NONE ||
-		   PortMatch(fp->opt.dstop, dport, fp->opt.dstport)) &&
-		  (fp->opt.estab == 0 || estab) &&
-		  (fp->opt.syn == 0 || syn) &&
-		  (fp->opt.finrst == 0 || finrst)) {
-		return (fp->action);
-	      }
+	    break;
+	  default:
+	    return (1);	/* We'll block unknown type of packet */
+	  }
+
+	  if (log_IsKept(LogDEBUG)) {
+	    if (estab != -1) {
+	      len = strlen(dbuff);
+	      snprintf(dbuff + len, sizeof dbuff - len,
+		       ", estab = %d, syn = %d, finrst = %d",
+		       estab, syn, finrst);
 	    }
-	  } else {
-	    /* Address is mached. Make a decision. */
-	    log_Printf(LogDEBUG, "  rule = %d: Address match, action = %s\n", n,
-                      filter_Action2Nam(fp->action));
-	    return (fp->action);
+	    log_Printf(LogDEBUG, " Filter: proto = %s, %s\n",
+		       filter_Proto2Nam(cproto), dbuff);
+	  }
+	  gotinfo = 1;
+	}
+	if (log_IsKept(LogDEBUG)) {
+	  if (fp->f_srcop != OP_NONE) {
+	    snprintf(dbuff, sizeof dbuff, ", src %s %d",
+		     filter_Op2Nam(fp->f_srcop), fp->f_srcport);
+	    len = strlen(dbuff);
+	  } else
+	    len = 0;
+	  if (fp->f_dstop != OP_NONE) {
+	    snprintf(dbuff + len, sizeof dbuff - len,
+		     ", dst %s %d", filter_Op2Nam(fp->f_dstop),
+		     fp->f_dstport);
+	  } else if (!len)
+	    *dbuff = '\0';
+
+	  log_Printf(LogDEBUG, "  rule = %d: Address match, "
+		     "check against proto %s%s, action = %s\n",
+		     n, filter_Proto2Nam(fp->f_proto),
+		     dbuff, filter_Action2Nam(fp->f_action));
+	}
+
+	if (cproto == fp->f_proto) {
+	  if ((fp->f_srcop == OP_NONE ||
+	       PortMatch(fp->f_srcop, sport, fp->f_srcport)) &&
+	      (fp->f_dstop == OP_NONE ||
+	       PortMatch(fp->f_dstop, dport, fp->f_dstport)) &&
+	      (fp->f_estab == 0 || estab) &&
+	      (fp->f_syn == 0 || syn) &&
+	      (fp->f_finrst == 0 || finrst)) {
+	    match = 1;
 	  }
-	} else
-	  log_Printf(LogDEBUG, "  rule = %d: Address mismatch\n", n);
+	}
+      } else {
+	/* Address is matched and no protocol specified. Make a decision. */
+	log_Printf(LogDEBUG, "  rule = %d: Address match, action = %s\n", n,
+		   filter_Action2Nam(fp->f_action));
+	match = 1;
       }
+    } else
+      log_Printf(LogDEBUG, "  rule = %d: Address mismatch\n", n);
+
+    if (match != fp->f_invert) {
+      /* Take specified action */
+      if (fp->f_action < A_NONE)
+	fp = &filter->rule[n = fp->f_action];
+      else
+	return (fp->f_action != A_PERMIT);
+    } else {
+      n++;
       fp++;
     }
-    return (A_DENY);		/* No rule is mached. Deny this packet */
   }
-  return (A_PERMIT);		/* No rule is given. Permit this packet */
+  return (1);		/* No rule is mached. Deny this packet */
 }
 
 #ifdef notdef
@@ -364,7 +415,7 @@ PacketCheck(struct bundle *bundle, char *cp, int nb, struct filter *filter)
     break;
   }
 
-  if ((FilterCheck(pip, filter) & A_DENY)) {
+  if (FilterCheck(pip, filter)) {
     if (logit)
       log_Printf(LogTCPIP, "%s - BLOCKED\n", logbuf);
 #ifdef notdef
@@ -375,7 +426,7 @@ PacketCheck(struct bundle *bundle, char *cp, int nb, struct filter *filter)
   } else {
     /* Check Keep Alive filter */
     if (logit) {
-      if (FilterCheck(pip, &bundle->filter.alive) & A_DENY)
+      if (FilterCheck(pip, &bundle->filter.alive))
         log_Printf(LogTCPIP, "%s - NO KEEPALIVE\n", logbuf);
       else
         log_Printf(LogTCPIP, "%s\n", logbuf);
@@ -412,7 +463,7 @@ ip_Input(struct bundle *bundle, struct link *l, struct mbuf *bp)
     return NULL;
 
   pip = (struct ip *)tun.data;
-  if (!(FilterCheck(pip, &bundle->filter.alive) & A_DENY))
+  if (!FilterCheck(pip, &bundle->filter.alive))
     bundle_StartIdleTimer(bundle);
 
   ipcp_AddInOctets(&bundle->ncp.ipcp, nb);
@@ -491,7 +542,7 @@ ip_PushPacket(struct link *l, struct bundle *bundle)
       bp = mbuf_Contiguous(mbuf_Dequeue(queue));
       cnt = mbuf_Length(bp);
       pip = (struct ip *)MBUF_CTOP(bp);
-      if (!(FilterCheck(pip, &bundle->filter.alive) & A_DENY))
+      if (!FilterCheck(pip, &bundle->filter.alive))
         bundle_StartIdleTimer(bundle);
       link_PushPacket(l, bp, bundle, PRI_NORMAL, PROTO_IP);
       ipcp_AddOutOctets(ipcp, cnt);