Bug 93815 - [patch] Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw.
Summary: [patch] Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw.
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 5.4-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-02-25 04:20 UTC by kitsune
Modified: 2017-12-31 22:29 UTC (History)
0 users

See Also:


Attachments
rc.d_ipfw.patch (728 bytes, patch)
2006-02-25 04:20 UTC, kitsune
no flags Details | Diff
rc.d_ip6fw.patch (748 bytes, patch)
2006-02-25 04:20 UTC, kitsune
no flags Details | Diff
file.diff (497 bytes, patch)
2006-02-25 04:20 UTC, kitsune
no flags Details | Diff
ip6fw.diff (1.32 KB, patch)
2006-03-12 06:47 UTC, kitsune
no flags Details | Diff
ipfw.diff (1.24 KB, patch)
2006-03-12 06:47 UTC, kitsune
no flags Details | Diff
rc.firewall.diff (219 bytes, patch)
2006-03-12 06:47 UTC, kitsune
no flags Details | Diff
rc.firewall6.diff (225 bytes, patch)
2006-03-12 06:47 UTC, kitsune
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description kitsune 2006-02-25 04:20:03 UTC
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>

firewall_type has to be set to last in rc.conf for starting it with a save, as of currently.

Fix: --- /usr/src/etc/rc.firewall	Sun Nov  2 07:31:44 2003
+++ /etc/rc.firewall	Sun Feb 19 09:08:52 2006
@@ -143,6 +143,17 @@
 	setup_loopback
 	${fwcmd} add 65000 pass all from any to any
 	;;
+	
+[Ll][Aa][Ss][Tt])
+	# Gets the name of the save to use.
+	if [ ! -z $1 ]; then
+		savename="$1"
+	else
+		savename="last"
+	fi
+	
+	. /var/db/ipfw/$savename
+	;;
 
 [Cc][Ll][Ii][Ee][Nn][Tt])
 	############
--- rc.firewall.patch ends here ---
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2006-02-25 06:39:16 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-rc

Over to maintainer(s).
Comment 2 Giorgos Keramidas freebsd_committer 2006-03-05 02:54:55 UTC
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 :)
Comment 3 kitsune 2006-03-09 04:23:21 UTC
On Sun, 5 Mar 2006 04:54:55 +0200
Giorgos Keramidas <keramida@FreeBSD.org> wrote:

> 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

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.
Comment 4 Giorgos Keramidas 2006-03-09 12:16:37 UTC
On 2006-03-08 22:23, Vulpes Velox <v.velox@vvelox.net> 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 :)))
Comment 5 kitsune 2006-03-12 06:47:33 UTC
On Thu, 9 Mar 2006 14:16:37 +0200
Giorgos Keramidas <keramida@ceid.upatras.gr> wrote:

> On 2006-03-08 22:23, Vulpes Velox <v.velox@vvelox.net> 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 :)))
> 

Made a few more changes. I just got thinking of the idea of
eliminating rc.firewall and rc.firewall6 entirely. Will be sending in another patch set shortly.

This set includes load and unload. This will load or unload a set of
rules. This will unload or load a save, with out flushing.  I have
also added a new variable. fwcmd2. This is like fwcmd in rc.firewall,
but has add/delete added to it depending on what it is doing.



BTW is there any good reason this is included in
rc.firewall/rc.firewall6 instead of ipfw/ip6fw?

############
# Set quiet mode if requested
#
case ${firewall_quiet} in
[Yy][Ee][Ss])
        fwcmd="/sbin/ipfw -q"
        ;;
*)
        fwcmd="/sbin/ipfw"
        ;;
esac


I see there being no problem moving that into ipfw.
Comment 6 Chris Rees 2012-10-29 16:21:46 UTC
Nowadays we have much simpler firewall scripts.

http://www.bayofrum.net/~crees/patches/firewall-saved-rulesets.diff

What does everyone think about this?

Chris
Comment 7 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 07:59:09 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