Update ipfilter from 3.4.31 -> 3.4.35. Some important changes:

* block packets that fail to create state table entries
* only allow non-fragmented packets to influence whether or not a logged
  packet is the same as the one logged before.
* correct the ICMP packet checksum fixing up when processing ICMP errors for NAT
* implement a maximum for the number of entries in the NAT table (NAT_TABLE_MAX
  and ipf_nattable_max)
* frsynclist() wasn't paying attention to all the places where interface
  names are, like it should.
* fix comparing ICMP packets with established TCP state where only 8 bytes
  of header are returned in the ICMP error.

MFC after:	1 week

1 files changed, 83 insertions, 51 deletions

diff --git a/sys/contrib/ipfilter/netinet/ip_state.c b/sys/contrib/ipfilter/netinet/ip_state.c
index b443e46..ef19b5e 100644
--- a/sys/contrib/ipfilter/netinet/ip_state.c
+++ b/sys/contrib/ipfilter/netinet/ip_state.c
@@ -923,7 +923,8 @@ tcphdr_t *tcp;
 		fdata->td_wscale = wscale;
 	else if (wscale == -2)
 		fdata->td_wscale = tdata->td_wscale = 0;
-	win <<= fdata->td_wscale;
+	if (!(tcp->th_flags & TH_SYN))
+		win <<= fdata->td_wscale;
 
 	if ((fdata->td_end == 0) &&
 	    (!is->is_fsm || ((tcp->th_flags & TH_OPENING) == TH_OPENING))) {
@@ -957,14 +958,15 @@ tcphdr_t *tcp;
 	    (SEQ_GE(seq, fdata->td_end - maxwin)) &&
 /* XXX what about big packets */
 #define MAXACKWINDOW 66000
-	    (ackskew >= -MAXACKWINDOW) &&
-	    (ackskew <= MAXACKWINDOW)) {
-		/* if ackskew < 0 then this should be due to fragented
+	    (-ackskew <= (MAXACKWINDOW << tdata->td_wscale)) &&
+	    ( ackskew <= (MAXACKWINDOW << tdata->td_wscale))) {
+
+		/* if ackskew < 0 then this should be due to fragmented
 		 * packets. There is no way to know the length of the
 		 * total packet in advance.
 		 * We do know the total length from the fragment cache though.
 		 * Note however that there might be more sessions with
-		 * exactly the same source and destination paramters in the
+		 * exactly the same source and destination parameters in the
 		 * state cache (and source and destination is the only stuff
 		 * that is saved in the fragment cache). Note further that
 		 * some TCP connections in the state cache are hashed with
@@ -1211,6 +1213,10 @@ fr_info_t *fin;
 
 	oip = (ip_t *)((char *)ic + ICMPERR_ICMPHLEN);
 	ohlen = oip->ip_hl << 2;
+	/*
+	 * Check if the at least the old IP header (with options) and
+	 * 8 bytes of payload is present.
+	 */
 	if (fin->fin_plen < ICMPERR_MAXPKTLEN + ohlen - sizeof(*oip))
 		return NULL;
 
@@ -1227,7 +1233,7 @@ fr_info_t *fin;
 	 * may be too big to be in this buffer but not so big that it's
 	 * outside the ICMP packet, leading to TCP deref's causing problems.
 	 * This is possible because we don't know how big oip_hl is when we
-	 * do the pullup early in fr_check() and thus can't gaurantee it is
+	 * do the pullup early in fr_check() and thus can't guarantee it is
 	 * all here now.
 	 */
 #ifdef  _KERNEL
@@ -1254,9 +1260,43 @@ fr_info_t *fin;
 	bzero((char *)&src, sizeof(src));
 	bzero((char *)&dst, sizeof(dst));
 	bzero((char *)&ofin, sizeof(ofin));
+	/*
+	 * We make an fin entry to be able to feed it to
+	 * matchsrcdst.  Note that not all fields are encessary
+	 * but this is the cleanest way. Note further that we
+	 * fill in fin_mp such that if someone uses it we'll get
+	 * a kernel panic. fr_matchsrcdst does not use this.
+	 */
 	ofin.fin_ifp = fin->fin_ifp;
 	ofin.fin_out = !fin->fin_out;
+	ofin.fin_mp = NULL;
 	ofin.fin_v = 4;
+	/*
+	 * watch out here, as ip is in host order and oip in network
+	 * order. Any change we make must be undone afterwards, like
+	 * oip->ip_off - it is still in network byte order so fix it.
+	 */
+	savelen = oip->ip_len;
+	oip->ip_len = len;
+	oip->ip_off = ntohs(oip->ip_off);
+	(void) fr_makefrip(ohlen, oip, &ofin);
+	/*
+	 * Reset the short flag here because in fr_matchsrcdst() the flags
+	 * for the current packet (fin_fl) are compared against * those for
+	 * the existing session.
+	 */
+	ofin.fin_fl &= ~FI_SHORT;
+
+	/*
+	 * Put old values of ip_len and ip_off back as we don't know
+	 * if we have to forward the packet (or process it again.
+	 */
+	oip->ip_len = savelen;
+	oip->ip_off = htons(oip->ip_off);
+
+#if SOLARIS
+	ofin.fin_qfm = NULL;
+#endif
 	fr = NULL;
 
 	switch (oip->ip_p)
@@ -1265,7 +1305,7 @@ fr_info_t *fin;
 		icmp = (icmphdr_t *)((char *)oip + ohlen);
 
 		/*
-		 * a ICMP error can only be generated as a result of an
+		 * an ICMP error can only be generated as a result of an
 		 * ICMP query, not as the response on an ICMP error
 		 *
 		 * XXX theoretically ICMP_ECHOREP and the other reply's are
@@ -1289,18 +1329,15 @@ fr_info_t *fin;
 		hv += icmp->icmp_seq;
 		hv %= fr_statesize;
 
-		savelen = oip->ip_len;
-		oip->ip_len = len;
-		fr_makefrip(ohlen, oip, &ofin);
-		oip->ip_len = savelen;
-
 		READ_ENTER(&ipf_state);
 		for (isp = &ips_table[hv]; (is = *isp); isp = &is->is_hnext)
 			if ((is->is_p == pr) && (is->is_v == 4) &&
+			    (is->is_icmppkts < is->is_pkts) &&
 			    fr_matchsrcdst(is, src, dst, &ofin, NULL) &&
-			    fr_matchicmpqueryreply(is->is_v, is, icmp, fin->fin_rev)) {
+			    fr_matchicmpqueryreply(is->is_v, is, icmp,
+						   fin->fin_rev)) {
 				ips_stats.iss_hits++;
-				is->is_pkts++;
+				is->is_icmppkts++;
 				is->is_bytes += ip->ip_len;
 				fr = is->is_rule;
 				break;
@@ -1329,20 +1366,7 @@ fr_info_t *fin;
 	hv += dport;
 	hv += sport;
 	hv %= fr_statesize;
-	/*
-	 * we make an fin entry to be able to feed it to
-	 * matchsrcdst note that not all fields are encessary
-	 * but this is the cleanest way. Note further we fill
-	 * in fin_mp such that if someone uses it we'll get
-	 * a kernel panic. fr_matchsrcdst does not use this.
-	 *
-	 * watch out here, as ip is in host order and oip in network
-	 * order. Any change we make must be undone afterwards.
-	 */
-	savelen = oip->ip_len;
-	oip->ip_len = len;
-	fr_makefrip(ohlen, oip, &ofin);
-	oip->ip_len = savelen;
+
 	READ_ENTER(&ipf_state);
 	for (isp = &ips_table[hv]; (is = *isp); isp = &is->is_hnext) {
 		/*
@@ -1350,13 +1374,16 @@ fr_info_t *fin;
 		 * encapsulated packet was allowed through the
 		 * other way around. Note that the minimal amount
 		 * of info present does not allow for checking against
-		 * tcp internals such as seq and ack numbers.
+		 * tcp internals such as seq and ack numbers.  Only the
+		 * ports are known to be present and can be even if the
+		 * short flag is set.
 		 */
 		if ((is->is_p == pr) && (is->is_v == 4) &&
+		    (is->is_icmppkts < is->is_pkts) &&
 		    fr_matchsrcdst(is, src, dst, &ofin, tcp)) {
 			fr = is->is_rule;
 			ips_stats.iss_hits++;
-			is->is_pkts++;
+			is->is_icmppkts++;
 			is->is_bytes += fin->fin_plen;
 			/*
 			 * we deliberately do not touch the timeouts
@@ -2083,7 +2110,7 @@ u_int type;
 	int types[1];
 
 	ipsl.isl_type = type;
-	ipsl.isl_pkts = is->is_pkts;
+	ipsl.isl_pkts = is->is_pkts + is->is_icmppkts;
 	ipsl.isl_bytes = is->is_bytes;
 	ipsl.isl_src = is->is_src;
 	ipsl.isl_dst = is->is_dst;
@@ -2111,7 +2138,11 @@ u_int type;
 	sizes[0] = sizeof(ipsl);
 	types[0] = 0;
 
-	(void) ipllog(IPL_LOGSTATE, NULL, items, sizes, types, 1);
+	if (ipllog(IPL_LOGSTATE, NULL, items, sizes, types, 1)) {
+		ATOMIC_INCL(ips_stats.iss_logged);
+	} else {
+		ATOMIC_INCL(ips_stats.iss_logfail);
+	}
 }
 #endif
 
@@ -2161,12 +2192,30 @@ fr_info_t *fin;
 	bzero((char *)&ofin, sizeof(ofin));
 	ofin.fin_out = !fin->fin_out;
 	ofin.fin_ifp = fin->fin_ifp;
+	ofin.fin_mp = NULL;
 	ofin.fin_v = 6;
+#if SOLARIS
+	ofin.fin_qfm = NULL;
+#endif
+	/*
+	 * We make a fin entry to be able to feed it to
+	 * matchsrcdst. Note that not all fields are necessary
+	 * but this is the cleanest way. Note further we fill
+	 * in fin_mp such that if someone uses it we'll get
+	 * a kernel panic. fr_matchsrcdst does not use this.
+	 *
+	 * watch out here, as ip is in host order and oip in network
+	 * order. Any change we make must be undone afterwards.
+	 */
+	savelen = oip->ip6_plen;
+	oip->ip6_plen = ip->ip6_plen - sizeof(*ip) - ICMPERR_ICMPHLEN;
+	fr_makefrip(sizeof(*oip), (ip_t *)oip, &ofin);
+	oip->ip6_plen = savelen;
 
 	if (oip->ip6_nxt == IPPROTO_ICMPV6) {
 		oic = (struct icmp6_hdr *)(oip + 1);
 		/*
-		 * a ICMP error can only be generated as a result of an
+		 * an ICMP error can only be generated as a result of an
 		 * ICMP query, not as the response on an ICMP error
 		 *
 		 * XXX theoretically ICMP_ECHOREP and the other reply's are
@@ -2187,10 +2236,6 @@ fr_info_t *fin;
 		hv += oic->icmp6_seq;
 		hv %= fr_statesize;
 
-		oip->ip6_plen = ntohs(oip->ip6_plen);
-		fr_makefrip(sizeof(*oip), (ip_t *)oip, &ofin);
-		oip->ip6_plen = htons(oip->ip6_plen);
-
 		READ_ENTER(&ipf_state);
 		for (isp = &ips_table[hv]; (is = *isp); isp = &is->is_hnext)
 			if ((is->is_p == pr) &&
@@ -2234,20 +2279,7 @@ fr_info_t *fin;
 	hv += dport;
 	hv += sport;
 	hv %= fr_statesize;
-	/*
-	 * we make an fin entry to be able to feed it to
-	 * matchsrcdst note that not all fields are encessary
-	 * but this is the cleanest way. Note further we fill
-	 * in fin_mp such that if someone uses it we'll get
-	 * a kernel panic. fr_matchsrcdst does not use this.
-	 *
-	 * watch out here, as ip is in host order and oip in network
-	 * order. Any change we make must be undone afterwards.
-	 */
-	savelen = oip->ip6_plen;
-	oip->ip6_plen = ip->ip6_plen - sizeof(*ip) - ICMPERR_ICMPHLEN;
-	fr_makefrip(sizeof(*oip), (ip_t *)oip, &ofin);
-	oip->ip6_plen = savelen;
+
 	READ_ENTER(&ipf_state);
 	for (isp = &ips_table[hv]; (is = *isp); isp = &is->is_hnext) {
 		/*