Project

General

Profile

Actions

Regression #13823

closed

RADIUS attribute pfSense-Max-Total-Octets is not parsed correctly

Added by Marcos M almost 2 years ago. Updated almost 2 years ago.

Status:
Rejected
Priority:
High
Category:
Authentication
Target version:
-
Start date:
Due date:
% Done:

0%

Estimated time:
Plus Target Version:
Release Notes:
Default
Affected Version:
2.7.0
Affected Architecture:

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

Related to Regression #13418: Captive Portal does not keep track of client data usageResolvedReid Linnemann

Actions
Related to Feature #13843: Add ability to properly configure RADIUS captive portal user quotas of 4096MB or moreNewReid Linnemann

Actions
Actions #1

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.

Actions #2

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.

Actions #3

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.

Actions #4

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

Actions #5

Updated by Marcos M almost 2 years ago

  • Description updated (diff)
  • Assignee changed from Christian McDonald to Reid Linnemann
Actions #6

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
Actions #7

Updated by Marcos M almost 2 years ago

  • Related to Regression #13418: Captive Portal does not keep track of client data usage added
Actions #8

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.

Actions #9

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.
Actions #10

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.

Actions #11

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
Actions #12

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.

Actions #13

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
Actions

Also available in: Atom PDF