sqlippool, allocate_find and duplicate address if NAS packets are received in the reverse order

[Alan DeKok]

On Sep 17, 2022, at 7:06 AM, Matteo Sgalaberni <sgala at sgala.com> wrote:
> Hi, I found on this patch a huge change on the sqlippool: 
> 
> https://github.com/FreeRADIUS/freeradius-server/commit/38e475f424d4abb1b3ec52c533a21fd413495d99 
> 
> I see that it's well documented on wiki. I have a doubt about the "allocate_existing". 
> 
> I'm using the oldest query "allocate_find" that do in the same query "allocate_existing+allocate_find" and works well. I can probably understand why the developer who wrote this patch divided this, so it's simpler to disable the sticky ip. 
> 
> I'm running in a problem with "sticky ip" (old query allocate_existing): 
> allocate_find = " \ SELECT framedipaddress \ FROM ${ ippool_table } \ WHERE pool_name = '%{control:${pool_name}}' \ AND ( \ expiry_time < 'now' :: timestamp ( 0 ) \ OR ( nasipaddress = '%{NAS-IP-Address}' AND pool_key = '${pool_key}' ) \ ) \ ORDER BY \ ( username <> '%{SQL-User-Name}' ), \ ( callingstationid <> '%{Calling-Station-Id}' ), \ expiry_time \ LIMIT 1 \ FOR UPDATE "
>
> Sometimes, in rare case when the customer have a very quick disconnection/reconnection happen that the NAS send the Access-Request before the stop Accounting-Request. So in that case happen: 
> 1) Access_Request -> allocate_find (find the same ip to the same user) + allocate_update (assign same ip, renew lease) 
> 2) Accounting-Request Stop -> allocate_free (clear ip) 

  That's wrong.  I'd argue that the NAS is broken.  But... it's RADIUS. :(

  One question is what's in the packets?  i.e. Is there any way to tell that the "stop" is for an old session, and not the new one?

  If the accounting packet contains an Acct-Start-Time that's *before* the time the Access-Request was received, then there's a simple solution.  Update the IP pool schema to include a "last allocated" time.  Set it when the IP is allocated (or renewed).  And then check it during the "stop query.  If the "last allocated" time is after the "Acct-Start-Time" of the "stop", then you can ignore the stop, and leave the IP address alone.

> Now, the IP is assigned, but on sqlippool is marked as free...and sometime in the future another user will obtain the same IP and I have a duplicate assignment. 
> 
> Is this behaviour normal and "known"? I can't find this "warn" on wiki/documentation. 

  It's RADIUS, anything can happen.  NAS vendors will do the most "inventive" things.  That's one reason why FreeRADIUS is so complicated: to deal with all of the corner cases like this.

> If this is normal and known I think that I need to kill the "stitky ip" capability to solve the issue in these way: 
> Way A)  remove " nasipaddress = '%{NAS-IP-Address}' AND pool_key = '${pool_key}'" and ORDER BY \ (username <> '%{SQL-User-Name}') from allocate_find 
> or 
> Way B) using the new queries.conf of the last branch  and commenting the "allocate_existing" 
> 
> The IP in that case is not assigned again to the same user and I'm sure that the NAS packets order are not creating issues. 

  Whatever solution works for you is fine.  That's why the queries are editable, and documented.  You can change them to work around issues in your network.

  Alan DeKok.