Enable IP forwarding at the end of FW initialization
Sandor Bodo-Merle
Wouldnt make more sense to enable forwarding after the rules are in place, and not as the first step?
Btw thx for this mod_perl based captive portal. Are your plans for 0.03 up somewhere? As it seems - i might need localization support for my portal, so im wondering wether the next version will have it, or should i start working on it?
Regards,
Sanyi
PS - this patch is untested as my portal is not functioning yet ....
diff --git a/lib/App/SilverSplash/IPTables.pm b/lib/App/SilverSplash/IPTables.pm
index af09d29..36f57ec 100644
--- a/lib/App/SilverSplash/IPTables.pm
+++ b/lib/App/SilverSplash/IPTables.pm
@@ -66,8 +66,6 @@ sub load_allows {
sub init_firewall {
my $class = shift;
- `echo 1 > /proc/sys/net/ipv4/ip_forward`;
-
# flush the existing firewall
$class->clear_firewall();
@@ -203,6 +201,7 @@ NATS
iptables( sprintf( $out_rule, $mac ) );
}
+ `echo 1 > /proc/sys/net/ipv4/ip_forward`;
}
Fred Moyer
> Im on my way to deploy SilverSplash on a debian machine. Things are shaping
> up slowly. One small issue i noticed.
> Wouldnt make more sense to enable forwarding after the rules are in place,
> and not as the first step?
That sounds like a good idea, and consistent with other firewalls I've
seen. I'll apply this patch for 0.03.
> Btw thx for this mod_perl based captive portal. Are your plans for 0.03 up
> somewhere? As it seems - i might need localization support for my portal, so
> im wondering wether the next version will have it, or should i start working
> on it?
0.03 includes the following features (in development):
* Multiple payment options
* Multiple currency options
Are you talking about internationalization or localization? I'm up to
speed on best practices for internationalization, so that might be
able to go into 0.03, or 0.04.
> Regards,
> Sanyi
>
> PS - this patch is untested as my portal is not functioning yet ....
S'ok, it looks good. Let us know of any install questions you have.
>
> diff --git a/lib/App/SilverSplash/IPTables.pm
> b/lib/App/SilverSplash/IPTables.pm
> index af09d29..36f57ec 100644
> --- a/lib/App/SilverSplash/IPTables.pm
> +++ b/lib/App/SilverSplash/IPTables.pm
> @@ -66,8 +66,6 @@ sub load_allows {
> sub init_firewall {
> my $class = shift;
>
> - `echo 1 > /proc/sys/net/ipv4/ip_forward`;
> -
> # flush the existing firewall
> $class->clear_firewall();
>
> @@ -203,6 +201,7 @@ NATS
> iptables( sprintf( $out_rule, $mac ) );
> }
>
> + `echo 1 > /proc/sys/net/ipv4/ip_forward`;
> }
>
> --
> ------------------------------------------------------------
> SilverSplash is an open source captive portal developed by Silver Lining
> Networks
> http://www.slwifi.com/
>
> You received this message because you are subscribed to the Google
> Groups "Silver Splash Captive Portal" group.
> To post to this group, send email to silver...@googlegroups.com
> To unsubscribe from this group, send email to
> silversplash...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/silversplash?hl=en
>
--
Silver Lining Networks
http://slwifi.com/
http://twitter.com/slwifi
o: 888.334.6602
m: 415.720.2103
Fred Moyer
> up slowly. One small issue i noticed.
> Wouldnt make more sense to enable forwarding after the rules are in place,
> and not as the first step?
>
> Btw thx for this mod_perl based captive portal. Are your plans for 0.03 up
> somewhere? As it seems - i might need localization support for my portal, so
> im wondering wether the next version will have it, or should i start working
> on it?
>
> Regards,
> Sanyi
>
> PS - this patch is untested as my portal is not functioning yet ....
Can you refactor this into a system call? The backticks were there to
get the initial version out fast, but if we are making this
optimization change, then we should be using system().
IPC::Run3 is a good choice here also.
>
> diff --git a/lib/App/SilverSplash/IPTables.pm
> b/lib/App/SilverSplash/IPTables.pm
> index af09d29..36f57ec 100644
> --- a/lib/App/SilverSplash/IPTables.pm
> +++ b/lib/App/SilverSplash/IPTables.pm
> @@ -66,8 +66,6 @@ sub load_allows {
> sub init_firewall {
> my $class = shift;
>
> - `echo 1 > /proc/sys/net/ipv4/ip_forward`;
> -
> # flush the existing firewall
> $class->clear_firewall();
>
> @@ -203,6 +201,7 @@ NATS
> iptables( sprintf( $out_rule, $mac ) );
> }
>
> + `echo 1 > /proc/sys/net/ipv4/ip_forward`;
> }
>
Sandor Bodo-Merle
On Wed, Oct 27, 2010 at 8:05 AM, Sandor Bodo-Merle <sbodo...@gmail.com> wrote:
> Im on my way to deploy SilverSplash on a debian machine. Things are shapingCan you refactor this into a system call? The backticks were there to
> up slowly. One small issue i noticed.
> Wouldnt make more sense to enable forwarding after the rules are in place,
> and not as the first step?
>
> Btw thx for this mod_perl based captive portal. Are your plans for 0.03 up
> somewhere? As it seems - i might need localization support for my portal, so
> im wondering wether the next version will have it, or should i start working
> on it?
>
> Regards,
> Sanyi
>
> PS - this patch is untested as my portal is not functioning yet ....
get the initial version out fast, but if we are making this
optimization change, then we should be using system().
IPC::Run3 is a good choice here also.
On a second tought, you need root to enable forwarding, so i wrapped sysctl, just as iptables is wrapped. This patch is also untested (yet). One should add the 'sl_sysctl' config option - mine ponts to '/sbin/sysctl'
diff --git a/lib/App/SilverSplash/IPTables.pm b/lib/App/SilverSplash/IPTables.pm
index af09d29..e558508 100644
--- a/lib/App/SilverSplash/IPTables.pm
+++ b/lib/App/SilverSplash/IPTables.pm
@@ -15,12 +15,13 @@ use constant DEBUG => $ENV{SL_DEBUG} || 0;
our (
$Config, $Iptables, $Wan_if, %tables_chains,
$Lan_if, $Perlbal_port, $Mark_op, $Lease_file,
- $Gateway_ip, $Wan_ip, $Wan_mac,
+ $Gateway_ip, $Wan_ip, $Wan_mac, $Sysctl,
);
BEGIN {
$Config = Config::SL->new;
$Iptables = $Config->sl_iptables || die 'oops';
+ $Sysctl = $Config->sl_sysctl || die 'oops';
$Perlbal_port = $Config->sl_perlbal_port || die 'oops';
# wan and lan interfaces
@@ -66,8 +67,6 @@ sub load_allows {
sub init_firewall {
my $class = shift;
- `echo 1 > /proc/sys/net/ipv4/ip_forward`;
-
# flush the existing firewall
$class->clear_firewall();
iptables( sprintf( $out_rule, $mac ) );
}
}
sub add_rules {
@@ -239,6 +239,15 @@ sub iptables {
return 1;
}
+sub sysctl {
+ my $cmd = shift;
+ system("sudo $Sysctl $cmd" == 0
+ or require Carp
+ && Carp::confess "could not $Sysctl '$cmd', err: $!, ret: $?\n";
+
+ return 1;
+}
+
sub fixup_access {
my ( $class, $mac, $ip, $type ) = @_;
Sandor Bodo-Merle
Can you refactor this into a system call? The backticks were there to
get the initial version out fast, but if we are making this
optimization change, then we should be using system().
IPC::Run3 is a good choice here also.
My previous patch unnecesarily opened up sysctl for sudo. The reason i sent it that my apache statup.pl script does not run as root (its a self compiled apache). For upstream inclusion this might be more appropriate, as you suggested:
diff --git a/lib/App/SilverSplash/IPTables.pm b/lib/App/SilverSplash/IPTables.pm
index af09d29..99b507f 100644
--- a/lib/App/SilverSplash/IPTables.pm
+++ b/lib/App/SilverSplash/IPTables.pm
@@ -9,6 +9,7 @@ use Data::Dumper qw(Dumper);
use Config::SL ();
use URI::Escape ();
+use IPC::Run3 ();
use constant DEBUG => $ENV{SL_DEBUG} || 0;
sub init_firewall {
my $class = shift;
- `echo 1 > /proc/sys/net/ipv4/ip_forward`;
-
# flush the existing firewall
$class->clear_firewall();
iptables( sprintf( $out_rule, $mac ) );
}
}
sub add_rules {
Sandor Bodo-Merle
0.03 includes the following features (in development):
* Multiple payment options
* Multiple currency options
Are you talking about internationalization or localization? I'm up to
For my currrent deployment i'll need localization. Ill try to make patches in a way that they can be sent upstream.
Fred Moyer
>
>> Can you refactor this into a system call? The backticks were there to
>> get the initial version out fast, but if we are making this
>> optimization change, then we should be using system().
>>
>> IPC::Run3 is a good choice here also.
>>
>
> My previous patch unnecesarily opened up sysctl for sudo. The reason i sent
> it that my apache statup.pl script does not run as root (its a self compiled
> apache). For upstream inclusion this might be more appropriate, as you
> suggested:
That's ok, the application needs sudo privileges as it will always be
running as an unprivileged user.
The only reason I suggested IPC::Run3 was so that you could catch any
exceptions from the call easily, but on second though you can probably
just check the return value $!
Thanks for the revised patch. If you want to fork on github and send
pull requests, that might speed up the integration process a bit.
>
> diff --git a/lib/App/SilverSplash/IPTables.pm
> b/lib/App/SilverSplash/IPTables.pm
> index af09d29..99b507f 100644
> --- a/lib/App/SilverSplash/IPTables.pm
> +++ b/lib/App/SilverSplash/IPTables.pm
> @@ -9,6 +9,7 @@ use Data::Dumper qw(Dumper);
>
> use Config::SL ();
> use URI::Escape ();
> +use IPC::Run3 ();
>
> use constant DEBUG => $ENV{SL_DEBUG} || 0;
>
> @@ -66,8 +67,6 @@ sub load_allows {
> sub init_firewall {
> my $class = shift;
>
> - `echo 1 > /proc/sys/net/ipv4/ip_forward`;
> -
> # flush the existing firewall
> $class->clear_firewall();
>
> @@ -203,6 +202,7 @@ NATS
> iptables( sprintf( $out_rule, $mac ) );
> }
>
> + IPC::Run3::run3 "echo 1", \undef, "/proc/sys/net/ipv4/ip_forward";
> }
>
> sub add_rules {
>
>
>
>
--