Project

General

Profile

Actions

Bug #3062

closed

Captive Portal NOT re-using PIPENO

Added by Alberto Palau over 11 years ago. Updated over 10 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
Captive Portal
Target version:
Start date:
07/01/2013
Due date:
% Done:

0%

Estimated time:
Plus Target Version:
Release Notes:
Affected Version:
2.1
Affected Architecture:

Description

Captive portal does not correctly release pipe numbers, is continually increasing them until they are exhausted, and then can not create new bandwidth limiters.

Only I have two clients connected and the numbers should start in 2000.

Limiters:
02122: unlimited 0 ms burst 0
q133194 100 sl. 0 flows (1 buckets) sched 67658 weight 0 lmax 0 pri 0 droptail
sched 67658 type FIFO flags 0x0 16 buckets 0 active
02123: unlimited 0 ms burst 0
q133195 100 sl. 0 flows (1 buckets) sched 67659 weight 0 lmax 0 pri 0 droptail
sched 67659 type FIFO flags 0x0 16 buckets 0 active
02077: unlimited 0 ms burst 0
q133149 100 sl. 0 flows (1 buckets) sched 67613 weight 0 lmax 0 pri 0 droptail
sched 67613 type FIFO flags 0x0 16 buckets 0 active
02076: unlimited 0 ms burst 0
q133148 100 sl. 0 flows (1 buckets) sched 67612 weight 0 lmax 0 pri 0 droptail
sched 67612 type FIFO flags 0x0 16 buckets 0 active

Included, unserialize_captiveportaldn.rules


Files

unserialize_captiveportaldn.rules (972 Bytes) unserialize_captiveportaldn.rules Alberto Palau, 07/01/2013 03:42 PM
captiveportal.inc (70 KB) captiveportal.inc File FIXED Alberto Palau, 07/02/2013 03:38 PM
Actions #1

Updated by Chris Buechler over 11 years ago

  • Assignee deleted (Chris Buechler)
  • Priority changed from High to Normal
Actions #2

Updated by Alberto Palau over 11 years ago

Version 2.0.3 is also affected by this problem.

Actions #3

Updated by Alberto Palau over 11 years ago

Ok, I'm working on a solution, and found the problem in the code, I put the fix and I'm probing now, it appears that the RADIUS function does not FREE the pineno when you try to authenticate the client and fails, I added some code to fix and so far it works well, in a while I release the fix for that if you want to include it in the next snapshoot.

Sorry my english is bad.

Actions #4

Updated by Alberto Palau over 11 years ago

Limiters:
02002: unlimited 0 ms burst 0
q133074 100 sl. 0 flows (1 buckets) sched 67538 weight 0 lmax 0 pri 0 droptail
sched 67538 type FIFO flags 0x0 16 buckets 0 active
02003: unlimited 0 ms burst 0
q133075 100 sl. 0 flows (1 buckets) sched 67539 weight 0 lmax 0 pri 0 droptail
sched 67539 type FIFO flags 0x0 16 buckets 0 active
02000: unlimited 0 ms burst 0
q133072 100 sl. 0 flows (1 buckets) sched 67536 weight 0 lmax 0 pri 0 droptail
sched 67536 type FIFO flags 0x0 16 buckets 0 active
02001: unlimited 0 ms burst 0
q133073 100 sl. 0 flows (1 buckets) sched 67537 weight 0 lmax 0 pri 0 droptail
sched 67537 type FIFO flags 0x0 16 buckets 0 active

After trying for a while captive portal behavior, it is working fine, These are the modifications captiveportal.inc ...

ORIGINAL FUNCTION

_function radius($username,$password,$clientip,$clientmac,$type, $radiusctx = null) {
global $g, $config;

$pipeno = captiveportal_get_next_dn_ruleno();
/* If the pool is empty, return appropriate message and fail authentication */
if (is_null($pipeno)) {
$auth_list = array();
$auth_list['auth_val'] = 1;
$auth_list['error'] = "System reached maximum login capacity";
return $auth_list;
}
$radiusservers = captiveportal_get_radius_servers();
if (is_null($radiusctx))
$radiusctx = 'first';
$auth_list = RADIUS_AUTHENTICATION($username,
$password,
$radiusservers[$radiusctx],
$clientip,
$clientmac,
$pipeno);
if ($auth_list['auth_val'] == 2) {
captiveportal_logportalauth($username,$clientmac,$clientip,$type);
$sessionid = portal_allow($clientip,
$clientmac,
$username,
$password,
$auth_list,
$pipeno,
$radiusctx);
}
return $auth_list;
}
_

MODIFIED FUNCTION

*function radius($username,$password,$clientip,$clientmac,$type, $radiusctx = null) {
global $g, $config;

$radiusservers = captiveportal_get_radius_servers();
$pipeno = captiveportal_get_next_dn_ruleno();

/* If the pool is empty, return appropriate message and fail authentication */
if (is_null($pipeno)) {
$auth_list = array();
$auth_list['auth_val'] = 1;
$auth_list['error'] = "System reached maximum login capacity";
return $auth_list;
}

if (is_null($radiusctx))
$radiusctx = 'first';
$auth_list = RADIUS_AUTHENTICATION($username,
$password,
$radiusservers[$radiusctx],
$clientip,
$clientmac,
$pipeno);

if ($auth_list['auth_val'] == 2) {
captiveportal_logportalauth($username,$clientmac,$clientip,$type);
$sessionid = portal_allow($clientip,
$clientmac,
$username,
$password,
$auth_list,
$pipeno,
$radiusctx);}
else {
captiveportal_free_dn_ruleno($pipeno);
}

return $auth_list;
}

*

In this function just changed a line position, to make the pinene began in 2000. I hope you learn the contribution.

function captiveportal_get_next_dn_ruleno($rulenos_start = 2000, $rulenos_range_max = 64500) {
global $config, $g;

$cpruleslck = lock("captiveportalrulesdn", LOCK_EX);
$ruleno = 0;
if (file_exists("{$g['vardb_path']}/captiveportaldn.rules")) {
$rules = unserialize(file_get_contents("{$g['vardb_path']}/captiveportaldn.rules"));
for ($ridx = $rulenos_start; $ridx < $rulenos_range_max; $ridx++) {
if ($rules[$ridx]) {
$ridx++;
continue;
}
$ruleno = $ridx;
$rules[$ridx] = "used";
$rules[++$ridx] = "used";
break;
}
} else {
$rules = array_pad(array(), $rulenos_range_max, false);
$ruleno = $rulenos_start;
$rules[$rulenos_start] = "used";
$rules[++$rulenos_start] = "used";
}
file_put_contents("{$g['vardb_path']}/captiveportaldn.rules", serialize($rules));
unlock($cpruleslck);
return $ruleno;
}

There is still a problem, I mean delete the file captiveportaldn.rules when the captive portal service is stoped or is restarted the server, to avoid leaving reserved the pipes that were in use. If you could do that job for me I would appreciate it. Thank's

Actions #5

Updated by Alberto Palau over 11 years ago

Excuse the mess in the text above, I did not know how to modify it, please if anyone can fix it, thanks

Actions #6

Updated by Alberto Palau over 11 years ago

Only correct a sentence, I meant that I hope will serve out the modification, instead of "I hope you learn the contribution."

Actions #7

Updated by Phillip Davis over 11 years ago

Alberto, it will be much easier if you put the changes in GitHub. Then the developers can easily see the differences, review and merge into the code.
Go to https://github.com/pfsense/pfsense
Signup for an account.
Press the "Fork" button.
Now you have a personal "copy" of the code.
Make your edits online.
Use the "Pull Request" button to submit to the main repository for review.

Actions #8

Updated by Ermal Luçi over 11 years ago

  • Status changed from New to Feedback

Merging of the patch has been done.
Thank you.

Actions #9

Updated by Alberto Palau over 11 years ago

I have put a fix for this part in github "There is still a problem, I mean delete the file captiveportaldn.rules When the Captive Portal service is stoped or is restarted the server, to avoid leaving reserved the pipes That Were in use. If You Could do that job for me I would appreciate it. Thank's " please if you could assess i would appreciate it.

Thank you

Actions #10

Updated by Ermal Luçi over 11 years ago

  • Status changed from Feedback to Resolved
Actions #11

Updated by Dsi Unicaen over 10 years ago

When a user is already authenticated and re-send an authentication, a new pipeno is created (function radius). The allow_portal function do not release this pipeno if user is already authenticate.

I have put a fix in github https://github.com/pfsense/pfsense/pull/1070

I can send the batch script to reproduce this bug but it is a potentially DoS on captive portal.

Actions #12

Updated by Ermal Luçi over 10 years ago

The patch was merged Dsi.

Actions #13

Updated by Doktor Notor over 10 years ago

Ermal Luçi wrote:

The patch was merged Dsi.

Cannot see this merged anywhere in 2.1.3; https://forum.pfsense.org/index.php?topic=70314.15

Actions

Also available in: Atom PDF