[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ipfilter
Subject:    Re: ipfilter 4.1.31 automatically drops fragments
From:       Steve Clark <sclark () netwolves ! com>
Date:       2008-12-29 3:24:12
Message-ID: 2882_1230521372_4958441B_2882_16380_1_495842DC.9050909 () netwolves ! com
[Download RAW message or body]

Darren Reed wrote:
> Oren K. wrote:
> 
>>Hi,
>>The 'current version' source downloadable from
>>http://coombs.anu.edu.au/~avalon/ gives a version of ipfilter in which
>>by default any packet fragments beyond the first are dropped with BAD-IN
>>status. I understand this is a result of a fix made due to kernel panic
>>that was reported here:
>>http://marc.info/?l=ipfilter&m=121267676118062&w=2
>>
>>The kernel panic is patched into 4.1.31. However the patch that was to
>>solve dropping subsequent fragments was only posted in the mailing list
>>(same post as above, as 'mypatch.txt') but not patched into the trunk. I
>>applied this patch manually and it solved the problem. My question is,
>>shouldn't this patch be in the main trunk? It seems to me that having
>>ipfilter drop packet fragments by default is an undesirable behavior.
>>  
> 
> 
> So I think you're arguing for something like this...which I'll add now.
> 
> Darren
> 
> 
> 
> ------------------------------------------------------------------------
> 
> Index: ip_nat.c
> ===================================================================
> RCS file: /devel/CVS/IP-Filter/ip_nat.c,v
> retrieving revision 2.195.2.121
> diff -c -r2.195.2.121 ip_nat.c
> *** ip_nat.c	6 Nov 2008 21:18:34 -0000	2.195.2.121
> --- ip_nat.c	28 Dec 2008 15:06:41 -0000
> ***************
> *** 3811,3826 ****
>   	} else {
>   		u_32_t hv, msk, nmsk;
>   
>   		/*
>   		 * If there is no current entry in the nat table for this IP#,
>   		 * create one for it (if there is a matching rule).
>   		 */
> - 		if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
> - 			natfailed = -1;
> - 			goto nonatfrag;
> - 		}
> - 		msk = 0xffffffff;
> - 		nmsk = nat_masks;
>   maskloop:
>   		iph = ipa & htonl(msk);
>   		hv = NAT_HASH_FN(iph, 0, ipf_natrules_sz);
> --- 3811,3825 ----
>   	} else {
>   		u_32_t hv, msk, nmsk;
>   
> + 		if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP))
> + 			goto nonatfrag;
> + 
> + 		msk = 0xffffffff;
> + 		nmsk = nat_masks;
>   		/*
>   		 * If there is no current entry in the nat table for this IP#,
>   		 * create one for it (if there is a matching rule).
>   		 */
>   maskloop:
>   		iph = ipa & htonl(msk);
>   		hv = NAT_HASH_FN(iph, 0, ipf_natrules_sz);
> ***************
> *** 4107,4116 ****
>   	} else {
>   		u_32_t hv, msk, rmsk;
>   
> ! 		if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
> ! 			natfailed = -1;
>   			goto nonatfrag;
> ! 		}
>   		rmsk = rdr_masks;
>   		msk = 0xffffffff;
>   		/*
> --- 4106,4114 ----
>   	} else {
>   		u_32_t hv, msk, rmsk;
>   
> ! 		if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP))
>   			goto nonatfrag;
> ! 
>   		rmsk = rdr_masks;
>   		msk = 0xffffffff;
>   		/*

Hello Darren,

What I found as the best way to implement the patch was in fr_checknatin() and in fr_checknatout()
was to make the following change after removing your original patch of:
> ! 		if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP))
>   			goto nonatfrag;
> ! 
etc.

Was to only do the nat_new if this was fin_off was zero.

            if ((fin->fin_off == 0) && (nat = nat_new(fin, np, NULL, nflags, NAT_INBOUND))) {
                np->in_hits++;
                break;
            } else
                natfailed = -1;
        }

attached is my patch against 4.1.28 which has been working flawlessly at over 400 sites for 
the past 6 months.

Steve

["mypatch.txt" (text/plain)]

*** ip_nat.c.orig	Thu Feb  7 11:46:47 2008
--- ip_nat.c	Thu May 22 11:55:27 2008
***************
*** 2571,2587 ****
  	nat->nat_p = fin->fin_p;
  	nat->nat_mssclamp = np->in_mssclamp;
  	if (nat->nat_p == IPPROTO_TCP)
!         {
!                 if(tcp != NULL)
!                 {
!                         nat->nat_seqnext[0] = ntohl(tcp->th_seq);
!                 }
!                 else
!                 {
!                         printf("IP Filter: nat_finalise() - bad fragment\n");
!                         return -1;
!                 }
!         }
  
  	if ((np->in_apr != NULL) && ((ni->nai_flags & NAT_SLAVE) == 0))
  		if (appr_new(fin, nat) == -1)
--- 2571,2577 ----
  	nat->nat_p = fin->fin_p;
  	nat->nat_mssclamp = np->in_mssclamp;
  	if (nat->nat_p == IPPROTO_TCP)
! 		nat->nat_seqnext[0] = ntohl(tcp->th_seq);
  
  	if ((np->in_apr != NULL) && ((ni->nai_flags & NAT_SLAVE) == 0))
  		if (appr_new(fin, nat) == -1)
***************
*** 3797,3802 ****
--- 3787,3798 ----
  		 * If there is no current entry in the nat table for this IP#,
  		 * create one for it (if there is a matching rule).
  		 */
+ #if 0
+         if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
+             natfailed = -1;
+             goto nonatfrag;
+         }
+ #endif
  		RWLOCK_EXIT(&ipf_nat);
  		msk = 0xffffffff;
  		nmsk = nat_masks;
***************
*** 3804,3813 ****
  maskloop:
  		iph = ipa & htonl(msk);
  		hv = NAT_HASH_FN(iph, 0, ipf_natrules_sz);
-               if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
-                       natfailed = -1;
-                       goto nonatfrag;
-               }
  		for (np = nat_rules[hv]; np; np = np->in_mnext)
  		{
  			if ((np->in_ifps[1] && (np->in_ifps[1] != ifp)))
--- 3800,3805 ----
***************
*** 3836,3842 ****
  					continue;
  			}
  
! 			if ((nat = nat_new(fin, np, NULL, nflags,
  					   NAT_OUTBOUND))) {
  				np->in_hits++;
  				break;
--- 3828,3834 ----
  					continue;
  			}
  
! 			if ((fin->fin_off == 0) && (nat = nat_new(fin, np, NULL, nflags,
  					   NAT_OUTBOUND))) {
  				np->in_hits++;
  				break;
***************
*** 3857,3863 ****
  		}
  		MUTEX_DOWNGRADE(&ipf_nat);
  	}
! 
  	if (nat != NULL) {
  		rval = fr_natout(fin, nat, natadd, nflags);
  		if (rval == 1) {
--- 3849,3855 ----
  		}
  		MUTEX_DOWNGRADE(&ipf_nat);
  	}
!  /*nonatfrag:*/
  	if (nat != NULL) {
  		rval = fr_natout(fin, nat, natadd, nflags);
  		if (rval == 1) {
***************
*** 3865,3871 ****
  			nat->nat_ref++;
  			MUTEX_EXIT(&nat->nat_lock);
  			nat->nat_touched = fr_ticks;
- nonatfrag:
  			fin->fin_nat = nat;
  		}
  	} else
--- 3857,3862 ----
***************
*** 4092,4098 ****
  		nflags = nat->nat_flags;
  	} else {
  		u_32_t hv, msk, rmsk;
! 
  		RWLOCK_EXIT(&ipf_nat);
  		rmsk = rdr_masks;
  		msk = 0xffffffff;
--- 4083,4094 ----
  		nflags = nat->nat_flags;
  	} else {
  		u_32_t hv, msk, rmsk;
! #if 0
!         if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
!             natfailed = -1;
!             goto nonatfrag;
!         }
! #endif
  		RWLOCK_EXIT(&ipf_nat);
  		rmsk = rdr_masks;
  		msk = 0xffffffff;
***************
*** 4100,4109 ****
  		/*
  		 * If there is no current entry in the nat table for this IP#,
  		 * create one for it (if there is a matching rule).
-               if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
-                       natfailed = -1;
-                       goto nonatfrag;
-               }
  		 */
  maskloop:
  		iph = in.s_addr & htonl(msk);
--- 4096,4101 ----
***************
*** 4135,4142 ****
  				}
  			}
  
! 			nat = nat_new(fin, np, NULL, nflags, NAT_INBOUND);
! 			if (nat != NULL) {
  				np->in_hits++;
  				break;
  			} else
--- 4127,4133 ----
  				}
  			}
  
! 			if ((fin->fin_off == 0) && (nat = nat_new(fin, np, NULL, nflags, NAT_INBOUND))) {
  				np->in_hits++;
  				break;
  			} else
***************
*** 4157,4162 ****
--- 4148,4154 ----
  		}
  		MUTEX_DOWNGRADE(&ipf_nat);
  	}
+  /*nonatfrag:*/
  	if (nat != NULL) {
  		rval = fr_natin(fin, nat, natadd, nflags);
  		if (rval == 1) {
***************
*** 4164,4170 ****
  			nat->nat_ref++;
  			MUTEX_EXIT(&nat->nat_lock);
  			nat->nat_touched = fr_ticks;
- nonatfrag:
  			fin->fin_nat = nat;
  		}
  	} else
--- 4156,4161 ----


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic