Bug 51341 - [ipfw] [patch] ipfw rule 'deny icmp from any to any icmptype 5' matches fragmented icmp packets
Summary: [ipfw] [patch] ipfw rule 'deny icmp from any to any icmptype 5' matches fragm...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 4.7-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-04-24 09:50 UTC by land
Modified: 2018-02-02 22:09 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description land 2003-04-24 09:50:13 UTC
	IPFW1 rule 'deny icmp from any to any icmptype 5' matches fragmented
	ICMP packets.

How-To-Repeat: 	ipfw add 1 deny icmp from any to any icmptype 5
	Try to ping external host with big ICMP packets:
	ping -s 2000 host
Comment 1 Maxim Konovalov 2003-04-24 12:14:05 UTC
Hello,

Could you please test a patch below? Thanks.

Index: sys/netinet/ip_fw.c
===================================================================
RCS file: /home/ncvs/src/sys/netinet/ip_fw.c,v
retrieving revision 1.131.2.39
diff -u -r1.131.2.39 ip_fw.c
--- sys/netinet/ip_fw.c	20 Jan 2003 02:23:07 -0000	1.131.2.39
+++ sys/netinet/ip_fw.c	24 Apr 2003 11:12:02 -0000
@@ -1434,7 +1434,7 @@
 			struct icmp *icmp;

 			if (offset != 0)	/* Type isn't valid */
-				break;
+				continue;
 			icmp = (struct icmp *) ((u_int32_t *)ip + ip->ip_hl);
 			if (!icmptype_match(icmp, f))
 				continue;

%%%

-- 
Maxim Konovalov, maxim@macomnet.ru, maxim@FreeBSD.org
Comment 2 Maxim Konovalov 2003-04-24 12:43:12 UTC
Add to audit trail.

-- 
Maxim Konovalov, maxim@macomnet.ru, maxim@FreeBSD.org

---------- Forwarded message ----------
Date: Thu, 24 Apr 2003 14:35:58 +0300
From: Andrey Lakhno <land@dnepr.net>
To: Maxim Konovalov <maxim@macomnet.ru>
Subject: Re: kern/51341: ipfw rule 'deny icmp from any to any icmptype 5'
    matches fragmented icmp packets

Hello,

On Thu, 24 Apr 2003, Maxim Konovalov wrote:

> Could you please test a patch below? Thanks.

It works.
Thank you !

> Index: sys/netinet/ip_fw.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet/ip_fw.c,v
> retrieving revision 1.131.2.39
> diff -u -r1.131.2.39 ip_fw.c
> --- sys/netinet/ip_fw.c	20 Jan 2003 02:23:07 -0000	1.131.2.39
> +++ sys/netinet/ip_fw.c	24 Apr 2003 11:12:02 -0000
> @@ -1434,7 +1434,7 @@
>  			struct icmp *icmp;
>
>  			if (offset != 0)	/* Type isn't valid */
> -				break;
> +				continue;
>  			icmp = (struct icmp *) ((u_int32_t *)ip + ip->ip_hl);
>  			if (!icmptype_match(icmp, f))
>  				continue;
>
> %%%

-- 
Andrey Lakhno,
land-ripe
Comment 3 Johan Karlsson freebsd_committer freebsd_triage 2003-05-06 20:51:20 UTC
Responsible Changed
From-To: freebsd-bugs->ipfw

Over to maintainer group.
Comment 4 Maxim Konovalov 2003-07-03 15:53:35 UTC
Hello Andrey,

Here is another workaround: add a following rule before any icmp deny
rules:

	ipfw add pass icmp from any to any frag

I would like to describe the problem in two words.  Please consider a
next rule:

	deny icmp from any to any icmptype 5

Consider we get an icmp fragment.  In fact, it does not consist
information about its type and due to the discussed bug ipfw1 will
terminate the search and drop it.  ipfw2 behaviour is different: if we
do not know about icmp type of the packet do not terminate the search
and check the packet against next rule.

At the moment I really do not want to fix this bug because it changes
a filtering policy and may have a negative effect to countless
installations.

Please let me know if you are satisfied with my explanation and I can
close the PR.

Thanks!

-- 
Maxim Konovalov, maxim@macomnet.ru, maxim@FreeBSD.org
Comment 5 Maxim Konovalov freebsd_committer freebsd_triage 2003-07-03 16:24:53 UTC
State Changed
From-To: open->feedback

Feedback awating.
Comment 6 Maxim Konovalov 2003-07-04 12:09:15 UTC
---------- Forwarded message ----------
Date: Fri, 4 Jul 2003 13:47:56 +0300
From: Andrey Lakhno <land@dnepr.net>
To: Maxim Konovalov <maxim@macomnet.ru>
Subject: Re: kern/51341

Hello,

On Thu, 03 Jul 2003, Maxim Konovalov wrote:

> Here is another workaround: add a following rule before any icmp deny
> rules:
>
> 	ipfw add pass icmp from any to any frag
>
> I would like to describe the problem in two words.  Please consider a
> next rule:
>
> 	deny icmp from any to any icmptype 5
>
> Consider we get an icmp fragment.  In fact, it does not consist
> information about its type and due to the discussed bug ipfw1 will
> terminate the search and drop it.  ipfw2 behaviour is different: if we
> do not know about icmp type of the packet do not terminate the search
> and check the packet against next rule.
>
> At the moment I really do not want to fix this bug because it changes
> a filtering policy and may have a negative effect to countless
> installations.
>
> Please let me know if you are satisfied with my explanation and I can
> close the PR.

I think this bug should be decribed in ipfw(8) or fixed.

-- 
Andrey Lakhno,
land-ripe
Comment 7 Remko Lodder freebsd_committer freebsd_triage 2006-11-26 20:27:46 UTC
Responsible Changed
From-To: freebsd-ipfw->remko

grab this; had this ever been fixed?
Comment 8 Mark Linimon freebsd_committer freebsd_triage 2008-03-01 20:10:45 UTC
State Changed
From-To: feedback->suspended

Feedback was received some time ago.  It sounds as though this behavior 
should simply be documented?
Comment 9 Maxim Konovalov freebsd_committer freebsd_triage 2011-07-28 21:29:32 UTC
State Changed
From-To: suspended->closed
Comment 10 Maxim Konovalov freebsd_committer freebsd_triage 2011-07-28 21:31:43 UTC
State Changed
From-To: closed->suspended

Closed unintentionally.  Restore its state.
Comment 11 Remko Lodder freebsd_committer freebsd_triage 2012-09-27 08:39:48 UTC
State Changed
From-To: suspended->open

This is likely to be documented, but I will not do this, reassign to the pool 


Comment 12 Remko Lodder freebsd_committer freebsd_triage 2012-09-27 08:39:48 UTC
Responsible Changed
From-To: remko->freebsd-doc

Reassign to the pool; this should probably be documented, but I will not do that.
Comment 13 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:01:17 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 14 Eugene Grosbein freebsd_committer freebsd_triage 2018-02-02 22:09:33 UTC
Fixed some time ago: "ipfw add 1 deny icmp from any to any icmptype 5" does not match fragments, as documented.