Bug 23446

Summary: ipfw fragment logging misses first frag
Product: Base System Reporter: Crist J. Clark <cjc>
Component: kernAssignee: bill fumerola <billf>
Status: Closed FIXED    
Severity: Affects Only Me CC: ipfw
Priority: Normal    
Version: 4.1-STABLE   
Hardware: Any   
OS: Any   

Description Crist J. Clark 2000-12-10 20:40:05 UTC
	Logging of fragmented IP datagrams is logged in
sys/netinet/ip_fw.c with the following code,

    if ((ip->ip_off & IP_OFFMASK))
	    snprintf(SNPARGS(fragment, 0), " Fragment = %d",
		ip->ip_off & IP_OFFMASK);
    else
	    fragment[0] = '\0';

That is, it tests if this datagram has a non-zero offset, and if it
does, it prints the offset (somewhat misleadingly labeled as 
"Fragment =").

	There is a problem with this methodology. It misses first
fragments, that is, fragments with zero offset.

Fix: There are a number of issues with ipfw logging. For fragments,
there is no way to tell in logs which fragments belong together,
first fragments are not seen, nor are final fragments marked. Adding
all of that would mean making changes that some people would not like
if for no other reason than it would be a change.

	However, adding the functionality to log first fragments
appropriately is a trivial code change and I cannot think of any
reason why someone would argue against it,



That is, rather than check if the fragment offset is non-zero, we just
check if the fragment offset is non-zero _or_ the more-fragments bit
is set. This will catch initial fragments with zero offset.

	This has no runtime cost and simply makes the logging a little
more precise. Thanks.--SPIaQZw6nclCr0KaMhpvkLlnWUWNI1GrkswH7EHk3CZolD4K
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

--- ip_fw.c.orig        Sun Dec 10 12:22:54 2000
+++ ip_fw.c     Sun Dec 10 12:24:43 2000
@@ -607,7 +607,7 @@
            break;
     }
 
-    if ((ip->ip_off & IP_OFFMASK))
+    if ((ip->ip_off & (IP_OFFMASK | IP_MF)))
            snprintf(SNPARGS(fragment, 0), " Fragment = %d",
                ip->ip_off & IP_OFFMASK);
     else
How-To-Repeat: 
	If you have a machine running ipfw with logging enabled, try
this little trick,

  # ipfw add 10 pass log icmp from 127.0.0.1 to 127.0.0.1 in via lo0
  # ifconfig lo0 mtu 1000
  # ping -c 1 -s 5000 127.0.0.1
  PING 127.0.0.1 (127.0.0.1): 5000 data bytes
  5008 bytes from 127.0.0.1: icmp_seq=0 ttl=255 time=0.657 ms

  --- 127.0.0.1 ping statistics ---
  1 packets transmitted, 1 packets received, 0% packet loss
  # ifconfig lo0 mtu 16384
  # ipfw delete 10

Now notice the log entries generated,

  ipfw: 10 Accept ICMP:8.0 127.0.0.1 127.0.0.1 in via lo0
  ipfw: 10 Accept ICMP 127.0.0.1 127.0.0.1 in via lo0 Fragment = 122
  ipfw: 10 Accept ICMP 127.0.0.1 127.0.0.1 in via lo0 Fragment = 244
  ...
  ipfw: 10 Accept ICMP 127.0.0.1 127.0.0.1 in via lo0 Fragment = 610

The first fragment is not identified as a fragment.
Comment 1 bill fumerola freebsd_committer freebsd_triage 2000-12-11 01:31:39 UTC
Responsible Changed
From-To: freebsd-bugs->billf

This looks good, I'll take it.
Comment 2 ru freebsd_committer freebsd_triage 2001-07-04 09:13:41 UTC
State Changed
From-To: open->closed

Closed per cjc's request.