Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlippool Pool-Name not defined #2282

Open
maximumG opened this Issue Aug 20, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@maximumG
Copy link

maximumG commented Aug 20, 2018

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'.

if (fr_pair_find_by_da(request->control, attr_pool_name, TAG_ANY) == NULL) {

{ .out = &attr_pool_name, .name = "Pool-Name", .type = FR_TYPE_STRING, .dict = &dict_freeradius },

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

This comment has been minimized.

Copy link
Member

alandekok commented Aug 21, 2018

I think that makes sense.

@maximumG

This comment has been minimized.

Copy link
Author

maximumG commented Aug 22, 2018

@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

This comment has been minimized.

Copy link
Member

alandekok commented Aug 22, 2018

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

@maximumG

This comment has been minimized.

Copy link
Author

maximumG commented Aug 22, 2018

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.

if (fr_pair_find_by_da(request->control, attr_pool_name, TAG_ANY) == NULL) {

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.