kern/137232: [ipfw] parser troubles

[Raffaele De Lorenzo]

>Number:         137232
>Category:       kern
>Synopsis:       [ipfw] parser troubles
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Jul 29 08:40:03 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Raffaele De Lorenzo
>Release:        FreeBSD 8.0-BETA2
>Organization:
>Environment:
FreeBSD noel 8.0-BETA2 FreeBSD 8.0-BETA2 #0: Wed Jul 15 21:48:41 UTC 2009     root at mason.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC  amd64
>Description:
ipfw parser doesn't recognize comma separated IPv6 address when the protocol isn't  IPv6 (TCP,UDP..)
>How-To-Repeat:
Try to add an IPv6 multi address rule like this:

ipfw add allow udp from any to 2001:xxx:3::113,2001:xxxx:3::116 dst-port 10001-10100 keep-state

parse say:
ipfw: bad netmask ``xxxx:3::113''
>Fix:
I patched the parser. See the attached patch

Patch attached with submission follows:

diff -bBc -ruN /usr/src/sbin/ipfw/ipfw2.c ./ipfw/ipfw2.c
--- /usr/src/sbin/ipfw/ipfw2.c	2009-07-20 16:09:15.000000000 +0200
+++ ./ipfw/ipfw2.c	2009-07-27 16:14:07.000000000 +0200
@@ -2497,6 +2497,42 @@
 	return NULL;
 }
 
+
+/* 
+ * Pre-Check multi address rules to avoid parser confusion about IPv4/IPv6 addresses.
+ * XXX I assume the first know address is the reference address (You cannot use both IPv4/IPv6 addresses inside 
+ * a multi-addresses rule). 
+ */
+
+int 
+check_multi_addr (char *av) {
+
+	av = strdup(av);
+	struct in6_addr d;
+	struct in_addr du;
+	int ret = -1;
+	/*
+	 * After the address we can have '/' indicating a mask,
+	 * or ',' indicating another address follows.
+	 */
+
+	char *p;
+	char md = '\0';
+
+	if ((p = strpbrk(av, "/,")) ) {
+		md = *p;	/* save the separator */
+		*p = '\0';	/* terminate address string */
+		p++;		/* and skip past it */
+	}
+	/* now p points to NULL, mask or next entry */
+
+	if (lookup_host6(av, &d) == 0)
+		ret = IPPROTO_IPV6;
+	else if (lookup_host(av, &du) == 0)
+		ret = IPPROTO_IP;
+	return ret;
+}
+
 static ipfw_insn *
 add_src(ipfw_insn *cmd, char *av, u_char proto)
 {
@@ -2510,6 +2546,9 @@
 		*ch = '\0';
 
 	if (proto == IPPROTO_IPV6  || strcmp(av, "me6") == 0 ||
+	    ((check_multi_addr (av) == IPPROTO_IPV6) & 
+	    ~(proto == IPPROTO_IP ||
+	    proto == IPPROTO_ICMP)) ||
 	    inet_pton(AF_INET6, host, &a))
 		ret = add_srcip6(cmd, av);
 	/* XXX: should check for IPv4, not !IPv6 */
@@ -2536,6 +2574,9 @@
 		*ch = '\0';
 
 	if (proto == IPPROTO_IPV6  || strcmp(av, "me6") == 0 ||
+	    ((check_multi_addr (av) == IPPROTO_IPV6) & 
+	    ~(proto == IPPROTO_IP ||
+	    proto == IPPROTO_ICMP)) ||
 	    inet_pton(AF_INET6, host, &a))
 		ret = add_dstip6(cmd, av);
 	/* XXX: should check for IPv4, not !IPv6 */
diff -bBc -ruN /usr/src/sbin/ipfw/ipfw2.h ./ipfw/ipfw2.h
--- /usr/src/sbin/ipfw/ipfw2.h	2009-07-20 16:09:15.000000000 +0200
+++ ./ipfw/ipfw2.h	2009-07-27 14:04:50.000000000 +0200
@@ -271,3 +271,7 @@
 void fill_unreach6_code(u_short *codep, char *str);
 void fill_icmp6types(struct _ipfw_insn_icmp6 *cmd, char *av);
 int fill_ext6hdr(struct _ipfw_insn *cmd, char *av);
+int lookup_host6 (char *host, struct in6_addr *ip6addr);
+
+/* Check multi address support */
+int check_multi_addr (char *av);
diff -bBc -ruN /usr/src/sbin/ipfw/ipv6.c ./ipfw/ipv6.c
--- /usr/src/sbin/ipfw/ipv6.c	2009-07-20 16:09:15.000000000 +0200
+++ ./ipfw/ipv6.c	2009-07-27 14:02:08.000000000 +0200
@@ -297,7 +297,7 @@
 }
 
 /* Try to find ipv6 address by hostname */
-static int
+int
 lookup_host6 (char *host, struct in6_addr *ip6addr)
 {
 	struct hostent *he;


>Release-Note:
>Audit-Trail:
>Unformatted: