Bug 93815
| Summary: | [patch] Adds in the ability to save ipfw rules to rc.d/ipfw and rc.d/ip6fw. | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | kitsune <v.velox> | ||||||||||||||||
| Component: | conf | Assignee: | freebsd-bugs (Nobody) <bugs> | ||||||||||||||||
| Status: | Open --- | ||||||||||||||||||
| Severity: | Affects Only Me | Keywords: | patch | ||||||||||||||||
| Priority: | Normal | ||||||||||||||||||
| Version: | 5.4-STABLE | ||||||||||||||||||
| Hardware: | Any | ||||||||||||||||||
| OS: | Any | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
Responsible Changed From-To: freebsd-bugs->freebsd-rc Over to maintainer(s). 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 :) 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. 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 :))) 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. Nowadays we have much simpler firewall scripts. http://www.bayofrum.net/~crees/patches/firewall-saved-rulesets.diff What does everyone think about this? Chris 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 Keyword:
patch
or patch-ready
– in lieu of summary line prefix:
[patch]
* bulk change for the keyword
* summary lines may be edited manually (not in bulk).
Keyword descriptions and search interface:
<https://bugs.freebsd.org/bugzilla/describekeywords.cgi>
|
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 ---