Commit 38d45fd5 authored by Nigel Kukard's avatar Nigel Kukard

Merge branch 'nk-bugfixes3' into 'master'

Another round of bugfixes



See merge request !421
parents c0f73d5e 0dcb75d1
Pipeline #299 passed with stages
in 3 minutes and 37 seconds
......@@ -25,11 +25,13 @@ use warnings;
# Exporter stuff
use base qw(Exporter);
our (@EXPORT);
@EXPORT = qw(
our @EXPORT = qw(
);
our @EXPORT_OK = qw(
);
use AWITPT::Util;
use smradius::logging;
......@@ -70,10 +72,8 @@ sub Init
# Should we use the packet timestamp?
if (defined($config->{'radius'}{'use_packet_timestamp'})) {
if ($config->{'radius'}{'use_packet_timestamp'} =~ /^\s*(yes|true|1)\s*$/i) {
$server->{'smradius'}{'use_packet_timestamp'} = 1;
} elsif ($config->{'radius'}{'use_packet_timestamp'} =~ /^\s*(no|false|0)\s*$/i) {
$server->{'smradius'}{'use_packet_timestamp'} = 0;
if (defined(my $val = isBoolean($config->{'radius'}{'use_packet_timestamp'}))) {
$server->{'smradius'}{'use_packet_timestamp'} = $val;
} else {
$server->log(LOG_NOTICE,"smradius/config.pm: Value for 'use_packet_timestamp' is invalid");
}
......@@ -83,10 +83,8 @@ sub Init
# Should we use abuse prevention?
if (defined($config->{'radius'}{'use_abuse_prevention'})) {
if ($config->{'radius'}{'use_abuse_prevention'} =~ /^\s*(yes|true|1)\s*$/i) {
$server->{'smradius'}{'use_abuse_prevention'} = 1;
} elsif ($config->{'radius'}{'use_abuse_prevention'} =~ /^\s*(no|false|0)\s*$/i) {
$server->{'smradius'}{'use_abuse_prevention'} = 0;
if (defined(my $val = isBoolean($config->{'radius'}{'use_abuse_prevention'}))) {
$server->{'smradius'}{'use_abuse_prevention'} = $val;
} else {
$server->log(LOG_NOTICE,"smradius/config.pm: Value for 'use_abuse_prevention' is invalid");
}
......
......@@ -590,7 +590,10 @@ sub process_request {
my $request = smradius::daemon::request->new($self);
$request->setTimeZone($self->{'smradius'}->{'event_timezone'});
if (!$request->setTimezone($self->{'smradius'}->{'event_timezone'})) {
$self->log(LOG_ERR,"[SMRADIUS] Setting event_timezone to '%s' failed",$self->{'smradius'}->{'event_timezone'});
return;
}
$request->parsePacket($self->{'radius'}->{'dictionary'},$rawPacket);
......@@ -612,20 +615,38 @@ sub process_request {
my $logReason = "UNKNOWN";
# First thing we do is to make sure the NAS behaves if we using abuse prevention
if ($self->{'smradius'}->{'use_abuse_prevention'} && defined($user->{'Username'})) {
my ($res,$val) = cacheGetKeyPair('FloodCheck',$server->{'peeraddr'}."/".$user->{'Username'}."/".$pkt->code);
if (defined($val)) {
my $timePeriod = $now - $val;
# Check if we're still within the abuse threshold
if ($pkt->code eq "Access-Request" && $timePeriod < $self->{'smradius'}->{'access_request_abuse_threshold'}) {
$self->log(LOG_NOTICE,"[SMRADIUS] ABUSE: Server trying too fast. server = ".$server->{'peeraddr'}.", user = ".$user->{'Username'}.
", code = ".$pkt->code.", timeout = ".($now - $val));
# Tell the NAS we got its packet
my $resp = smradius::Radius::Packet->new($self->{'radius'}->{'dictionary'});
$resp->set_code('Access-Reject');
$resp->set_identifier($pkt->identifier);
$resp->set_authenticator($pkt->authenticator);
$server->{'client'}->send(
auth_resp($resp->pack, getAttributeValue($user->{'ConfigAttributes'},"SMRadius-Config-Secret"))
);
return;
} elsif ($pkt->code eq "Accounting-Request" && $timePeriod < $self->{'smradius'}->{'accounting_request_abuse_threshold'}) {
$self->log(LOG_NOTICE,"[SMRADIUS] ABUSE: Server trying too fast. server = ".$server->{'peeraddr'}.", user = ".$user->{'Username'}.
", code = ".$pkt->code.", timeout = ".($now - $val));
# Tell the NAS we got its packet
my $resp = smradius::Radius::Packet->new($self->{'radius'}->{'dictionary'});
$resp->set_code('Accounting-Response');
$resp->set_identifier($pkt->identifier);
$resp->set_authenticator($pkt->authenticator);
$server->{'client'}->send(
auth_resp($resp->pack, getAttributeValue($user->{'ConfigAttributes'},"SMRadius-Config-Secret"))
);
return;
}
}
# We give the benefit of the doubt and let a query take 60s. We update to right stamp at end of this function
......@@ -1254,9 +1275,9 @@ sub process_request {
$request->addLogLine(
"%s/%s: %s",
$vendor,
$attrName,
$attrVal,
$resp->rawattr($attrName)
$attrVal
);
}
}
......
......@@ -25,6 +25,10 @@ use warnings;
use base qw{AWITPT::Object};
use DateTime;
use DateTime::TimeZone;
use Try::Tiny;
use smradius::Radius::Packet;
......@@ -83,7 +87,7 @@ sub setTimestamp
# Grab real event timestamp in local time uzing the time zone
my $eventTimestamp = DateTime->from_epoch(
epoch => $self->{'user'}->{'_Internal'}->{'Timestamp-Unix'},
time_zone => $self->{'timeZone'},
time_zone => $self->{'timezone'},
);
# Set the timestamp (not in unix)
$self->{'user'}->{'_Internal'}->{'Timestamp'} = $eventTimestamp->strftime('%Y-%m-%d %H:%M:%S');
......@@ -94,12 +98,20 @@ sub setTimestamp
# Set internal time zone
sub setTimeZone
sub setTimezone
{
my ($self,$timeZone) = @_;
my ($self,$timezone) = @_;
my $timezone_obj;
try {
$timezone_obj = DateTime::TimeZone->new('name' => $timezone);
};
# Retrun if we don't have a value, this means we failed
return if (!defined($timezone_obj));
$self->{'timeZone'} = $timeZone;
$self->{'timezone'} = $timezone_obj;
return $self;
}
......@@ -147,7 +159,7 @@ sub _init
$self->{'logLine'} = [ ];
$self->{'logLineParams'} = [ ];
$self->{'timeZone'} = "UTC";
$self->{'timezone'} = "UTC";
# Initialize user
$self->{'user'} = {
......
......@@ -299,10 +299,15 @@ sub init
}
}
if (defined($scfg->{'mod_accounting_sql'}->{'accounting_usage_cache_time'})) {
if ($scfg->{'mod_accounting_sql'}{'accounting_usage_cache_time'} =~ /^\s*(yes|true|1)\s*$/i) {
# Default?
} elsif ($scfg->{'mod_accounting_sql'}{'accounting_usage_cache_time'} =~ /^\s*(no|false|0)\s*$/i) {
$config->{'accounting_usage_cache_time'} = undef;
# Check if we're a boolean
if (defined(my $val = isBoolean($scfg->{'mod_accounting_sql'}{'accounting_usage_cache_time'}))) {
# If val is true, we default to the default anyway
# We're disabled
if (!$val) {
$config->{'accounting_usage_cache_time'} = undef;
}
# We *could* have a value...
} elsif ($scfg->{'mod_accounting_sql'}{'accounting_usage_cache_time'} =~ /^[0-9]+$/) {
$config->{'accounting_usage_cache_time'} = $scfg->{'mod_accounting_sql'}{'accounting_usage_cache_time'};
} else {
......
......@@ -73,20 +73,33 @@ sub init
my $scfg = $server->{'inifile'};
# Defaults
$config->{'enable_mikrotik'} = 0;
$config->{'caveat_captrafzero'} = 0;
# Setup SQL queries
if (defined($scfg->{'mod_feature_capping'})) {
# Check if option exists
if (defined($scfg->{'mod_feature_capping'}{'enable_mikrotik'})) {
# Pull in config
if ($scfg->{'mod_feature_capping'}{'enable_mikrotik'} =~ /^\s*(yes|true|1)\s*$/i) {
$server->log(LOG_NOTICE,"[MOD_FEATURE_CAPPING] Mikrotik-specific vendor return attributes ENABLED");
$config->{'enable_mikrotik'} = $scfg->{'mod_feature_capping'}{'enable_mikrotik'};
# Default?
} elsif ($scfg->{'mod_feature_capping'}{'enable_mikrotik'} =~ /^\s*(no|false|0)\s*$/i) {
$config->{'enable_mikrotik'} = undef;
if (defined(my $val = isBoolean($scfg->{'mod_feature_capping'}{'enable_mikrotik'}))) {
if ($val) {
$server->log(LOG_NOTICE,"[MOD_FEATURE_CAPPING] Mikrotik-specific vendor return attributes ENABLED");
$config->{'enable_mikrotik'} = $val;
}
} else {
$server->log(LOG_NOTICE,"[MOD_FEATURE_CAPPING] Value for 'enable_mikrotik' is invalid");
}
if (defined(my $val = isBoolean($scfg->{'mod_feature_capping'}{'caveat_captrafzero'}))) {
if ($val) {
$server->log(LOG_NOTICE,"[MOD_FEATURE_CAPPING] Caveat to swap '0' and -undef- for ".
"SMRadius-Capping-Traffic-Limit ENABLED");
$config->{'caveat_captrafzero'} = $val;
}
} else {
$server->log(LOG_NOTICE,"[MOD_FEATURE_CAPPING] Value for 'caveat_captrafzero' is invalid");
}
}
}
......@@ -121,6 +134,15 @@ sub post_auth_hook
my $uptimeLimit = _getAttributeKeyLimit($server,$user,$UPTIME_LIMIT_ATTRIBUTE);
my $trafficLimit = _getAttributeKeyLimit($server,$user,$TRAFFIC_LIMIT_ATTRIBUTE);
# Swap around 0 and undef if we need to apply the captrafzero caveat
if ($config->{'caveat_captrafzero'}) {
if (!defined($trafficLimit)) {
$trafficLimit = 0;
} elsif ($trafficLimit == 0) {
$trafficLimit = undef;
}
}
#
# Get current traffic and uptime usage
......@@ -225,8 +247,8 @@ sub post_auth_hook
# Check if we've exceeded our limits
#
# Uptime..
if (!defined($uptimeLimit) || $uptimeLimit > 0) {
# Uptime...
if (defined($uptimeLimit)) {
# Check session time has not exceeded what we're allowed
if ($accountingUsage->{'TotalSessionTime'} >= $uptimeLimitWithTopups) {
......@@ -237,7 +259,7 @@ sub post_auth_hook
} else {
# Check if we returning Mikrotik vattributes
# FIXME: NK - this is not mikrotik specific
if (defined($config->{'enable_mikrotik'})) {
if ($config->{'enable_mikrotik'}) {
# FIXME: NK - We should cap the maximum total session time to that which is already set, if something is set
# Setup reply attributes for Mikrotik HotSpots
my %attribute = (
......@@ -251,7 +273,7 @@ sub post_auth_hook
}
# Traffic
if (!defined($trafficLimit) || $trafficLimit > 0) {
if (defined($trafficLimit)) {
# Capped
if ($accountingUsage->{'TotalDataUsage'} >= $trafficLimitWithTopups) {
......@@ -261,7 +283,7 @@ sub post_auth_hook
# Setup limits
} else {
# Check if we returning Mikrotik vattributes
if (defined($config->{'enable_mikrotik'})) {
if ($config->{'enable_mikrotik'}) {
# Get remaining traffic
my $remainingTraffic = $trafficLimitWithTopups - $accountingUsage->{'TotalDataUsage'};
my $remainingTrafficLimit = ( $remainingTraffic % 4096 ) * 1024 * 1024;
......@@ -328,6 +350,14 @@ sub post_acct_hook
my $uptimeLimit = _getAttributeKeyLimit($server,$user,$UPTIME_LIMIT_ATTRIBUTE);
my $trafficLimit = _getAttributeKeyLimit($server,$user,$TRAFFIC_LIMIT_ATTRIBUTE);
# Swap around 0 and undef if we need to apply the captrafzero caveat
if ($config->{'caveat_captrafzero'}) {
if (!defined($trafficLimit)) {
$trafficLimit = 0;
} elsif ($trafficLimit == 0) {
$trafficLimit = undef;
}
}
#
# Get current traffic and uptime usage
......@@ -420,7 +450,7 @@ sub post_acct_hook
#
# Uptime..
if (!defined($uptimeLimit) || $uptimeLimit > 0) {
if (defined($uptimeLimit)) {
# Capped
if ($accountingUsage->{'TotalSessionTime'} >= $uptimeLimitWithTopups) {
......@@ -431,7 +461,7 @@ sub post_acct_hook
}
# Traffic
if (!defined($trafficLimit) || $trafficLimit > 0) {
if (defined($trafficLimit)) {
# Capped
if ($accountingUsage->{'TotalDataUsage'} >= $trafficLimitWithTopups) {
......@@ -512,10 +542,10 @@ sub _logUsage
my $typeKey = ucfirst($type);
# Check if our limit is defined
if (defined($limit) && !$limit) {
$limit = '-none-';
} else {
if (defined($limit) && $limit == 0) {
$limit = '-topup-';
} else {
$limit = '-none-';
}
$server->log(LOG_INFO,"[MOD_FEATURE_CAPPING] Capping information [type: %s, total: %s, limit: %s, topups: %s]",
......
......@@ -225,10 +225,14 @@ sub init
}
if (defined($scfg->{'mod_userdb_sql'}->{'userdb_data_cache_time'})) {
if ($scfg->{'mod_userdb_sql'}{'userdb_data_cache_time'} =~ /^\s*(yes|true|1)\s*$/i) {
# Default?
} elsif ($scfg->{'mod_userdb_sql'}{'userdb_data_cache_time'} =~ /^\s*(no|false|0)\s*$/i) {
$config->{'userdb_data_cache_time'} = undef;
if (defined(my $val = isBoolean($scfg->{'mod_userdb_sql'}{'userdb_data_cache_time'}))) {
# If val is true, we default to the default anyway
# We're disabled
if (!$val) {
$config->{'userdb_data_cache_time'} = undef;
}
# We *could* have a value...
} elsif ($scfg->{'mod_userdb_sql'}{'userdb_data_cache_time'} =~ /^[0-9]+$/) {
$config->{'userdb_data_cache_time'} = $scfg->{'mod_userdb_sql'}{'userdb_data_cache_time'};
} else {
......
......@@ -544,6 +544,10 @@ EOT
# MOD_FEATURE_CAPPING
[mod_feature_capping]
# Enable Mikrotik-specific return vattributes
#enable_mikrotik=1
# Enable caveat for SMRadius-Capping-Traffic-Limit having the meaning of 0 and -undef- swapped up to v1.0.x
#caveat_captrafzero=1
......@@ -168,7 +168,7 @@ if ($child = fork()) {
#
# Check we get an Access-Reject for an unconfigured user
# Check we get an Access-Accept for a bare user
#
my $user1_ID = testDBInsert("Create user 'testuser1'",
......@@ -189,12 +189,12 @@ if ($child = fork()) {
'User-Password=test123',
);
is(ref($res),"HASH","smradclient should return a HASH");
is($res->{'response'}->{'code'},"Access-Reject","Check our return is 'Access-Reject' for unconfigured user");
is($res->{'response'}->{'code'},"Access-Accept","Check our return is 'Access-Accept' for bare user");
#
# Check we get a Access-Accept for an uncapped usage user
# Check we get a Access-Reject for a traffic topup user
#
my $user2_ID = testDBInsert("Create user 'testuser2'",
......@@ -211,9 +211,34 @@ if ($child = fork()) {
$user2_ID,'SMRadius-Capping-Traffic-Limit',':=','0'
);
my $user2attr3_ID = testDBInsert("Create user 'testuser2' attribute 'SMRadius-Capping-Uptime-Limit'",
$res = smradius::client->run(
"--raddb","dicts",
"127.0.0.1",
"auth",
"secret123",
'User-Name=testuser2',
'User-Password=test123',
);
is(ref($res),"HASH","smradclient should return a HASH");
is($res->{'response'}->{'code'},"Access-Reject","Check our return is 'Access-Reject' for a traffic topup user");
#
# Check we get a Access-Reject for a uptime topup user
#
my $user2b_ID = testDBInsert("Create user 'testuser2b'",
"INSERT INTO users (UserName,Disabled) VALUES ('testuser2b',0)"
);
my $user2battr1_ID = testDBInsert("Create user 'testuser2b' attribute 'User-Password'",
"INSERT INTO user_attributes (UserID,Name,Operator,Value,Disabled) VALUES (?,?,?,?,0)",
$user2_ID,'SMRadius-Capping-Uptime-Limit',':=','0'
$user2b_ID,'User-Password','==','test123'
);
my $user2battr3_ID = testDBInsert("Create user 'testuser2b' attribute 'SMRadius-Capping-Uptime-Limit'",
"INSERT INTO user_attributes (UserID,Name,Operator,Value,Disabled) VALUES (?,?,?,?,0)",
$user2b_ID,'SMRadius-Capping-Uptime-Limit',':=','0'
);
$res = smradius::client->run(
......@@ -221,11 +246,11 @@ if ($child = fork()) {
"127.0.0.1",
"auth",
"secret123",
'User-Name=testuser2',
'User-Name=testuser2b',
'User-Password=test123',
);
is(ref($res),"HASH","smradclient should return a HASH");
is($res->{'response'}->{'code'},"Access-Accept","Check our return is 'Access-Accept' for a basically configured user");
is($res->{'response'}->{'code'},"Access-Reject","Check our return is 'Access-Reject' for a uptime topup user");
#
......@@ -466,7 +491,7 @@ if ($child = fork()) {
#
# Check we get a Access-Accept for an autotopup user
# Check we get a Access-Accept for a traffic autotopup user
#
my $topuptest1_amount = 100;
......@@ -495,9 +520,9 @@ if ($child = fork()) {
$user3_ID,'SMRadius-AutoTopup-Traffic-Limit',':=','500'
);
my $user3attr5_ID = testDBInsert("Create user 'testuser3' attribute 'SMRadius-Capping-Uptime-Limit'",
my $user3attr5_ID = testDBInsert("Create user 'testuser3' attribute 'SMRadius-Capping-Traffic-Limit'",
"INSERT INTO user_attributes (UserID,Name,Operator,Value,Disabled) VALUES (?,?,?,?,0)",
$user3_ID,'SMRadius-Capping-Uptime-Limit',':=','0'
$user3_ID,'SMRadius-Capping-Traffic-Limit',':=','0'
);
$res = smradius::client->run(
......@@ -509,7 +534,7 @@ if ($child = fork()) {
'User-Password=test456',
);
is(ref($res),"HASH","smradclient should return a HASH");
is($res->{'response'}->{'code'},"Access-Accept","Check our return is 'Access-Accept' for a basically configured user");
is($res->{'response'}->{'code'},"Access-Accept","Check our return is 'Access-Accept' for a traffic autotopup user");
# Get the time now
my $topuptest1_now = time();
......@@ -538,6 +563,79 @@ if ($child = fork()) {
#
# Check we get a Access-Accept for a uptime autotopup user
#
my $topuptest1b_amount = 100;
my $user3b_ID = testDBInsert("Create user 'testuser3b'",
"INSERT INTO users (UserName,Disabled) VALUES ('testuser3b',0)"
);
my $user3battr1_ID = testDBInsert("Create user 'testuser3b' attribute 'User-Password'",
"INSERT INTO user_attributes (UserID,Name,Operator,Value,Disabled) VALUES (?,?,?,?,0)",
$user3b_ID,'User-Password','==','test456'
);
my $user3battr2_ID = testDBInsert("Create user 'testuser3b' attribute 'SMRadius-AutoTopup-Uptime-Enabled'",
"INSERT INTO user_attributes (UserID,Name,Operator,Value,Disabled) VALUES (?,?,?,?,0)",
$user3b_ID,'SMRadius-AutoTopup-Uptime-Enabled',':=','yes'
);
my $user3battr3_ID = testDBInsert("Create user 'testuser3b' attribute 'SMRadius-AutoTopup-Uptime-Amount'",
"INSERT INTO user_attributes (UserID,Name,Operator,Value,Disabled) VALUES (?,?,?,?,0)",
$user3b_ID,'SMRadius-AutoTopup-Uptime-Amount',':=',$topuptest1b_amount
);
my $user3battr4_ID = testDBInsert("Create user 'testuser3b' attribute 'SMRadius-AutoTopup-Uptime-Limit'",
"INSERT INTO user_attributes (UserID,Name,Operator,Value,Disabled) VALUES (?,?,?,?,0)",
$user3b_ID,'SMRadius-AutoTopup-Uptime-Limit',':=','500'
);
my $user3battr5_ID = testDBInsert("Create user 'testuser3b' attribute 'SMRadius-Capping-Uptime-Limit'",
"INSERT INTO user_attributes (UserID,Name,Operator,Value,Disabled) VALUES (?,?,?,?,0)",
$user3b_ID,'SMRadius-Capping-Uptime-Limit',':=','0'
);
$res = smradius::client->run(
"--raddb","dicts",
"127.0.0.1",
"auth",
"secret123",
'User-Name=testuser3b',
'User-Password=test456',
);
is(ref($res),"HASH","smradclient should return a HASH");
is($res->{'response'}->{'code'},"Access-Accept","Check our return is 'Access-Accept' for a uptime autotopup user");
# Get the time now
my $topuptest1b_now = time();
my $topuptest1b = DateTime->from_epoch( 'epoch' => $topuptest1b_now, 'time_zone' => 'UTC');
# Use truncate to set all values after 'month' to their default values
my $topuptest1b_thisMonth = $topuptest1b->clone()->truncate( to => "month" );
# This month, in string form
my $topuptest1b_thisMonth_str = $topuptest1b_thisMonth->strftime("%Y-%m-%d %H:%M:%S");
# Next month..
my $topuptest1b_nextMonth = $topuptest1b_thisMonth->clone()->add( months => 1 );
my $topuptest1b_nextMonth_str = $topuptest1b_nextMonth->strftime("%Y-%m-%d %H:%M:%S");
testDBResults("Check autotopup is added correctly",'topups',{'UserID' => $user3b_ID},
{
'UserID' => $user3b_ID,
'Timestamp' => sub { return _timestampCheck(shift,$topuptest1b_now) },
'Type' => 6,
'ValidFrom' => $topuptest1b_thisMonth_str,
'ValidTo' => $topuptest1b_nextMonth_str,
'Value' => $topuptest1b_amount,
'Depleted' => 0,
'SMAdminDepletedOn' => undef,
}
);
#
# Check that if we send an accounting ALIVE we update the auto-topups
#
......@@ -568,9 +666,9 @@ if ($child = fork()) {
$user4_ID,'SMRadius-AutoTopup-Traffic-Limit',':=','500'
);
my $user4attr5_ID = testDBInsert("Create user 'testuser4' attribute 'SMRadius-Capping-Uptime-Limit'",
my $user4attr5_ID = testDBInsert("Create user 'testuser4' attribute 'SMRadius-Capping-Traffic-Limit'",
"INSERT INTO user_attributes (UserID,Name,Operator,Value,Disabled) VALUES (?,?,?,?,0)",
$user4_ID,'SMRadius-Capping-Uptime-Limit',':=','0'
$user4_ID,'SMRadius-Capping-Traffic-Limit',':=','0'
);
my $user4attr6_ID = testDBInsert("Create user 'testuser4' attribute 'SMRadius-AutoTopup-Traffic-Notify'",
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment