Shorewall devel - Nov 2004 - [PATCH] Configurable variable for iptables executable

Hello,

I want to test Shorewall''s IPsec feature. It requires patched netfilter
(and kernel but that''s another story). I didn''t want to
replace my
distribution''s iptables package with my own compilation so I installed 
it to another prefix. Now Shoreall uses the iptables command found 
first in $PATH. I don''t think this is the most elegant way. I think the
command should be configurable in shorewall.conf. So I patched my 
shorewall installation with this rather large but very straightforward 
patch which allows setting used iptables executable with new IPTABLES 
variable. Default is /sbin/iptables which I believe is used by all 
Linux distributions.  If you like the idea, feel free to use the patch. 
It requires some discipline in the future, though, since you should use 
$IPTABLES instead of just iptables when running the program in upcoming 
additions to the scripts though. Also I know that you probably could''ve
effectively achieved my requirements by modifying PATH in 
shorewall.conf. But I still feel that this patch proposes safer and 
more transparent way to implement things. For example one might want to 
use your distribution''s /sbin/iptables to manage the firewall settings 
but instead of calling it directly, one wants to use a wrapper script 
/usr/local/sbin/iptables-with-logging which logs the iptables arguments 
and then executes the actual program. With current scheme this isn''t 
nicely doable since it would require one to put /usr/local/sbin before 
/sbin in $PATH and that might not be a fun thing to do. With the patch 
applied, one would keep $PATH intact and neatly put 
IPTABLES=/usr/local/sbin/iptables-with-logging to shorewall.conf and be 
happy. Comments?

Best regards,

-- 
Tuomas Jormola <tjormola@cc.hut.fi>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: shorewall-2.2.0-Beta4-iptables.diff
Type: application/octet-stream
Size: 18576 bytes
Desc: not available
Url :
http://lists.shorewall.net/pipermail/shorewall-devel/attachments/20041124/8c926896/shorewall-2.2.0-Beta4-iptables.obj

On Wed, 2004-11-24 at 11:14 +0200, Tuomas Jormola wrote:
> Comments?
a) One must deal with PATH every day -- I don''t believe the reasons you
cite necessarily justify having a separate variable for the iptables
path. And if you believe that they do, then you need a variable for each
of the utilities that Shorewall uses.

If you want to run a special iptables, why not install it
in /usr/local/iptables/sbin (and /usr/local/iptables/lib) and
place /usr/local/iptables/sbin at the beginning of PATH? There could be
no bad side effects from doing that and the ''prefix''
configuration
variable allows you to set that up easily when you build iptables.

b) If you are going to patch Shorewall to add an IPTABLES variable then
you must patch /usr/share/shorewall/functions and /sbin/shorewall as
well as /usr/share/shorewall/firewall.

c) You make no provision for IPTABLES not being set. So your patch would
require all users to modify their /etc/shorewall/shorewall.conf file
after upgrading before they could start Shorewall.

d) I''m too late in the development cycle for 2.2.0 to consider such a
potentially disruptive patch, even if I did like it. I will think about
it though and will reconsider when I start development of 2.3.

Thanks,
-Tom
-- 
Tom Eastep    \ Nothing is foolproof to a sufficiently talented fool
Shoreline,     \ http://shorewall.net
Washington USA  \ teastep@shorewall.net
PGP Public Key   \ https://lists.shorewall.net/teastep.pgp.key

On 24.11.2004, at 17:50, Tom Eastep wrote:
> On Wed, 2004-11-24 at 11:14 +0200, Tuomas Jormola wrote:
>> Comments?
>
> a) One must deal with PATH every day -- I don''t believe the
reasons you
> cite necessarily justify having a separate variable for the iptables
> path. And if you believe that they do, then you need a variable for 
> each
> of the utilities that Shorewall uses.
Well, I don''t think this is a valid point. I hardly doubt
there''s need
for $GREP or $CUT for instance. grep is plain and simple grep, why 
would someone use wrapper script for grep? But as for complex iptables, 
the core of Shorewall, it''s another story. There are valid situations 
when you might want more flexibility regarding iptables.
> If you want to run a special iptables, why not install it
> in /usr/local/iptables/sbin (and /usr/local/iptables/lib) and
> place /usr/local/iptables/sbin at the beginning of PATH? There could be
> no bad side effects from doing that and the ''prefix''
configuration
> variable allows you to set that up easily when you build iptables.
Yes, you are right, this would solve my current problem. But what if 
you wanted to use wrapper script for iptables which eventually executes 
the default /sbin/iptables? Currently you should put some path at the 
beginning of PATH and name your wrapper script ''iptables''. Now
wouldn''t
that be confusing? I''d rather call it something else which implies
it''s
not the real iptables executable.
> b) If you are going to patch Shorewall to add an IPTABLES variable then
> you must patch /usr/share/shorewall/functions and /sbin/shorewall as
> well as /usr/share/shorewall/firewall.
/sbin/shorewall is patched. /usr/share/shorewall/functions doesn''t seem
to execute iptables so no need for patching. Sorry if I missed 
something...
> c) You make no provision for IPTABLES not being set. So your patch 
> would
> require all users to modify their /etc/shorewall/shorewall.conf file
> after upgrading before they could start Shorewall.
This is incorrect. In /sbin/shorewall this code is added after the 
config file is processed:
+    [ -n "$IPTABLES" ] || IPTABLES=/sbin/iptables
+    if [ ! -e "$IPTABLES" ]; then
+	echo "   ERROR: The program specified in IPTABLES does not exist or 
is not executable" >&2
+	exit 2
+    fi

and /usr/share/shorewall/firewall:
+    [ -z "$IPTABLES" ] && IPTABLES=/sbin/iptables
+

So if no IPTABLES is set in the config file, /sbin/iptables is used. 
Users wouldn''t need to update their config, everything would work as 
before upgrade. But for sure in the case they''ve not modified PATH, 
though. Now that I think more of it, the default value should be just 
"iptables" without full path so everything would be as before even if 
PATH is modified and desired iptables executable isn''t /sbin/iptables 
and IPTABLES is not set in the config. In this case also the added "if 
[ ! -e "$IPTABLES" ]" check in /sbin/firewall must be removed / 
altered. I''ll rewrite these bits and then I can submit a new patch if 
you end up being interested in this thing after all.
> d) I''m too late in the development cycle for 2.2.0 to consider
such a
> potentially disruptive patch, even if I did like it. I will think about
> it though and will reconsider when I start development of 2.3.
This is quite intelligible point, I agree.

-- 
Tuomas Jormola <tjormola@cc.hut.fi>

On Wed, 2004-11-24 at 22:11 +0200, Tuomas Jormola wrote:
> 
> > If you want to run a special iptables, why not install it
> > in /usr/local/iptables/sbin (and /usr/local/iptables/lib) and
> > place /usr/local/iptables/sbin at the beginning of PATH? There could
be
> > no bad side effects from doing that and the ''prefix''
configuration
> > variable allows you to set that up easily when you build iptables.
> Yes, you are right, this would solve my current problem. But what if 
> you wanted to use wrapper script for iptables which eventually executes 
> the default /sbin/iptables? Currently you should put some path at the 
> beginning of PATH and name your wrapper script
''iptables''. Now wouldn''t
> that be confusing? I''d rather call it something else which implies
it''s
> not the real iptables executable.
I''m not sure that your preference warrents patching Shorewall.
> 
> > b) If you are going to patch Shorewall to add an IPTABLES variable
then
> > you must patch /usr/share/shorewall/functions and /sbin/shorewall as
> > well as /usr/share/shorewall/firewall.
> /sbin/shorewall is patched. /usr/share/shorewall/functions doesn''t
seem
> to execute iptables so no need for patching. Sorry if I missed 
> something...
> 
No, you''re right.
> > c) You make no provision for IPTABLES not being set. So your patch 
> > would
> > require all users to modify their /etc/shorewall/shorewall.conf file
> > after upgrading before they could start Shorewall.
> This is incorrect. In /sbin/shorewall this code is added after the 
> config file is processed:
> +    [ -n "$IPTABLES" ] || IPTABLES=/sbin/iptables
> +    if [ ! -e "$IPTABLES" ]; then
> +	echo "   ERROR: The program specified in IPTABLES does not exist or 
> is not executable" >&2
> +	exit 2
> +    fi
> 
> and /usr/share/shorewall/firewall:
> +    [ -z "$IPTABLES" ] && IPTABLES=/sbin/iptables
> +
> 
> So if no IPTABLES is set in the config file, /sbin/iptables is used. 
Sorry -- missed that. 

OTOH, in my case that code would break things since I currently run
iptables from /usr/local/sbin. A better approach is:

	[ -z "$IPTABLES"] && IPTABLES=$(which iptables)

In other words, fall back to PATH if no IPTABLES is specified.
> > d) I''m too late in the development cycle for 2.2.0 to
consider such a
> > potentially disruptive patch, even if I did like it. I will think
about
> > it though and will reconsider when I start development of 2.3.
> This is quite intelligible point, I agree.
> 
As I said, I''ll reconsider this for 2.3. I often have to think about
these things a while before accepting the idea :-)

Thanks again,
-Tom
-- 
Tom Eastep    \ Nothing is foolproof to a sufficiently talented fool
Shoreline,     \ http://shorewall.net
Washington USA  \ teastep@shorewall.net
PGP Public Key   \ https://lists.shorewall.net/teastep.pgp.key

Tom Eastep wrote:
> On Wed, 2004-11-24 at 22:11 +0200, Tuomas Jormola wrote:
> 
> 
>>>If you want to run a special iptables, why not install it
>>>in /usr/local/iptables/sbin (and /usr/local/iptables/lib) and
>>>place /usr/local/iptables/sbin at the beginning of PATH? There could
be
>>>no bad side effects from doing that and the
''prefix'' configuration
>>>variable allows you to set that up easily when you build iptables.
>>
>>Yes, you are right, this would solve my current problem. But what if 
>>you wanted to use wrapper script for iptables which eventually executes 
>>the default /sbin/iptables? Currently you should put some path at the 
>>beginning of PATH and name your wrapper script
''iptables''. Now wouldn''t
>>that be confusing? I''d rather call it something else which
implies it''s
>>not the real iptables executable.
> 
> 
> I''m not sure that your preference warrents patching Shorewall.
What happened to the good old days when Tom would just change fw to $FW 
on request from me, and add patches that consisted of blank lines so vi 
paragraph move commands work?  :-)

Nowadays i rely on $FW functionality a lot, and it''s possible others
may
come to do the same with Tuomas'' patch.

-- 
Paul
<http://paulgear.webhop.net>
--
I''m a great believer in luck, and I find the harder I work, the more I
have of it.
	-- Thomas Jefferson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url :
http://lists.shorewall.net/pipermail/shorewall-devel/attachments/20041126/b5a2d9c1/signature.bin

On Fri, 2004-11-26 at 06:44 +1000, Paul Gear wrote:
> > 
> > 
> > I''m not sure that your preference warrents patching
Shorewall.
> 
> What happened to the good old days when Tom would just change fw to $FW 
> on request from me, and add patches that consisted of blank lines so vi 
> paragraph move commands work?  :-)
> 
> Nowadays i rely on $FW functionality a lot, and it''s possible
others may
> come to do the same with Tuomas'' patch.
As I quote on the author''s page:

        Perfection in design is achieved not when there is nothing left
        to add, but rather when there is nothing left to take away.
        
                                                Antoine de Saint-Exupery

We already have PATH which serves a very similar purpose; with both
IPTABLES and PATH, I think that there is something "left to take
away".

-Tom
-- 
Tom Eastep    \ Nothing is foolproof to a sufficiently talented fool
Shoreline,     \ http://shorewall.net
Washington USA  \ teastep@shorewall.net
PGP Public Key   \ https://lists.shorewall.net/teastep.pgp.key

On 25.11.2004, at 22:59, Tom Eastep wrote:
> On Fri, 2004-11-26 at 06:44 +1000, Paul Gear wrote:
>
>>>
>>>
>>> I''m not sure that your preference warrents patching
Shorewall.
>>
>> What happened to the good old days when Tom would just change fw to 
>> $FW
>> on request from me, and add patches that consisted of blank lines so 
>> vi
>> paragraph move commands work?  :-)
>>
>> Nowadays i rely on $FW functionality a lot, and it''s possible
others
>> may
>> come to do the same with Tuomas'' patch.
Thanks for support but I think Tom''s doing great job here. Responsible 
maintainer should review patches with grain of salt especially if she 
or he isn''t completely convinced of the usefulness of the changes. But 
maybe her or his views can be influenced with some rational piece of 
discussion and that''s what devel mailing lists are for :)
> As I quote on the author''s page:
>
>         Perfection in design is achieved not when there is nothing left
>         to add, but rather when there is nothing left to take away.
>
>                                                 Antoine de 
> Saint-Exupery
>
> We already have PATH which serves a very similar purpose; with both
> IPTABLES and PATH, I think that there is something "left to take
away".
Alright then, let''s get rid of PATH :) Shorewall is already started 
from shell of somekind (either by init scripts or by admin with 
interactive shell). What ever the method is, then already you can 
adjust PATH visible to shorewall, either globally for the whole shell 
session and thus for all its parent processes, which also shorewall is, 
or specially for shorewall when executing the command. Why do you think 
it''s a good idea to mess with PATH inside Shorewall in the first place?
Fine, I can think of one valid and actually recommended reason why you 
might want to do this. If you''re writing safe shell scripts
it''s
recommended that the script sets minimal PATH with which it can operate 
at the beginning execution before calling any external programs. Now 
this security aspect is invalidated if it''s suggested that those who 
want special functionality should alter the runtime PATH of the script 
as the whole point is to have a minimal, read-only PATH. What''s left is
an extraneous PATH setting is Shorewall. Just some food for thought :)

Btw to what preference of mine did you refer? Using a variable for 
iptables command vs. altering PATH or using unique names for wrapper 
scripts? Because in former there''s always room for discussion,
we''re
doing it right now, but your stand doesn''t provide meaningful solution 
to the latter. I hope you''re not suggesting that wrapper scripts should
be named identically with with actual command, what if you want to 
place both in the same directory?

-- 
Tuomas Jormola <tjormola@cc.hut.fi>

On Fri, 2004-11-26 at 00:06 +0200, Tuomas Jormola wrote:
> On 25.11.2004, at 22:59, Tom Eastep wrote:
> > We already have PATH which serves a very similar purpose; with both
> > IPTABLES and PATH, I think that there is something "left to take
away".
> Alright then, let''s get rid of PATH :) 
We can''t just do away with PATH without modifying the installer to
locate the executable ''iptables'' and set IPTABLES accordingly.
Your
original patch set IPTABLES=/sbin/iptables which would not work on SuSE
where iptables is installed in /usr/sbin/iptables!

I have applied your patch but I''ve changed it:

1) to use PATH to locate the iptables executable in the case where
IPTABLES isn''t set.
2) to set IPTABLES="" in shorewall.conf

This will:

a) Maintain combatibility with existing configurations.
b) Allow Shorewall to work "out of the box" without having to set
IPTABLES.

I''ll release Beta 5 later in the day.

Thanks,
-Tom
-- 
Tom Eastep    \ Nothing is foolproof to a sufficiently talented fool
Shoreline,     \ http://shorewall.net
Washington USA  \ teastep@shorewall.net
PGP Public Key   \ https://lists.shorewall.net/teastep.pgp.key

On 26.11.2004, at 20:39, Tom Eastep wrote:
> I have applied your patch but I''ve changed it:
Thank you!

I try to re-test it soon.

-- 
Tuomas Jormola <tjormola@cc.hut.fi>