Project

General

Profile

Bug #3062

Captive Portal NOT re-using PIPENO

Added by Alberto Palau almost 6 years ago. Updated about 5 years ago.

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

0%

Estimated time:
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

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

Associated revisions

Revision 7fb23399 (diff)
Added by Ermal Luçi almost 6 years ago

Implement proper releasing of pipes allocated based on CPzone. Keep track of which zone a pipe is and release those pipes during disabling/deleting of zone. Ticket #3062, Pull request #698

Revision bc59bcff (diff)
Added by Ermal Luçi almost 6 years ago

Implement proper releasing of pipes allocated based on CPzone. Keep track of which zone a pipe is and release those pipes during disabling/deleting of zone. Ticket #3062, Pull request #698

Revision 39f3d843 (diff)
Added by Dsi Unicaen about 5 years ago

Update captiveportal.inc

Release unused pipeno when client is already authenticated.

Bug #3062

Revision a7ee038b (diff)
Added by Ermal Luçi about 5 years ago

Put the fix to be more generic to prevent any other leak possible in the long run. Fixes #3062

Revision 4ec6b54d (diff)
Added by Ermal Luçi about 5 years ago

Merge the forgotten Ticket #3062 patch for CP pipeno leaking issue which leads to the 'Maximum login reached' on CP

History

#1 Updated by Chris Buechler almost 6 years ago

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

#2 Updated by Alberto Palau almost 6 years ago

Version 2.0.3 is also affected by this problem.

#3 Updated by Alberto Palau almost 6 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.

#4 Updated by Alberto Palau almost 6 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

#5 Updated by Alberto Palau almost 6 years ago

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

#6 Updated by Alberto Palau almost 6 years ago

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

#7 Updated by Phillip Davis almost 6 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.

#8 Updated by Ermal Luçi almost 6 years ago

  • Status changed from New to Feedback

Merging of the patch has been done.
Thank you.

#9 Updated by Alberto Palau almost 6 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

#10 Updated by Ermal Luçi almost 6 years ago

  • Status changed from Feedback to Resolved

#11 Updated by Dsi Unicaen about 5 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.

#12 Updated by Ermal Luçi about 5 years ago

The patch was merged Dsi.

#13 Updated by Doktor Notor about 5 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

Also available in: Atom PDF