Re: ipfilter 4.1.31 automatically drops fragments
Steve Clark <[email protected]>
| Newsgroups | gmane.comp.security.firewalls.ipfilter |
|---|---|
| Message-ID | <2882_1230521155_49584343_2882_16360_1_495842DC.9050909@netwolves.com> |
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, 4.2 KB)
*** 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 ----