From owner-freebsd-current@FreeBSD.ORG Mon Nov 23 15:56:18 2009 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 93E15106566C; Mon, 23 Nov 2009 15:56:18 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 650638FC15; Mon, 23 Nov 2009 15:56:18 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id EAB1C46B37; Mon, 23 Nov 2009 10:56:17 -0500 (EST) Received: from jhbbsd.hudson-trading.com (unknown [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 2F2218A01D; Mon, 23 Nov 2009 10:56:17 -0500 (EST) From: John Baldwin To: freebsd-current@freebsd.org Date: Mon, 23 Nov 2009 10:56:14 -0500 User-Agent: KMail/1.9.7 References: <4B098D21.4040607@FreeBSD.org> In-Reply-To: MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200911231056.15247.jhb@freebsd.org> Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Mon, 23 Nov 2009 10:56:17 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-net@freebsd.org, Doug Barton , Hajimu UMEMOTO Subject: Re: [CFR] unified rc.firewall X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Nov 2009 15:56:18 -0000 On Monday 23 November 2009 10:13:54 am Hajimu UMEMOTO wrote: > Hi, > > >>>>> On Sun, 22 Nov 2009 11:12:33 -0800 > >>>>> Doug Barton said: > > dougb> In rc.firewall you seem to have copied afexists() from network.subr. > dougb> Is there a reason that you did not simply source that file? That would > dougb> be the preferred method. Also in that file you call "if afexists > dougb> inet6" quite a few times. My preference from a performance standpoint > dougb> would be to call it once, perhaps in a start_precmd then cache the value. > > Thank you for the comments. > Ah, yes, afexists() is only in 9-CURRENT, and is not MFC'ed into 8, > yet. So, I thought the patch should be able to work on both 9 and 8, > for review. I've changed to source network.subr for afexists(). > Calling afexists() several times was not good idea. So, I've changed > to call afexists() just once. > The new patch is attached. > > dougb> And of course, you have regression tested this thoroughly, yes? :) > dougb> Please include scenarios where there is no INET6 in the kernel as well. > > Okay, I've tested it on INET6-less kernel, as well. Some comments I have: @@ -178,6 +212,16 @@ # Allow any traffic to or from my own net. ${fwcmd} add pass all from me to ${net} ${fwcmd} add pass all from ${net} to me + if [ -n "$net6" ]; then + ${fwcmd} add pass ip6 from me6 to ${net6} + ${fwcmd} add pass ip6 from ${net6} to me6 + fi + + if [ -n "$net6" ]; then + # Allow any link-local multicast traffic + ${fwcmd} add pass ip6 from fe80::/10 to ff02::/16 + ${fwcmd} add pass ip6 from ${net6} to ff02::/16 + fi Any reason to not use 'all' here rather than 'ip6' to match the earlier IPv4 rules? @@ -273,6 +329,55 @@ ${fwcmd} add deny all from 224.0.0.0/4 to any via ${oif} ${fwcmd} add deny all from 240.0.0.0/4 to any via ${oif} + if [ -n "$oif6" -a -n "$onet6" -a -n "$iif6" -a -n "$inet6" ]; then + # Stop unique local unicast address on the outside interface + ${fwcmd} add deny ip6 from fc00::/7 to any via ${oif6} + ${fwcmd} add deny ip6 from any to fc00::/7 via ${oif6} + .... Similarly here, why not use 'all' instead of 'ip6'? @@ -291,7 +396,11 @@ ${fwcmd} add pass tcp from any to me 80 setup # Reject&Log all setup of incoming connections from the outside - ${fwcmd} add deny log tcp from any to any in via ${oif} setup + ${fwcmd} add deny log ip4 from any to any in via ${oif} setup proto tcp + if [ -n "$oif6" -a -n "$onet6" -a -n "$iif6" -a -n "$inet6" ]; then + ${fwcmd} add deny log ip6 from any to any in via ${oif6} \ + setup proto tcp + fi I would actually not use separate v6 interfaces for the 'simple' firewall but just have 'oif', 'onet', and 'onet_ipv6' variables. Then you don't need this diff at all as the existing rule will work fine. # For services permitted below. ${fwcmd} add pass tcp from me to any established + if [ $ipv6_available -eq 0 ]; then + ${fwcmd} add pass ip6 from any to any proto tcp established + fi I think this extra rule here isn't needed at all as the first rule should already match all of those packets. # Allow any connection out, adding state for each. ${fwcmd} add pass tcp from me to any setup keep-state ${fwcmd} add pass udp from me to any keep-state ${fwcmd} add pass icmp from me to any keep-state + if [ $ipv6_available -eq 0 ]; then + ${fwcmd} add pass ip6 from me6 to any proto tcp setup + ${fwcmd} add pass ip6 from me6 to any proto udp keep-state + ${fwcmd} add pass ip6 from me6 to any proto ipv6-icmp \ + keep-state + fi I think it is more consistent to use 'pass tcp from me6 to any' similar to the IPv4 rules here. It is also shorter and easier to read that way IMO. -- John Baldwin