[Ipfilter-bugs] [ ipfilter-Bugs-2031730 ] 4.1.31 Nat drops fragmented packets after the first

[SourceForge.net <no...@so...>]

Bugs item #2031730, was opened at 2008-07-29 11:29
Message generated for change (Comment added) made by darren_r
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=849053&aid=2031730&group_id=169098

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: FreeBSD
Group: 4.1.*
Status: Open
Resolution: Accepted
Priority: 5
Private: No
Submitted By: Stephen E. Clark (sclark46)
Assigned to: Darren (darren_r)
Summary: 4.1.31 Nat drops fragmented packets after the first

Initial Comment:
When NAT is active and there are no matching nat rules against the incomming packet and the packet has a fragment offset other than zero nat marks it to be dropped by ipf.

in function int fr_checknatout(fin, passp)
...
		if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
			natfailed = -1;
			goto nonatfrag;
		}

and
in
int fr_checknatin(fin, passp)
...

		if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
			natfailed = -1;
			goto nonatfrag;
		}

In both of these cases NAT should make sure the packet matches a nat rule before
marking the packet to be dropped. It currently only checks to see if there is an
entry already in the NAT table - which there would not be if there is no matching
nat rule.

As an example the initial packet in a fragmented series comes in thru NAT- nat checks fin->fin_off = 0 - yes let see if there is a matching nat rule - no ok let it pass. The next packet in the fragmented series comes in thru NAT - nat checks
fin->fin_off = 0 - not zero and there is no entry in the NAT table mark it to be dropped.

----------------------------------------------------------------------

>Comment By: Darren (darren_r)
Date: 2008-07-30 00:22

Message:
Logged In: YES 
user_id=1448875
Originator: NO

Here's a scenario to consider:

Packet 1. outbound non-fragment(id=1), UDP, src=s:a, dst=d,b => sp=x,dp=b
Packet 2. outbound body-fragment(off=100,id=2), UDP src=s, dst=b
Packet 3. outbound head-fragment(off=0,id=2), UDP, src=s:x, dst=d:b
Packet 4. inbound non-fragment, UDP, src=d:b, dst=s':x

After packet 1, IPFilter creates a NAT session for (s,a) => (s',x).
If we don't drop packet 2 but NAT it, then IPFilter creates a NAT session
for (s) = (s') and because packet 3 is another fragment that is associated
with a known NAT session, it gets the same treatment as #2 did and its
ports aren't changed (nor is there a lookup to see if there is a collision
with the first packet).

The problem arises with the reply packet, #4, what do we do with it?
Translate the destination port from x to a or leave it?
It could belong to either one of two sessions.


----------------------------------------------------------------------

Comment By: Darren (darren_r)
Date: 2008-07-29 14:52

Message:
Logged In: YES 
user_id=1448875
Originator: NO

It would seem that the correct thing to do is simply say no NAT took
place,
rather than say that NAT failed.  Fixing this could be as simple as what's
below.

Index: ip_nat.c
===================================================================
RCS file: /devel/CVS/IP-Filter/ip_nat.c,v
retrieving revision 2.195.2.115
diff -u -r2.195.2.115 ip_nat.c
--- ip_nat.c    26 Jul 2008 18:41:30 -0000      2.195.2.115
+++ ip_nat.c    29 Jul 2008 21:51:14 -0000
@@ -3816,7 +3816,7 @@
                 * create one for it (if there is a matching rule).
                 */
                if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
-                       natfailed = -1;
+                       natfailed = 0;
                        goto nonatfrag;
                }
                msk = 0xffffffff;
@@ -4125,7 +4125,7 @@
                u_32_t hv, msk, rmsk;
 
                if ((fin->fin_off != 0) && (fin->fin_flx & FI_TCPUDP)) {
-                       natfailed = -1;
+                       natfailed = 0;
                        goto nonatfrag;
                }
                rmsk = rdr_masks;


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=849053&aid=2031730&group_id=169098