Date: Tue, 19 Jun 2007 06:10:08 GMT From: Sean McNeil <sean@mcneil.com> To: freebsd-ipfw@FreeBSD.org Subject: Re: conf/78762: [ipfw] [patch] /etc/rc.d/ipfw should excecute $firewall_script not read it Message-ID: <200706190610.l5J6A8RZ009491@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR conf/78762; it has been noted by GNATS.
From: Sean McNeil <sean@mcneil.com>
To: jonw@whoweb.com
Cc: bug-followup@freebsd.org
Subject: Re: conf/78762: [ipfw] [patch] /etc/rc.d/ipfw should excecute
$firewall_script not read it
Date: Mon, 18 Jun 2007 23:02:21 -0700
On Tue, 2007-06-19 at 01:12 -0400, jonw@whoweb.com wrote:
> Sourcing is intended to be used like "#include" for including libraries of
> functions and variable assignments, not running "scripts" that are
> intended to be executed. The fact that the shell executes code that is
> sourced, doesn't make it correct policy to use it as such and is
> indicative of someone finding a loophole for supporting /etc/rc.conf.d but
> forgetting the basics of real programming.
>
> Only the simplest of scripts will survive being sourced. Anyone who tries
> to build a complex script to support numerous conditions and branches is
> going to assume they can use an exit statement if they require one. I
> did. You can't call something a script and not support exiting, and
> suggesting to simply not use exit is the reason that we are discussing
> this now. Not using exit suits your requirements for including options
> from /etc/rc.conf.d fine, but doesn't suit my needs to actually execute a
> script that has conditions and branches based upon various OS
> configurations and from which I might need to exit immediately if certain
> conditions are met.
>
> It's wrong to call something a script (ie firewall_script) and treat it
> like an include file, so reverting to the previous functionality is not
> the correct solution. I must be missing something regarding your
> variables from rc.conf.d/ipfw not being included in the ipfw script. The
> load_rc_config routine looks for /etc/rc.conf.d/ipfw and sources that in
> before executing the startup code. Executing or sourcing firewall_script
> shouldn't have any impact on the rc.conf.d/ipfw variables.
>
> It sounds to me like the correct solution is to support both includes and
> executables. That can be done a couple of ways, maybe more.
>
> 1) If firewall_script is defined, execute it. If firewall_include is
> defined, source it.
>
> 2) Check the mode of firewall_script. If it's executable, execute it. If
> it's not executable, source it.
>
> Jon
Thank you, Jon. I like your suggestion. Indeed, having something named
_script and sourcing it would be misleading. The problem is that when
you execute the script you lose all assignments made in /etc/rc.d/ipfw
and /etc/rc.firewall only sources /etc/rc.conf and /etc/rc.conf.local.
Actually, I think your suggestion should have been applied and
firewall_script should have been executed without forcing the shell with
the /bin/sh or ".". That way you can direct which shell to use in the
script (#!/bin/sh, #!/bin/csh) or in the assignment of $firewall_script
and the default could be changed from "/etc/rc.firewall" to
". /etc/rc.firewall". Except then something would have to be done with
the -z and -r tests, so that wouldn't quite work as is.
Funny how such a little thing can become so complicated.
If I understand you correctly about read vs. execute, it should look
something like
if [ -x "${firewall_script}" ]; then
"${firewall_script}"
else
. "${firewall_script}"
fi
This would restore the rcNG /etc/rc.conf.d/ipfw setting ability and
allow you to use the shell of your choosing.
>
> > This is a bad idea and has broken the new feature of rcNG allowing us to
> > place options into /etc/rc.conf.d/ipfw and /etc/rc.conf.d/ip6fw. The
> > commit to src/etc/rc.d/ipfw revision 1.15 and src/etc/rc.d/ip6fw 1.9
> > have now broken this basic concept.
> >
> > IMHO, the correct thing is: Don't use exit in your firewall script. I
> > offer 3 solutions, however, below.
> >
> > What has been broken:
> >
> > /etc/rc.conf.d/ipfw
> > firewall_enable="YES"
> > firewall_type="/etc/fw/rc.firewall.rules"
> >
> > /etc/rc.conf.d/ip6fw
> > ipv6_firewall_enable="YES"
> > ipv6_firewall_type="/etc/fw/rc.firewall6.rules"
> >
> > Now, this no longer works and I must once again pollute and move more
> > stuff back into /etc/rc.conf. Namely,
> >
> > firewall_type="/etc/fw/rc.firewall.rules"
> > ipv6_firewall_type="/etc/fw/rc.firewall6.rules"
> >
> > must now be in /etc/rc.conf or /etc/rc.conf.local.
> >
> > Solution:
> >
> > 1) revert to sourcing the rc.firewall script.
> > 2) Fix rc.firewall and rc.firewall6 to somehow get stuff
> > from /etc/rc.conf.d as it should (as ipfw and ip6fw?).
> > 3) completely remove rc.conf.d support as more things fail to work with
> > it.
> >
> >
> >
>
>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200706190610.l5J6A8RZ009491>