range plugin never calls free

[purpleidea]

I was reviewing the code in the dhcp range plugin which is found here:

https://github.com/coredhcp/coredhcp/tree/master/plugins/range

I noticed that it never calls the Free() method on the allocator.

In effect, this is a serious "lease leak" since any expired lease will never get deallocated. This means you will eventually get lease exhaustion which is a kind of denial of service issue.

Cheers!

[purpleidea]

FWIW I noticed this when building the dhcp:range resource for https://github.com/purpleidea/mgmt/ and I will definitely have to build something in to work around this, but I figured you'd like to know.

[insomniacslk]

at a quick look I think the fix is to check for dhcpv4.MessageTypeRelease in range.Handle4 (if an entry is found in the lease db) and then call Free and remove (or expire) the entry from the leases DB. CC @Natolumin any thoughts? I can proceed with the above fix if you agree

[Natolumin]

I think this is the same issue as #148 but that was on the previous implementation of the plugin.
There's both handling release messages and also that we don't actually have a gc process for expired leases, but they're 2 separate problems that probably have 2 separate solutions.

Your solution sounds good for the release messages in any case.
For GCing expired leases It'd be nice to do something reusable by other plugins, but in the meantime probably just a goroutine that frees and deletes long-expired leases on a timer would be better than nothing?

[purpleidea]

at a quick look I think the fix is to check for dhcpv4.MessageTypeRelease in range.Handle4 (if an entry is found in the lease db) and then call Free and remove (or expire) the entry from the leases DB. CC @Natolumin any thoughts? I can proceed with the above fix if you agree

I think this is the incorrect fix.

The correct fix is to have a clean script which checks the records and removes anything that's old and runs Free. When should that get called?

(1) Periodically when it's time for the next cleaning to occur. (This can be calculated)
(2) If a host requests a release than we can have that happen early. (Assuming I understand how DHCP protocol works.)

I have a simplistic implementation in https://github.com/purpleidea/mgmt/ for this now.

HTH