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 that you understand the "problem"....
If a non-first fragment arrives first, you cannot NAT that
packet at all.
Why?
Because the way in which IPFilter has been designed, it will
look for NAT information in a table built especially for tracking
fragments, before it looks to see if it has enough information to
look in the normal table.
If your non-first fragment seeded that table then the fragment
that contains the port numbers would also match there and because
it won't be using the NAT entry that describes how to translate
the port numbers, the packet will be NAT'd wrongly, quite likely
leading to the connection ultimately being torn down.
Hmm...so what I can do to mitigate that is to only search the
fragment NAT table for non-first fragments in the NAT code.
Try this patch out for size (since it changes behaviour, it
will cause some of the existing regression tests to fail.)
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:42:43 -0000
***************
*** 3803,3809 ****
if (((fin->fin_flx & FI_ICMPERR) != 0) &&
(nat = nat_icmperror(fin, &nflags, NAT_OUTBOUND)))
/*EMPTY*/;
! else if ((fin->fin_flx & FI_FRAG) && (nat = fr_nat_knownfrag(fin)))
natadd = 0;
else if ((nat = nat_outlookup(fin, nflags|NAT_SEARCH, (u_int)fin->fin_p,
fin->fin_src, fin->fin_dst))) {
--- 3803,3810 ----
if (((fin->fin_flx & FI_ICMPERR) != 0) &&
(nat = nat_icmperror(fin, &nflags, NAT_OUTBOUND)))
/*EMPTY*/;
! else if ((fin->fin_flx & FI_FRAG) && (fin->fin_off != 0) &&
! (nat = fr_nat_knownfrag(fin)))
natadd = 0;
else if ((nat = nat_outlookup(fin, nflags|NAT_SEARCH, (u_int)fin->fin_p,
fin->fin_src, fin->fin_dst))) {
***************
*** 3811,3837 ****
} 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);
for (np = nat_rules[hv]; np; np = npnext) {
npnext = np->in_mnext;
! if ((np->in_ifps[1] && (np->in_ifps[1] != ifp)))
continue;
if (np->in_v != fin->fin_v)
continue;
if (np->in_p && (np->in_p != fin->fin_p))
continue;
if ((np->in_flags & IPN_RF) && !(np->in_flags & nflags))
continue;
if (np->in_flags & IPN_FILTER) {
--- 3812,3837 ----
} else {
u_32_t hv, msk, nmsk;
+ 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);
for (np = nat_rules[hv]; np; np = npnext) {
npnext = np->in_mnext;
! if (np->in_ifps[1] && (np->in_ifps[1] != ifp))
continue;
if (np->in_v != fin->fin_v)
continue;
if (np->in_p && (np->in_p != fin->fin_p))
continue;
+ if ((fin->fin_off != 0) &&
+ (np->in_flags & IPN_TCPUDPICMP))
+ goto nonatfrag;
if ((np->in_flags & IPN_RF) && !(np->in_flags & nflags))
continue;
if (np->in_flags & IPN_FILTER) {
***************
*** 4099,4105 ****
if (((fin->fin_flx & FI_ICMPERR) != 0) &&
(nat = nat_icmperror(fin, &nflags, NAT_INBOUND)))
/*EMPTY*/;
! else if ((fin->fin_flx & FI_FRAG) && (nat = fr_nat_knownfrag(fin)))
natadd = 0;
else if ((nat = nat_inlookup(fin, nflags|NAT_SEARCH, (u_int)fin->fin_p,
fin->fin_src, in))) {
--- 4099,4106 ----
if (((fin->fin_flx & FI_ICMPERR) != 0) &&
(nat = nat_icmperror(fin, &nflags, NAT_INBOUND)))
/*EMPTY*/;
! else if ((fin->fin_flx & FI_FRAG) && (fin->fin_off != 0) &&
! (nat = fr_nat_knownfrag(fin)))
natadd = 0;
else if ((nat = nat_inlookup(fin, nflags|NAT_SEARCH, (u_int)fin->fin_p,
fin->fin_src, in))) {
***************
*** 4107,4118 ****
} 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;
/*
* If there is no current entry in the nat table for this IP#,
* create one for it (if there is a matching rule).
--- 4108,4115 ----
} else {
u_32_t hv, msk, rmsk;
msk = 0xffffffff;
+ rmsk = rdr_masks;
/*
* If there is no current entry in the nat table for this IP#,
* create one for it (if there is a matching rule).
***************
*** 4128,4133 ****
--- 4125,4133 ----
continue;
if (np->in_p && (np->in_p != fin->fin_p))
continue;
+ if ((fin->fin_off != 0) &&
+ (np->in_flags & IPN_TCPUDPICMP))
+ goto nonatfrag;
if ((np->in_flags & IPN_RF) && !(np->in_flags & nflags))
continue;
if (np->in_flags & IPN_FILTER) {
