sqlippool Pool-Name not defined

[maximumG]

Issue type

• sqlippool module requires that a Pool-Name is defined
• Feature request.

Feature description

I have a use case for the sqlippool module where we defined our own Pool-Name attribute (specific dictinary) with the following meaning:

• Pool-Name is used for IP reservation when a request is a pure RADIUS one (PPP user) which sends back a Framed-IP-Address attribute.
• DHCPv4-Pool-Name is used for IP reservation when a request is a pure DHCPv4 message (DHCPv4 user) which sends back a DHCP-Your-IP-Address attribute.
• DHCPv6-Pool-Name-Pd ise used for IPv6 delegated prefix when a request is a pure RADIUS one which sends a IPv6-Delegated-prefix attribute.

For one of our group in radgroupcheck we only have DHCPv4-Pool-Name & DHCPv6-Pool-Name-Pd attributes set but NOT Pool-Name. When the module is running it is not possible to allocate any IP address as the Pool-Name attribute is 'hard coded'.

https://github.com/FreeRADIUS/freeradius-server/blob/a9c75f03dfda610453b92753d1e5b225965373cf/src/modules/rlm_sqlippool/rlm_sqlippool.c#L487

https://github.com/FreeRADIUS/freeradius-server/blob/a9c75f03dfda610453b92753d1e5b225965373cf/src/modules/rlm_sqlippool/rlm_sqlippool.c#L189

My suggestion would be to have this specific check based on the inst->pool_name variable instead of a hardcoded pool-name. By doig this way we would be able to defined our own Pool-Name attribute inside the module configuration.

if (fr_pair_find_by_da(request->control, inst->pool_name, TAG_ANY) == NULL) {
        RDEBUG("No %s defined",  inst->pool_nam);

        return do_logging(inst, request, inst->log_nopool, RLM_MODULE_NOOP);
}

I can work on this modification and send a pull request if you want.

[alandekok]

I think that makes sense.

[maximumG]

@alandekok : Thanks for the reply. I will work on this ASAP. Do you need this pull request on both master (v4) and v3.0.x branches ?

[alandekok]

I think v3 already has a pool name in it's configuration. So only v4 would be best.

[maximumG]

After checking v3 code and doing some testing, it is indeed possible to define a custom pool_name in the sqlippool configuration but the function which handled checking for attribute is using the 'Pool-Name' hardcoded and not the one from the configuration. I will send 2 pull request for both version.

https://github.com/FreeRADIUS/freeradius-server/blob/a9c75f03dfda610453b92753d1e5b225965373cf/src/modules/rlm_sqlippool/rlm_sqlippool.c#L487

From my understaing the pool_name configuration variable is used to set a default pool_name in case the pool is not found in the check attribute (default value for pool_name: ippool).

@alandekok : Do you confirm that ?

I would rather create a new configuration variable named pool_attribute which will be used to specifiy a custom pool attribute with default value beind 'Pool-Name'. This new configuration would be present along with the existing pool_name.