From owner-freebsd-rc@FreeBSD.ORG Sun Mar 5 03:00:26 2006 Return-Path: X-Original-To: freebsd-rc@hub.freebsd.org Delivered-To: freebsd-rc@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 6A66B16A422 for ; Sun, 5 Mar 2006 03:00:26 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id C833D43D45 for ; Sun, 5 Mar 2006 03:00:25 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k2530P48069973 for ; Sun, 5 Mar 2006 03:00:25 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k2530PCU069972; Sun, 5 Mar 2006 03:00:25 GMT (envelope-from gnats) Date: Sun, 5 Mar 2006 03:00:25 GMT Message-Id: <200603050300.k2530PCU069972@freefall.freebsd.org> To: freebsd-rc@FreeBSD.org From: Giorgos Keramidas Cc: Subject: Re: conf/93815: Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw. X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Giorgos Keramidas List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 05 Mar 2006 03:00:26 -0000 The following reply was made to PR conf/93815; it has been noted by GNATS. From: Giorgos Keramidas To: Vulpes Velox Cc: bug-followup@FreeBSD.org Subject: Re: conf/93815: Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw. Date: Sun, 5 Mar 2006 04:54:55 +0200 On 2006-02-25 04:19, Vulpes Velox wrote: > This allows ipfw rules to be saved. /var/db/ipfw is used for that. If > a name for the save is not specified, last will be used. > > They can be saved like this... > /etc/rc.d/ipfw save > > They can be recalled like this... > /etc/rc.d/ipfw restart I feel a bit worried about allowing unquoted user-supplied names to a shell script and then using them as filenames. > --- rc.d_ipfw.patch begins here --- > 18a19,29 > > extra_commands="save" > > save_cmd="ipfw_save" > > > > > > #gets the name of the save to use > > if [ ! -z $2 ]; then > > savename="$2" > > usingsave="yes" > > else > > savename="last" > > fi Please don't. This should be written at least with a proper quote set around $2 like this: if [ -z "$2" ]; then savename="last" else savename="$2" usingsave="yes" fi > 31a43,49 > > ipfw_save() > > { > > # Saves the firewall rules to /var/db/ipfw/$savename > > [ ! -d /var/db/ipfw ] && mkdir /var/db/ipfw && chmod go-rwx /var/db/ipfw > > ipfw list | awk '{print "${fwcmd} add " $0 }' > /var/db/ipfw/$savename > > } The style sucks a bit here, but it's mostly ok. I'd probably avoid constructs that have the potential to end up using really-very-long lines, like cmd && cmd && cmd a bit and make the directory of the saved firewalls tunable through rc.conf: ipfw_save() { # set the firewall save directory if none was specified [ -z "${firewall_savedir}" ] && firewall_savedir=/var/db/ipfw if [ ! -d "${firewall_savedir}" ]; then mkdir -p "${firewall_savedir}" || return 1 fi ipfw list | sed -e 's/^/add /' > "${firewall_savedir}/${savename}" } Also, in my opinion, loading saved rulesets shouldn't be overloaded with the special 'last' savename, but supported by a similar ipfw_load() function. Then 'last' could be used as a valid savename too :) From owner-freebsd-rc@FreeBSD.ORG Mon Mar 6 11:03:21 2006 Return-Path: X-Original-To: freebsd-rc@freebsd.org Delivered-To: freebsd-rc@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7BFA416A420 for ; Mon, 6 Mar 2006 11:03:21 +0000 (GMT) (envelope-from owner-bugmaster@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3CCD743D46 for ; Mon, 6 Mar 2006 11:03:21 +0000 (GMT) (envelope-from owner-bugmaster@freebsd.org) Received: from freefall.freebsd.org (peter@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k26B3LUn098726 for ; Mon, 6 Mar 2006 11:03:21 GMT (envelope-from owner-bugmaster@freebsd.org) Received: (from peter@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k26B3Jm5098720 for freebsd-rc@freebsd.org; Mon, 6 Mar 2006 11:03:19 GMT (envelope-from owner-bugmaster@freebsd.org) Date: Mon, 6 Mar 2006 11:03:19 GMT Message-Id: <200603061103.k26B3Jm5098720@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: peter set sender to owner-bugmaster@freebsd.org using -f From: FreeBSD bugmaster To: freebsd-rc@FreeBSD.org Cc: Subject: Current problem reports assigned to you X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Mar 2006 11:03:21 -0000 Current FreeBSD problem reports Critical problems Serious problems S Submitted Tracker Resp. Description ------------------------------------------------------------------------------- o [2005/02/10] conf/77340 rc awk used in /etc/rc.d/nsswitch when not a o [2006/02/13] conf/93287 rc [patch] Make rc.subr jail-aware 2 problems total. Non-critical problems S Submitted Tracker Resp. Description ------------------------------------------------------------------------------- o [2002/11/12] conf/45226 rc Fix for rc.network, ppp-user annoyance o [2004/11/13] conf/73909 rc [patch] rc.d/sshd does not work with port o [2005/02/18] conf/77663 rc Suggestion: add /etc/rc.d/addnetswap afte o [2005/03/16] conf/78906 rc [patch] Allow mixer_enable="NO" in rc.con o [2005/05/14] kern/81006 rc ipnat not working with tunnel interfaces o [2005/06/28] conf/82738 rc [patch] add amd_program line to defaults/ o [2005/08/27] conf/85363 rc syntax error in /etc/rc.d/devfs o [2005/11/13] conf/88913 rc [patch] wrapper support for rc.subr o [2005/11/14] conf/88974 rc autoconfigured vlans confuse rc.d/netif o [2005/12/03] conf/89870 rc [patch] feature request to make netif ver o [2006/01/30] conf/92523 rc [patch] allow rc scripts to kill process o [2006/02/25] conf/93815 rc [patch] Adds in the ability to save ipfw 12 problems total. From owner-freebsd-rc@FreeBSD.ORG Thu Mar 9 04:20:09 2006 Return-Path: X-Original-To: freebsd-rc@hub.freebsd.org Delivered-To: freebsd-rc@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4769016A420 for ; Thu, 9 Mar 2006 04:20:09 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id D7F9243D48 for ; Thu, 9 Mar 2006 04:20:08 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k294K7Kh092774 for ; Thu, 9 Mar 2006 04:20:08 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k294K7Mn092773; Thu, 9 Mar 2006 04:20:07 GMT (envelope-from gnats) Date: Thu, 9 Mar 2006 04:20:07 GMT Message-Id: <200603090420.k294K7Mn092773@freefall.freebsd.org> To: freebsd-rc@FreeBSD.org From: Vulpes Velox Cc: Subject: Re: conf/93815: Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw. X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Vulpes Velox List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Mar 2006 04:20:09 -0000 The following reply was made to PR conf/93815; it has been noted by GNATS. From: Vulpes Velox To: Giorgos Keramidas Cc: bug-followup@FreeBSD.org Subject: Re: conf/93815: Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw. Date: Wed, 8 Mar 2006 22:23:21 -0600 On Sun, 5 Mar 2006 04:54:55 +0200 Giorgos Keramidas wrote: > On 2006-02-25 04:19, Vulpes Velox wrote: > > This allows ipfw rules to be saved. /var/db/ipfw is used for > > that. If a name for the save is not specified, last will be used. > > > > They can be saved like this... > > /etc/rc.d/ipfw save > > > > They can be recalled like this... > > /etc/rc.d/ipfw restart > > I feel a bit worried about allowing unquoted user-supplied names to > a shell script and then using them as filenames. > > > --- rc.d_ipfw.patch begins here --- > > 18a19,29 > > > extra_commands="save" > > > save_cmd="ipfw_save" > > > > > > > > > #gets the name of the save to use > > > if [ ! -z $2 ]; then > > > savename="$2" > > > usingsave="yes" > > > else > > > savename="last" > > > fi Cool. Fixed. > Please don't. This should be written at least with a proper quote > set around $2 like this: > > if [ -z "$2" ]; then > savename="last" > else > savename="$2" > usingsave="yes" > fi > > > 31a43,49 > > > ipfw_save() > > > { > > > # Saves the firewall rules to /var/db/ipfw/$savename > > > [ ! -d /var/db/ipfw ] && mkdir /var/db/ipfw && chmod > > > go-rwx /var/db/ipfw ipfw list | awk '{print "${fwcmd} add " > > > $0 }' > /var/db/ipfw/$savename } > > The style sucks a bit here, but it's mostly ok. I'd probably avoid > constructs that have the potential to end up using really-very-long > lines, like cmd && cmd && cmd a bit and make the directory of the > saved firewalls tunable through rc.conf: > > ipfw_save() > { > # set the firewall save directory if none was specified > [ -z "${firewall_savedir}" ] && > firewall_savedir=/var/db/ipfw > > if [ ! -d "${firewall_savedir}" ]; then > mkdir -p "${firewall_savedir}" || return 1 > fi > > ipfw list | sed -e 's/^/add /' > > "${firewall_savedir}/${savename}" } Cool. I like the that idea for the savedir. I am some what mixed about making it longer, but I see the point in making it more readable though. > Also, in my opinion, loading saved rulesets shouldn't be overloaded > with the special 'last' savename, but supported by a similar > ipfw_load() function. Then 'last' could be used as a valid > savename too :) > True. It can just be thrown in /etc/defualts/rc.conf. I will have the new patch set pr submitted tomorrow. From owner-freebsd-rc@FreeBSD.ORG Thu Mar 9 12:20:08 2006 Return-Path: X-Original-To: freebsd-rc@hub.freebsd.org Delivered-To: freebsd-rc@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2F45116A420 for ; Thu, 9 Mar 2006 12:20:08 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id B4E4E43D45 for ; Thu, 9 Mar 2006 12:20:07 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k29CK7fh017880 for ; Thu, 9 Mar 2006 12:20:07 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k29CK7do017879; Thu, 9 Mar 2006 12:20:07 GMT (envelope-from gnats) Date: Thu, 9 Mar 2006 12:20:07 GMT Message-Id: <200603091220.k29CK7do017879@freefall.freebsd.org> To: freebsd-rc@FreeBSD.org From: Giorgos Keramidas Cc: Subject: Re: conf/93815: Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw. X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Giorgos Keramidas List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Mar 2006 12:20:08 -0000 The following reply was made to PR conf/93815; it has been noted by GNATS. From: Giorgos Keramidas To: Vulpes Velox Cc: bug-followup@FreeBSD.org Subject: Re: conf/93815: Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw. Date: Thu, 9 Mar 2006 14:16:37 +0200 On 2006-03-08 22:23, Vulpes Velox wrote: > Cool. I like the that idea for the savedir. I am some what mixed > about making it longer, but I see the point in making it more > readable though. > [...] > I will have the new patch set pr submitted tomorrow. Note that the patch still has to be reviewed by one of our rc.d experts, but thank you for considering to make the changes to match some of my suggestions. Keep the good work up :)))