Regression #13823
closedRADIUS attribute pfSense-Max-Total-Octets is not parsed correctly
0%
Description
The RADIUS attribute pfSense-Max-Total-Octets
is used in FreeRADIUS with the option Amount of Download and Upload Traffic
. Currently, it is not parsed correctly, leading to sessions being logged out prematurely. It seems to be an issue somewhere in the Auth_RADIUS wrapper given that calling getAttributes()
returns the incorrect maxbytes
attribute.
For example, entering 1000
in the FreeRADIUS user's configuration for the options Amount of Download and Upload Traffic
results in the following:
[maxbytes] => 1048576000
but a value of 10000
returns:
[maxbytes] => 1895825408
Related issues
Updated by Christian McDonald almost 2 years ago
/usr/local/etc/raddb/users "bob" Cleartext-Password := "password" WISPr-Bandwidth-Max-Up := 100000, WISPr-Bandwidth-Max-Down := 100000, pfSense-Max-Total-Octets := 104857600, Exec-Program-Wait = "/bin/sh /usr/local/etc/raddb/scripts/datacounter_auth.sh bob daily"
And from the captive portal sqlite db:
sqlite> PRAGMA table_info(captiveportal); 0|allow_time|INTEGER|0||0 1|pipeno|INTEGER|0||0 2|ip|TEXT|0||0 3|mac|TEXT|0||0 4|username|TEXT|0||0 5|sessionid|TEXT|0||0 6|bpassword|TEXT|0||0 7|session_timeout|INTEGER|0||0 8|idle_timeout|INTEGER|0||0 9|session_terminate_time|INTEGER|0||0 10|interim_interval|INTEGER|0||0 11|traffic_quota|INTEGER|0||0 12|bw_up|INTEGER|0||0 13|bw_down|INTEGER|0||0 14|authmethod|TEXT|0||0 15|context|TEXT|0||0 sqlite> select * FROM captiveportal; 1672755621|2000|172.21.60.135|88:d8:2e:42:f0:9f|bob|9f38eaac218e0cea||||||104857600|0|0|radius|first sqlite>
I'm not obviously seeing the error here. 100MB = 104857600 Bytes
It would appear the sessions being logged out prematurely would be caused by a bug after reading the value back from the db, as it appears that the correct value is inserted into the db.
Updated by Jim Pingle almost 2 years ago
- Status changed from New to Not a Bug
I agree, it looks right.
In FreeRADIUS the label even mentions MB:
Enter the amount of download and upload traffic (summarized) in megabytes (MB) for this user.
Though these days maybe we should change that to say MiB/mebibyte since it's 1024*1024? That's just a cosmetic label though, not a bug.
Updated by Marcos M almost 2 years ago
- Status changed from Not a Bug to Confirmed
The values used to generate the files by Captive Portal are correct - such as what gets placed in the db and quota tracking files. However, the check that determines if the quota has been exceeded is done against the incorrect value. When this part of the code is reached:
https://github.com/pfsense/pfsense/blob/c1bc55a9f37e5977110a3bb1f170321738fdf3d2/src/etc/inc/captiveportal.inc#L674
The value of $cpentry
is as follows:
( [0] => 1672707288 [allow_time] => 1672707288 [1] => 2000 [pipeno] => 2000 [2] => 10.0.1.104 [ip] => 10.0.1.104 [3] => 00:50:56:b2:3a:37 [mac] => 00:50:56:b2:3a:37 [4] => testuser [username] => testuser [5] => 1909bb9c6370b2ec [sessionid] => 1909bb9c6370b2ec [6] => dGVzdA== [bpassword] => dGVzdA== [7] => [session_timeout] => [8] => [idle_timeout] => [9] => [session_terminate_time] => [10] => 61 [interim_interval] => 61 [11] => 1778384896 [traffic_quota] => 1778384896 [12] => 0 [bw_up] => 0 [13] => 0 [bw_down] => 0 [14] => radius [authmethod] => radius [15] => first [context] => first )
Here, traffic_quota
should be 10GB, not 1.7GB. The issue description may be incorrect and can be updated if so (edit: seems to be an overflow issue, I've updated the description). There is certainly an issue however, see https://redmine.pfsense.org/issues/13418.
Updated by Christian McDonald almost 2 years ago
Maybe we should pass this one to Reid as he handled https://redmine.pfsense.org/issues/13418
Updated by Marcos M almost 2 years ago
- Description updated (diff)
- Assignee changed from Christian McDonald to Reid Linnemann
Updated by Jim Pingle almost 2 years ago
- Subject changed from RADIUS attribute pfSense-Max-Total-Octets is not parsed correctly. to RADIUS attribute pfSense-Max-Total-Octets is not parsed correctly
Updated by Marcos M almost 2 years ago
- Related to Regression #13418: Captive Portal does not keep track of client data usage added
Updated by Reid Linnemann almost 2 years ago
This smells to me like your user authenticated and then you modified the user's traffic quota. The quota in the database is only set when the user logs in and is not updated asynchronously. Can you confirm if this is what happened?
I've tested with a 10GB quota:
[2.7.0-DEVELOPMENT][root@pfSense.home.arpa]/var/db: sqlite3 captiveportaltest.db SQLite version 3.39.3 2022-09-05 11:02:23 Enter ".help" for usage hints. sqlite> select * from captiveportal; 1672963506|2000|192.168.60.101|58:9c:fc:09:8f:ea|test|ea20051786b85fae|dGVzdA==|||||10737418240|0|0|radius|first
php > var_dump(captiveportal_read_db()); array(1) { [0]=> array(32) { ... [11]=> int(10737418240) ["traffic_quota"]=> int(10737418240) ... } }
The value in the sqlite db and the value unmarshalled by the php sqlite3 library are the same.
Updated by Reid Linnemann almost 2 years ago
Ok, I misunderstood the actual problem here, which is that the database record is having the wrong value inserted. It's due to 32-bit integer limitations in the PECL radius library, I believe:
>>> hex(10485760000); '0x271000000' >>> 0x71000000 1895825408
10000 MB (stored as a number of bytes) is a value that overflows 32 bit unsigned int. I'm not certain yet if this is a limitation of radius_cvt_int() in the library or if more than four bytes are actually being sent and received in the first place.
Updated by Reid Linnemann almost 2 years ago
According to the RFCs, integers types are all 32 bits, period. To support larger limits we'll need to have an alternative attribute type for lower granularity (pfSense-Max-Used-MB e.g.), use a string attribute type, or add an additional max octets high word for 64 bit values and do the appropriate work for splitting and recombining 64 bit values. Regardless, I don't think we want to alter the existing vendor unique type we've defined as pfSense-Max-Used-Octets, as that is a highly disruptive change and breaks backward compatibility.
Updated by Reid Linnemann almost 2 years ago
- Related to Bug #13842: RADIUS user accounting limit inputs for bandwidth and total usage are not validated to prevent exceeding a 32 bit unsigned value added
Updated by Reid Linnemann almost 2 years ago
- Status changed from Confirmed to Rejected
I'm rejecting this as parsing is not the actual issue, and I'm linking to a new bug and enhancement request.
Updated by Reid Linnemann almost 2 years ago
- Related to Feature #13843: Add ability to properly configure RADIUS captive portal user quotas of 4096MB or more added