o IPFW incorrectly handled filtering in the presence of previously

  reserved and now allocated TCP flags in incoming packets.  This patch
  stops overloading those bits in the IP firewall rules, and moves
  colliding flags to a seperate field, ipflg.  The IPFW userland
  management tool, ipfw(8), is updated to reflect this change.  New TCP
  flags related to ECN are now included in tcp.h for reference, although
  we don't currently implement TCP+ECN.

o To use this fix without completely rebuilding, it is sufficient to copy
  ip_fw.h and tcp.h into your appropriate include directory, then rebuild
  the ipfw kernel module, and ipfw tool, and install both.  Note that a
  mismatch between module and userland tool will result in incorrect
  installation of firewall rules that may have unexpected effects.  This
  is an MFC candidate, following shakedown.  This bug does not appear
  to affect ipfilter.

Reviewed by:	security-officer, billf
Reported by:	Aragon Gouveia <aragon@phat.za.net>

3 files changed, 18 insertions, 8 deletions

diff --git a/sys/netinet/ip_fw.c b/sys/netinet/ip_fw.c
index 7e69223..3f3d325 100644
--- a/sys/netinet/ip_fw.c
+++ b/sys/netinet/ip_fw.c
@@ -244,10 +244,16 @@ static int
 tcpflg_match(struct tcphdr *tcp, struct ip_fw *f)
 {
 	u_char		flg_set, flg_clr;
-	
-	if ((f->fw_tcpf & IP_FW_TCPF_ESTAB) &&
-	    (tcp->th_flags & (IP_FW_TCPF_RST | IP_FW_TCPF_ACK)))
-		return 1;
+
+	/*
+	 * If an established connection is required, reject packets that
+	 * have only SYN of RST|ACK|SYN set.  Otherwise, fall through to
+	 * other flag requirements.
+	 */
+	if ((f->fw_ipflg & IP_FW_IF_TCPEST) &&
+	    ((tcp->th_flags & (IP_FW_TCPF_RST | IP_FW_TCPF_ACK |
+	    IP_FW_TCPF_SYN)) == IP_FW_TCPF_SYN))
+		return 0;
 
 	flg_set = tcp->th_flags & f->fw_tcpf;
 	flg_clr = tcp->th_flags & f->fw_tcpnf;
@@ -1243,7 +1249,9 @@ again:
 
 			if (f->fw_ipflg & IP_FW_IF_TCPOPT && !tcpopts_match(tcp, f))
 				continue;
-			if (f->fw_ipflg & IP_FW_IF_TCPFLG && !tcpflg_match(tcp, f))
+			if (((f->fw_ipflg & IP_FW_IF_TCPFLG) ||
+			    (f->fw_ipflg & IP_FW_IF_TCPEST)) &&
+			    !tcpflg_match(tcp, f))
 				continue;
 			if (f->fw_ipflg & IP_FW_IF_TCPSEQ && tcp->th_seq != f->fw_tcpseq)
 				continue;
diff --git a/sys/netinet/ip_fw.h b/sys/netinet/ip_fw.h
index f61abd1..a642d72 100644
--- a/sys/netinet/ip_fw.h
+++ b/sys/netinet/ip_fw.h
@@ -222,7 +222,8 @@ struct ipfw_dyn_rule {
 #define IP_FW_IF_TCPSEQ	0x00000004      /* tcp sequence number		*/
 #define IP_FW_IF_TCPACK	0x00000008      /* tcp acknowledgement number	*/
 #define IP_FW_IF_TCPWIN	0x00000010      /* tcp window size		*/
-#define IP_FW_IF_TCPMSK	0x0000001f      /* mask of all tcp values	*/
+#define	IP_FW_IF_TCPEST 0x00000020	/* established TCP connection	*/
+#define	IP_FW_IF_TCPMSK	0x0000003f	/* mask of all tcp values	*/
 
 #define IP_FW_IF_IPOPT	0x00000100	/* ip options			*/
 #define IP_FW_IF_IPLEN	0x00000200	/* ip length			*/
@@ -274,7 +275,6 @@ struct ipfw_dyn_rule {
 #define IP_FW_TCPF_PSH		TH_PUSH
 #define IP_FW_TCPF_ACK		TH_ACK
 #define IP_FW_TCPF_URG		TH_URG
-#define IP_FW_TCPF_ESTAB	0x40
 
 /*
  * Main firewall chains definitions and global var's definitions.
diff --git a/sys/netinet/tcp.h b/sys/netinet/tcp.h
index 6c214e8..3b72094 100644
--- a/sys/netinet/tcp.h
+++ b/sys/netinet/tcp.h
@@ -67,7 +67,9 @@ struct tcphdr {
 #define	TH_PUSH	0x08
 #define	TH_ACK	0x10
 #define	TH_URG	0x20
-#define TH_FLAGS (TH_FIN|TH_SYN|TH_RST|TH_ACK|TH_URG)
+#define	TH_ECE	0x40
+#define	TH_CWR	0x80
+#define TH_FLAGS (TH_FIN|TH_SYN|TH_RST|TH_ACK|TH_URG|TH_ECE|TH_CWR)
 
 	u_short	th_win;			/* window */
 	u_short	th_sum;			/* checksum */