From owner-freebsd-net@FreeBSD.ORG Wed Oct 31 00:47:21 2007 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 60DBA16A417; Wed, 31 Oct 2007 00:47:21 +0000 (UTC) (envelope-from matus.harvan@inf.ethz.ch) Received: from XSMTP0.ethz.ch (xsmtp0.ethz.ch [82.130.70.14]) by mx1.freebsd.org (Postfix) with ESMTP id E2F8513C48D; Wed, 31 Oct 2007 00:47:20 +0000 (UTC) (envelope-from matus.harvan@inf.ethz.ch) Received: from xfe0.d.ethz.ch ([82.130.124.40]) by XSMTP0.ethz.ch with Microsoft SMTPSVC(6.0.3790.3959); Wed, 31 Oct 2007 01:47:10 +0100 Received: from styx.inf.ethz.ch ([77.56.100.193]) by xfe0.d.ethz.ch over TLS secured channel with Microsoft SMTPSVC(6.0.3790.3959); Wed, 31 Oct 2007 01:47:10 +0100 Received: by styx.inf.ethz.ch (Postfix, from userid 1001) id F116849AC8F; Wed, 31 Oct 2007 01:47:09 +0100 (CET) Date: Wed, 31 Oct 2007 01:47:09 +0100 From: Matus Harvan To: "Bruce M. Simpson" Message-ID: <20071031004709.GB2564@styx.ethz.ch> References: <20070909201152.GA18039@inf.ethz.ch> <20071026153128.GF1049@styx.ethz.ch> <4722A8DD.6060601@FreeBSD.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tjCHc7DPkfUGtrlw" Content-Disposition: inline In-Reply-To: <4722A8DD.6060601@FreeBSD.org> User-Agent: Mutt/1.5.16 (2007-06-09) X-OriginalArrivalTime: 31 Oct 2007 00:47:10.0760 (UTC) FILETIME=[921DF280:01C81B57] Cc: freebsd-net@freebsd.org, Brooks Davis , Max Laier Subject: Re: icmp echo_user X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Oct 2007 00:47:21 -0000 --tjCHc7DPkfUGtrlw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Oct 27, 2007 at 03:56:29AM +0100, Bruce M. Simpson wrote: > Matus Harvan wrote: >> Hi, >>=20 >> I was wondering if I could get some feedback about the patch and >> whether others think it could be committed. >> =20 >=20 > Thanks for your hard work on mtund. I'm not keen on this patch going int= o=20 > a mainline kernel, though. In any case your feedback is appreciated. > It stomps on bandwidth limitation if that's in effect -- which is a=20 > possible DoS vector -- and also stops updating icmp protocol counters. I am not sure bandwidth limitation should be enforced for receiving an icmp echo. ip_icmp.c:471 if (badport_bandlim(BANDLIM_ICMP_ECHO) < 0) goto freeit; was meant to limit sending out echo replies or for limiting the receiving of echo requests? > I believe we should track echo requests in netstat -p regardless of wheth= er=20 > the kernel calls icmp_reflect() or not, as it can readily be inferred if = a)=20 > your diversion to SOCK_RAW is in effect or b) the kernel processed the ec= ho=20 > request. I do not have a strong opinion about updating the counters so I updating them is fine. You mean only icmpstat.icps_bmcastecho? Or is there another counter getting updated somewhere else? > I also believe that a user who installs and configures the tunneling daem= on=20 > is in a position to know that the ICMP thresholds need to be changed. >=20 > Assuming the tunneling daemon doesn't process echoes unrelated to its=20 > tunneling (I haven't read the code), then the fact that rip_input() may= =20 > exhaust its socket input buffer will provide a basic form of hysteresis,= =20 > however I would suggest that if you intend to deploy this on the open=20 > Internet that the daemon either a) provides its own hysteresis too, b)=20 > tunes itself around the bandwidth limit in effect or c) tunes the bandwid= th=20 > limit itself. Currently mtund tries to process anything it receives on its socket. The first byte of the received payload is used to determine the type of the traffic, so some random garbage would be discarded while other random garbage would be interpreted as a tunneled packet and passed into the tun interface. Later on it would probably fail a (higher laer protocol) checksum. The daemon tries to process as much traffic as it can. To me the bandwidth limit did not seem very useful in the mtund case. If there is "legitimate" (tunneled) traffic via ICMP then I don't think it makes sense to enforce a limit on it. In general my feeling was that the amount of data or rate of packets received on a raw socket should not be bandwidth limited. Is there a bandwidth limit on receiving for example (legitimate) UDP or TCP traffic? > A better approach would be to conditionalise the 'goto raw' next to the= =20 > 'goto reflect'. So what you're suggesting is the following, right? case ICMP_ECHO: if (!icmpbmcastecho && (m->m_flags & (M_MCAST | M_BCAST)) !=3D 0) { icmpstat.icps_bmcastecho++; break; } icp->icmp_type =3D ICMP_ECHOREPLY; if (badport_bandlim(BANDLIM_ICMP_ECHO) < 0) goto freeit; else { if (icmpechouser) goto raw; else goto reflect; } In case icmpechouser is enabled * should the packet be dropped if it was multicast/broadcast and icmpbcastecho is disabled? I guess yes. * should the packet be subject to bandwidth limitations from badport_bandlim(BANDLIM_ICMP_ECHO)? IMHO not as it is passed to a raw socket rather than the kernel producing an icmp echo reply in response. Matus --tjCHc7DPkfUGtrlw Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFHJ9CN43LQWDWf0QIRAo89AJ9l7gGuWFF1BqLQiiqj/bzGmsfMQACfapSB 7IT+OeSKxBwD06CZ6hGF6HA= =v33l -----END PGP SIGNATURE----- --tjCHc7DPkfUGtrlw--