Should i do it ?



  • First of all awesome job guys!!

    Using Using Captive Portal + FreeRadius + ( mySQL on a different machine) Custom Login Page , Custom Logout Page
    (NO Window Pop-up)

    2.4.2-RELEASE-p1 (amd64)
    FreeRadius 0.15.4

    Problem:

    If i make ANY change to the Captive portal configuration page and click save
    all users are disconnected from the Captive portal user cache, but not from the Radius Database resulting to a mismatch.
    (at least that's what i think happens..)

    What i mean is that already logged in users access the internet.
    I make a change in Captive Portal Config .. let's say change login page and Click Save.
    All users from the user local database are disconnected, but not from the Radius Database.

    Logged in  Users Try to access the Internet, Captive Portal seas that users haven't logged in, asks from Radius if user has logged in , Radius returns YES and upon return Captive portal replies that User is already logged in without adding them to the Captive Portal user database or ipfw.

    Now on etc/inc/captiveportal.inc

    at line 185 seas

    if (platform_booting()) {
    echo "Starting captive portal({$cpcfg['zone']})… ";
    /* remove old information */
    unlink_if_exists("{$g['vardb_path']}/captiveportal{$cpzone}.db");
    } else {
    captiveportal_syslog("Reconfiguring captive portal({$cpcfg['zone']}).");
    }

    /* init ipfw rules */
    captiveportal_init_rules(true);

    Should i change the captiveportal_init_rules(true);

    to

    if (platform_booting()) {
    /* init ipfw rules */
    captiveportal_init_rules(true);
    } else {
      captiveportal_init_rules(false);
    }

    ? So users database does not get cleared ?



  • Hi,

    When the captive portal is reconfigured, https://github.com/pfsense/pfsense/blob/7c1aa62bc3890faa8a617a6ea734c703a088c602/src/etc/inc/captiveportal.inc#L178 is called - ok.

    But this one also a bit further allong :
    https://github.com/pfsense/pfsense/blob/7c1aa62bc3890faa8a617a6ea734c703a088c602/src/etc/inc/captiveportal.inc#L350
    which stops all freeradius connections. (captiveportal_radius_stop_all(10); // NAS-Request )

    Yours doesn't ? Focus on that.



  • Yeah thanks i saw it after i made the post…

    did not make any changes

    still the problem persists.

    Any ideas ?



  • Checkout this captiveportal_radius_stop_all()  function.
    There is a log line here :
    See https://github.com/pfsense/pfsense/blob/7c1aa62bc3890faa8a617a6ea734c703a088c602/src/etc/inc/captiveportal.inc#L1173

    So, when reconfiguring, do you see a line in the log for each logged in user  ? You should se them.

    And also : when logging out (you throwing out) some one from the dashboard, did you see a log line ?

    Note : I'm not using the radius solution myself.



  • Yes i can see the logs coming in, when i disconnect a single or all users. User / Users are logged out  everything works fine.

    maybe i should be using

    https://github.com/pfsense/pfsense/blob/7c1aa62bc3890faa8a617a6ea734c703a088c602/src/etc/inc/captiveportal.inc#L1128

    instead of

    captiveportal_radius_stop_all(10); // NAS-Request  ?

    i see that there is a different "logout reason" in the parameters…



  • Worth trying to test.

    The difference is
    "10" or "6"
    as a reason for FreeRadius.



  • Update:  i removed / Uninstalled FreeRadius 3  / Disabled Captive Portal /

    Reconfigured all Captive Portal Settings with  Local user management this time.

    Enabled CP.

    Same issue: Once i make any Change in CP Config page all users are dropped from the database and all logged in users are stuck. CP thinks that they are logged in but they are not able to browse



  • Is there Any Change that anybody can confirm this behavior ?



  • @jaspras:

    Is there Any Change that anybody can confirm this behavior ?

    Oh, yes !
    I confirm ….  :o

    Thus, ipfw tables myzone_auth_down and myzone_auth_up which contain the logged in users get emptied (reset).
    But the database (an SQLLite database) isn't emptied.
    Now, things go bad.

    When I select another Wifi network, and then come back to my Captive portal, I do get the captive portal login page, can enter my user/password, but after validating, the redirect fails.
    The user/password was already logged - present in the database - ipfw tables aren't updated with my IP/MAC.
    I'm stuck.

    I have to delete the user from the pfSense GUI (delete it from the SQLLite  database) , or wait for the soft time out to happen.

    Using another user/password works (of course).

    I didn't notice this before, I actually never change changes my portal settings so didn't encounter this bug.

    I remember very well that saving settings didn't empty the logged in users, and didn't reset the ipfw firewall (logged in users are recently stored in the 2 tables).
    I guess that these 2 tables "myzone_auth_down and myzone_auth_up" shouldn't be reset after a "Save" on the captive portal page Config page (good).
    Or, if it is ok, empty also the SQLLite database (less good) - which means that everybody gets thrown out (no good at all).

    edit : I went for the (good) solution : NOT recreating the ipfw rules when config gets saved.
    @jaspras:

    .....
       captiveportal_init_rules(false);
    

    }

    ? So users database does not get cleared ?

    The database didn't get cleared, when the ipfw firewall rules (tables actually) got destroyed - that sucks.

    https://github.com/pfsense/pfsense/blob/7c1aa62bc3890faa8a617a6ea734c703a088c602/src/etc/inc/captiveportal.inc#L195
    I'm testing now with

    captiveportal_init_rules(false); 
    

    on that line.
    "testing" because ipfw rules no only get created when the system starts.
    AND when captive portal get disabled and enabled again.
    This seems ok to me.

    edit 2 : and it's wrong : when Captive portal is disabled at boot, and you enable it manually, rules are NOT created => NO GOOD.
    So forget about the "edit" part.
    I'll be testing something like :
    When Save is pressed, and thus captiveportal_configure_zone() is called, we need to check if the portal was activated at that moment, which implies  captiveportal_configure_zone(true) is needed, or it was already activated (== a simple config save) and this could work with a captiveportal_configure_zone(false).
    I can't test that right know, because it needs also some GUI editing probably, not ony /etc/inc/captiveportal.inc.

    For now : when you save the Captive Portal config, chain on to the Status Page of the captive portal and kill all connection (or drop in a call to the function that does this after captiveportal_configure_zone(true) gets called.
    Yes, your Captive portal users won't be happy ^^



  • Yes I also remember that saving CP config page did not delete Ipfw rules nor logged in users.  At least for version 2.1.3 I think.

    The best solution for me would be to delete ipfw rules and recreate them on the fly keeping the user database intact, since it seems that the rules have to be pruned. That can occur at “save” time



  • @jaspras:

    Yes I also remember that saving CP config page did not delete Ipfw rules nor logged in users.  At least for version 2.1.3 I think.

    Same for me.

    @jaspras:

    The best solution for me would be to delete ipfw rules and recreate them on the fly keeping the user database intact, since it seems that the rules have to be pruned. That can occur at “save” time

    Actually, the ipfw rules and tables are rewritten from the ground up, with a nasty side effect : tables myzone_auth_down and myzone_auth_up are empty now.
    When using the Local user Manager, a small SQLi database maintains the logged in user that is NOT emptied when saving.
    Current user can not re-authenticate because pfSense 'thinks' that they are already logged in, so ipfw tables myzone_auth_down and myzone_auth_up are not loaded with their MAC and user IP.

    A solution might be : reading the SQLi database to re-fill tables myzone_auth_down and myzone_auth_up
    Or : saving the state of myzone_auth_down and myzone_auth_up when rewriting the rules, and after that, put them back in.

    I'll have to figure out how all this works when a FreeRadus is used for authenticating (is that local database still used, or not).



  • I'll have to figure out how all this works when a FreeRadus is used for authenticating (is that local database still used, or not).

    Local Database is used only when "re authentication every minute" is checked from FreeRadius Config page



  • [Gertjan]

    https://github.com/pfsense/pfsense/blob/58a2ba621c390362170aa2e377e4b41c8fdce1c6/src/etc/inc/captiveportal.inc#L629

    ! look at that !

    this is called at
    https://github.com/pfsense/pfsense/blob/58a2ba621c390362170aa2e377e4b41c8fdce1c6/src/etc/inc/captiveportal.inc#L195

    that means that firewall rules for authenticated users are being created ? or is it just my idea ?

    i see something odd though ..

    at Github master there's this

    /* Authenticated users rules. */
    $cprules .= "table {$cpzone}_auth_up create type addr valtype pipe\n";
    $cprules .= "table {$cpzone}_auth_down create type addr valtype pipe\n";
    $cprules .= captiveportal_create_ipfw_rule("add", $rulenum,
        "pipe tablearg ip from table({$cpzone}_auth_up) to any layer2 in");
    $cprules .= captiveportal_create_ipfw_rule("add", $rulenum,
        "pipe tablearg ip from any to table({$cpzone}_auth_down) layer2 out");

    committed on Jul 26, 2017 by rbgarga changing it to layer 2 only ?

    previous version

    /* Authenticated users rules. */
    $cprules .= "table {$cpzone}_auth_up create type addr valtype pipe\n";
    $cprules .= "table {$cpzone}_auth_down create type addr valtype pipe\n";
    $cprules .= captiveportal_create_ipfw_rule("add", $rulenum,
        "pipe tablearg ip from table({$cpzone}_auth_up) to any in");
    $cprules .= captiveportal_create_ipfw_rule("add", $rulenum,
        "pipe tablearg ip from any to table({$cpzone}_auth_down) out");

    ??? could this be the blame ?



  • I don't think so.

    This is what I know :
    The ipfw rules for a captive portal instance (more then one can exist) are pretty static.
    The only dynamic rules are table {$cpzone}_auth_up and table {$cpzone}_auth_down
    Every instance has this pair of table {n$cpzone}_auth_up and table {$cpzone}_auth_down set.
    These two table contain the authorized, logged in users.

    When "Saving" the config page of an instance, the entire ipfw rule set for that instance is flushed and recreated.
    table {$cpzone}_auth_up and table {$cpzone}_auth_down will be empty.

    Check out where captiveportal_init_rules() is called :
    In function captiveportal_disconnect_all() - and there, the local database is also emptied (with the function captiveportal_remove_entries())
    So this is ok - ipfw rules and database are cleaned.

    And here : captiveportal_configure_zone() (gets called when "Saving" the config page - our issue) and there the database is not getting emptied, and that's the problem.

    So, if it is ok** that everybody gets kicked out correctly on Saving the config page, the this (see below) code should be put here : https://github.com/pfsense/pfsense/blob/58a2ba621c390362170aa2e377e4b41c8fdce1c6/src/etc/inc/captiveportal.inc#L193
    just before the captiveportal_init_rules() in function captiveportal_configure_zone().

    ** this is probably not what we want. We want the old situation back where current logged in users aren't kicked from the portal when a Saving of the config happens.
    This means that the tables {$cpzone}_auth_up and table {$cpzone}_auth_down should stay in place so that all related firewall states do not get destroyed = current connections.

    Example : if we destroy the tables and recreate them then ipfw states are destroyed (can they be saved and recreated ?) so I guess an impact will happen when saving the config, no matter what.

    I think that recreating isn't really needed when saving the config if the portal is/was already running.
    Creating is only needed when starting up a portal instance. Destroying when shutting down that portal instance.
    A change in the config of a given instance should only change related ipfw rules (if any - as said, most rules are actually static and last for the duration of the portal's instance live).
    For all this to happen, much rewriting is needed.

    For now, this will do the job :

    	/* remove users from the database */
    	$cpdb = captiveportal_read_db();
    	$unsetindexes = array_column($cpdb,5);
    	if (!empty($unsetindexes)) {
    		captiveportal_remove_entries($unsetindexes);
    	}
    

    I took this code from here https://github.com/pfsense/pfsense/blob/58a2ba621c390362170aa2e377e4b41c8fdce1c6/src/etc/inc/captiveportal.inc#L1130

    Btw : I can't test right now, I'm not on site - and I hate to patch code when I'm not there, because I can't test if it "works".

    edit :
    Take a look at the captive portal file /etc/inc/captiveportal.inc on 7 December 2016 !
    https://github.com/pfsense/pfsense/blob/e85f3a2b93e318f37569be7f57355fe9fed6ab43/src/etc/inc/captiveportal.inc#L178

    Back then, when booting the database was deleted (unlinked) - see line 189.
    And then, no matter what, line 195, captiveportal_init_rules(true); was called, which deletes and recreates all ipfw rules (and thus our tables with logged in users).
    When reconfiguring the portal instance, this function is called also - but the database (of that portal instance) isn't deleted.
    So, the very same issue was already there on 7/12/2016 ?!

    This makes me think I don't understand the whole problem yet - and tend to believe that this issue is already there all this time.



  • I tested this :
    replace :

    		if (platform_booting()) {
    			echo "Starting captive portal({$cpcfg['zone']})... ";
    
    			/* remove old information */
    			unlink_if_exists("{$g['vardb_path']}/captiveportal{$cpzone}.db");
    		} else {
    			captiveportal_syslog("Reconfiguring captive portal({$cpcfg['zone']}).");
    		}
    
    		/* init ipfw rules */
    
    

    for this :```

    	if (platform_booting()) {
    		echo "Starting captive portal({$cpcfg['zone']})... ";
    
    		/* remove old information */
    		unlink_if_exists("{$g['vardb_path']}/captiveportal{$cpzone}.db");
    	} else {
    		captiveportal_syslog("Reconfiguring captive portal({$cpcfg['zone']}).");
    
    		/* remove users from the database */
    		$unsetindexes = array();
    		$cpdb = captiveportal_read_db();
    		$unsetindexes = array_column($cpdb,5);
    		if (!empty($unsetindexes)) {
    			captiveportal_remove_entries($unsetindexes);
    		}
    		captiveportal_syslog("Reconfiguring : database emptied ({$cpcfg['zone']}).");
    	}
    
    	/* init ipfw rules */
    
    
    What happens is that when **booting**, de database file is just deleted - that's ok, the system is booting, so no ipfw rules, neither logged in user are present anyway.
    If not, when enabling or reconfiguring, the database file is emptied.
    
    True, a simple
    

    unlink_if_exists("{$g['vardb_path']}/captiveportal{$cpzone}.db");

    does the job also.
    
    All this :
    
    	if (platform_booting()) {
    		echo "Starting captive portal({$cpcfg['zone']})... ";
    
    		/* remove old information */
    		unlink_if_exists("{$g['vardb_path']}/captiveportal{$cpzone}.db");
    	} else {
    		captiveportal_syslog("Reconfiguring captive portal({$cpcfg['zone']}).");
    	}
    
    	/* init ipfw rules */
    	......
    
    
    could be replace by a mere (no more if statement) :
    
    echo "Starting captive portal({$cpcfg['zone']})... ";
    
    /* remove old information */
    unlink_if_exists("{$g['vardb_path']}/captiveportal{$cpzone}.db");
    
    /* init ipfw rules */
    .....
    
    
    
    	captiveportal_init_rules(true);
    
    that follows just after these lines finishes the job.
    
    Remember : every time you (re) configure the Captive Portal instance, all user on that instance** get thrown out.
    This is definitely not a good thing when you are running a very busy captive portal.
    
    ** Looking through the code, only the reconfigured instance is concerned.
    Other instances are not touched,  I have tested this with a second captive portal instance.