Date: Sun, 5 Mar 2006 03:00:25 GMT From: Giorgos Keramidas <keramida@FreeBSD.org> To: freebsd-rc@FreeBSD.org Subject: Re: conf/93815: Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw. Message-ID: <200603050300.k2530PCU069972@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR conf/93815; it has been noted by GNATS.
From: Giorgos Keramidas <keramida@FreeBSD.org>
To: Vulpes Velox <v.velox@vvelox.net>
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 <v.velox@vvelox.net> 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 <name>
>
> They can be recalled like this...
> /etc/rc.d/ipfw restart <name>
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 :)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200603050300.k2530PCU069972>