bin/102422: ipfw & kernel problems where firewall rules aren't interpreted correctly

[Stephen E. Halpin]

The following reply was made to PR bin/102422; it has been noted by GNATS.

From: "Stephen E. Halpin" <seh-10lzx4 at mail.quadrizen.com>
To: Andrey V. Elsukov <bu7cher at yandex.ru>
Cc: bug-followup at FreeBSD.org,
 Oleg Bulyzhin <oleg at FreeBSD.org>,
 Gleb Smirnoff <glebius at FreeBSD.org>,
 Luigi Rizzo <rizzo at icir.org>
Subject: Re: bin/102422: ipfw & kernel problems where firewall rules aren't interpreted correctly
Date: Sat, 26 Aug 2006 05:01:39 -0400

 Sorry for taking so long to get back to you.  The changes look good.   
 I've tested the changes for ip_fw2.c for both source and destination  
 processing, and it worked fine.  I still have a question about PR  
 91245, as when I went to the following page:
 
      http://www.freebsd.org/cgi/cvsweb.cgi/src/sbin/ipfw/
 
 and it looks like the last version of ipfw2.c is 1.96 on the MAIN  
 branch, and the changes in PR 91245 are not there.  It would be  
 awesome if all three fixes could make it into the 6.2 release!
 
 -Steve
 
 
 On Aug 24, 2006, at 6:09 AM, Andrey V. Elsukov wrote:
 
 > Stephen Halpin wrote:
 > > The rule is accepted with icmp6types 1,2,32,33,34,...94,95,128,129.
 > > The problem is the data structure in
 > > /usr/src/sbin/ipfw/ipfw2.c:fill_icmp6types() is not properly
 > > initialized.
 >
 > Yes, you are right. A data buffer is previously zeroed, but
 > fill_ip6() function can modified some data while parsing ipv6
 > destination addresses. Quick fix is simple:
 >
 > --- ipfw2.c     23 Aug 2006 14:29:18 -0000      1.96
 > +++ ipfw2.c     24 Aug 2006 09:08:06 -0000
 > @@ -1206,7 +1206,7 @@
 >  {
 >         uint8_t type;
 >
 > -       cmd->d[0] = 0;
 > +       bzero(cmd, sizeof(*cmd));
 >         while (*av) {
 >             if (*av == ',')
 >                 av++;
 >
 >
 > But i think that here can be another similar issues.
 >
 > > addressed with bug number 91245, which the query interface won't
 > > bring up for anything.  I was able to find it using the global
 > > Google.  I found a set of diffs at:
 >
 > PR 91245 was closed.
 > http://www.freebsd.org/cgi/query-pr.cgi?pr=91245
 >
 > > Problem 3:
 > >
 > > ipfw add allow ip6 from any to 2000::/16,2002::/16
 >
 > Can you try the attached patch?
 >
 > -- 
 > WBR, Andrey V. Elsukov
 > Index: ip_fw2.c
 > ===================================================================
 > RCS file: /mnt/cvs/ncvs/src/sys/netinet/ip_fw2.c,v
 > retrieving revision 1.144
 > diff -u -r1.144 ip_fw2.c
 > --- ip_fw2.c	18 Aug 2006 22:36:04 -0000	1.144
 > +++ ip_fw2.c	24 Aug 2006 09:55:38 -0000
 > @@ -2869,22 +2869,20 @@
 >  				    &((ipfw_insn_ip6 *)cmd)->addr6);
 >  				break;
 >  			case O_IP6_SRC_MASK:
 > -				if (is_ipv6) {
 > -					ipfw_insn_ip6 *te = (ipfw_insn_ip6 *)cmd;
 > -					struct in6_addr p = args->f_id.src_ip6;
 > -
 > -					APPLY_MASK(&p, &te->mask6);
 > -					match = IN6_ARE_ADDR_EQUAL(&te->addr6, &p);
 > -				}
 > -				break;
 > -
 >  			case O_IP6_DST_MASK:
 >  				if (is_ipv6) {
 > -					ipfw_insn_ip6 *te = (ipfw_insn_ip6 *)cmd;
 > -					struct in6_addr p = args->f_id.dst_ip6;
 > +					int i = cmdlen - 1;
 > +					struct in6_addr p;
 > +					struct in6_addr *d = &((ipfw_insn_ip6 *)cmd)->addr6;
 >
 > -					APPLY_MASK(&p, &te->mask6);
 > -					match = IN6_ARE_ADDR_EQUAL(&te->addr6, &p);
 > +					for (; !match && i > 0; d += 2,
 > +						i -= F_INSN_SIZE(struct in6_addr) * 2)
 > +					{
 > +						p = (cmd->opcode == O_IP6_SRC_MASK) ?
 > +							args->f_id.src_ip6: args->f_id.dst_ip6;
 > +						APPLY_MASK(&p, &d[1]);
 > +						match = IN6_ARE_ADDR_EQUAL(&d[0], &p);
 > +					}
 >  				}
 >  				break;
 >