kern/45929: ipnat does not remove rules with -r

[darrenr%NetBSD.org@localhost]

>Number:         45929
>Category:       kern
>Synopsis:       ipnat does not remove rules with -r
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Feb 05 13:55:00 +0000 2012
>Originator:     Darren Reed
>Release:        NetBSD 5.99.63
>Organization:
>Description:
        There are two issues current with ipnat. The most serious is
that "ipnat -rf /etc/ipnat.conf" will not remove NAT rules because the
"-r" function will not work. Another side effect of this is that it is
possile to load the same rule twice - rule matching is broken. Finally,
if ipfilter is disabled and ipnat is run, it will print a very broken
error message:
:ioctl(SIOCGNATS)
whereas what it should print is this:
130003:ioctl(SIOCGNATS) ioctl on device denied, ipfitler is disabled
>How-To-Repeat:
        
>Fix:

Index: ip_fil_netbsd.c
===================================================================
RCS file: /room/netbsd/src/sys/dist/ipf/netinet/ip_fil_netbsd.c,v
retrieving revision 1.59
diff -c -r1.59 ip_fil_netbsd.c
*** ip_fil_netbsd.c     1 Feb 2012 02:21:19 -0000       1.59
--- ip_fil_netbsd.c     5 Feb 2012 13:01:39 -0000
***************
*** 638,644 ****
        }
  
        if (ipfmain.ipf_running <= 0) {
!               if (unit != IPL_LOGIPF) {
                        ipfmain.ipf_interror = 130003;
                        return EIO;
                }
--- 638,644 ----
        }
  
        if (ipfmain.ipf_running <= 0) {
!               if (unit != IPL_LOGIPF && cmd != SIOCIPFINTERROR) {
                        ipfmain.ipf_interror = 130003;
                        return EIO;
                }
Index: ip_nat.c
===================================================================
RCS file: /room/netbsd/src/sys/dist/ipf/netinet/ip_nat.c,v
retrieving revision 1.46
diff -c -r1.46 ip_nat.c
*** ip_nat.c    1 Feb 2012 02:21:20 -0000       1.46
--- ip_nat.c    5 Feb 2012 13:40:44 -0000
***************
*** 229,234 ****
--- 229,235 ----
  static        void    ipf_nat_addrdr(ipf_nat_softc_t *, ipnat_t *);
  static        int     ipf_nat_builddivertmp(ipf_nat_softc_t *, ipnat_t *);
  static        int     ipf_nat_clearlist(ipf_main_softc_t *, ipf_nat_softc_t 
*);
+ static        int     ipf_nat_cmp_rules(ipnat_t *, ipnat_t *);
  static        int     ipf_nat_decap(fr_info_t *, nat_t *);
  static        void    ipf_nat_del_active(int, u_32_t *);
  static        void    ipf_nat_del_map_mask(ipf_nat_softc_t *, int);
***************
*** 1273,1280 ****
                MUTEX_ENTER(&softn->ipf_nat_io);
                for (np = &softn->ipf_nat_list; ((n = *np) != NULL);
                     np = &n->in_next)
!                       if (!bcmp((char *)&nat->in_v, (char *)&n->in_v,
!                                       IPN_CMPSIZ))
                                break;
        }
  
--- 1274,1280 ----
                MUTEX_ENTER(&softn->ipf_nat_io);
                for (np = &softn->ipf_nat_list; ((n = *np) != NULL);
                     np = &n->in_next)
!                       if (ipf_nat_cmp_rules(nat, n) == 0)
                                break;
        }
  
***************
*** 8896,8898 ****
--- 8896,8958 ----
  
        RWLOCK_EXIT(&softc->ipf_nat);
  }
+ 
+ 
+ /* ------------------------------------------------------------------------ */
+ /* Function:    ipf_nat_cmp_rules                                           */
+ /* Returns:     int   - 0 == success, else rules do not match.              */
+ /* Parameters:  n1(I) - first rule to compare                               */
+ /*              n2(I) - first rule to compare                               */
+ /*                                                                          */
+ /* Compare two rules using pointers to each rule. A straight bcmp will not  */
+ /* work as some fields (such as in_dst, in_pkts) actually do change once    */
+ /* the rule has been loaded into the kernel. Whilst this function returns   */
+ /* various non-zero returns, they're strictly to aid in debugging. Use of   */
+ /* this function should simply care if the result is zero or not.           */
+ /* ------------------------------------------------------------------------ */
+ static int
+ ipf_nat_cmp_rules(ipnat_t *n1, ipnat_t *n2)
+ {
+       if (n1->in_size != n2->in_size)
+               return 1;
+ 
+       if (bcmp((char *)&n1->in_v, (char *)&n2->in_v,
+                offsetof(ipnat_t, in_ndst) - offsetof(ipnat_t, in_v)) != 0)
+               return 2;
+ 
+       if (bcmp((char *)&n1->in_tuc, (char *)&n2->in_tuc,
+                offsetof(ipnat_t, in_pkts) - offsetof(ipnat_t, in_tuc)) != 0)
+               return 3;
+       if (bcmp((char *)&n1->in_namelen, (char *)&n2->in_namelen,
+                n1->in_size  - offsetof(ipnat_t, in_namelen)) != 0)
+               return 4;
+       if (n1->in_ndst.na_atype != n2->in_ndst.na_atype)
+               return 5;
+       if (n1->in_ndst.na_function != n2->in_ndst.na_function)
+               return 6;
+       if (bcmp((char *)&n1->in_ndst.na_addr, (char *)&n2->in_ndst.na_addr,
+                sizeof(n1->in_ndst.na_addr)))
+               return 7;
+       if (n1->in_nsrc.na_atype != n2->in_nsrc.na_atype)
+               return 8;
+       if (n1->in_nsrc.na_function != n2->in_nsrc.na_function)
+               return 9;
+       if (bcmp((char *)&n1->in_nsrc.na_addr, (char *)&n2->in_nsrc.na_addr,
+                sizeof(n1->in_nsrc.na_addr)))
+               return 10;
+       if (n1->in_odst.na_atype != n2->in_odst.na_atype)
+               return 11;
+       if (n1->in_odst.na_function != n2->in_odst.na_function)
+               return 12;
+       if (bcmp((char *)&n1->in_odst.na_addr, (char *)&n2->in_odst.na_addr,
+                sizeof(n1->in_odst.na_addr)))
+               return 13;
+       if (n1->in_osrc.na_atype != n2->in_osrc.na_atype)
+               return 14;
+       if (n1->in_osrc.na_function != n2->in_osrc.na_function)
+               return 15;
+       if (bcmp((char *)&n1->in_osrc.na_addr, (char *)&n2->in_osrc.na_addr,
+                sizeof(n1->in_osrc.na_addr)))
+               return 16;
+       return 0;
+ }