--- src/include/modules.h | 11 +++---- src/include/radius.h | 1 + src/lib/dhcp.c | 23 ++++++++++---- src/main/dhcpd.c | 6 +++- src/main/modules.c | 23 +++++++++------ src/modules/rlm_policy/parse.c | 3 +- src/modules/rlm_sql/conf.h | 1 + src/modules/rlm_sql/rlm_sql.c | 61 ++++++++++++++++++++++++++++++++++++++-- 8 files changed, 101 insertions(+), 28 deletions(-) diff --git a/src/include/modules.h b/src/include/modules.h index dbb17f3..2dff528 100644 --- a/src/include/modules.h +++ b/src/include/modules.h @@ -28,11 +28,10 @@ enum { RLM_COMPONENT_PRE_PROXY, /* 5 */ RLM_COMPONENT_POST_PROXY, /* 6 */ RLM_COMPONENT_POST_AUTH, /* 7 */ -#ifdef WITH_COA RLM_COMPONENT_RECV_COA, /* 8 */ RLM_COMPONENT_SEND_COA, /* 9 */ -#endif - RLM_COMPONENT_COUNT /* 8 / 10: How many components are there */ + RLM_COMPONENT_DHCP, /* 10 */ + RLM_COMPONENT_COUNT /* 11: the number of components */ }; #define RLM_TYPE_THREAD_SAFE (0 << 0) @@ -79,9 +78,9 @@ int module_post_auth(int type, REQUEST *request); #ifdef WITH_COA int module_recv_coa(int type, REQUEST *request); int module_send_coa(int type, REQUEST *request); -#define MODULE_NULL_COA_FUNCS ,NULL,NULL -#else -#define MODULE_NULL_COA_FUNCS +#endif +#ifdef WITH_DHCP +int module_dhcp(int type, REQUEST *request); #endif int indexed_modcall(int comp, int idx, REQUEST *request); diff --git a/src/include/radius.h b/src/include/radius.h index 3d9c221..76e4dcb 100644 --- a/src/include/radius.h +++ b/src/include/radius.h @@ -239,6 +239,7 @@ #define PW_MSCHAP_PASSWORD 1133 #define PW_PACKET_TRANSMIT_COUNTER 1134 #define PW_CACHED_SESSION_POLICY 1135 +#define PW_DHCP_TYPE 1136 /* * Integer Translations diff --git a/src/lib/dhcp.c b/src/lib/dhcp.c index 1bcaf44..971fac7 100644 --- a/src/lib/dhcp.c +++ b/src/lib/dhcp.c @@ -1022,18 +1022,26 @@ int fr_dhcp_encode(RADIUS_PACKET *packet, RADIUS_PACKET *original) } } else if ((packet->code == PW_DHCP_NAK) || - ((dhcp->flags & 0x8000) != 0) || - (dhcp->ciaddr == htonl(INADDR_ANY))) { + ((dhcp->flags & 0x8000) != 0)) { /* * The kernel will take care of sending it to * the broadcast MAC. */ - packet->dst_ipaddr.ipaddr.ip4addr.s_addr = htonl(INADDR_BROADCAST); - + packet->dst_ipaddr.ipaddr.ip4addr.s_addr = lvalue; + } else if (dhcp->ciaddr == htonl(INADDR_ANY))) { + + vp = pairfind(packet->vps, 264, DHCP_MAGIC_VENDOR); /* Your IP address */ + if (vp) { + lvalue = vp->vp_ipaddr; + packet->offset = 0; + } else { + lvalue = htonl(INADDR_BROADCAST); + } + packet->dst_ipaddr.ipaddr.ip4addr.s_addr = lvalue; } else { /* - * It was broadcast to us: we need to - * broadcast the response. + * It was unicast to us: we need to + * unicast the response. */ if (packet->src_ipaddr.ipaddr.ip4addr.s_addr != dhcp->ciaddr) { packet->offset = 0; @@ -1286,7 +1294,8 @@ int fr_dhcp_encode(RADIUS_PACKET *packet, RADIUS_PACKET *original) for (i = 0; i < 14; i++) { vp = pairmake(dhcp_header_names[i], NULL, T_OP_EQ); if (!vp) { - fr_strerror_printf("Parse error %s", fr_strerror()); + fr_strerror_printf("%s Parse error %s", + fr_strerror(), dhcp_header_names[i]); return -1; } diff --git a/src/main/dhcpd.c b/src/main/dhcpd.c index 975f159..7247e42 100644 --- a/src/main/dhcpd.c +++ b/src/main/dhcpd.c @@ -52,7 +52,11 @@ static int dhcp_process(REQUEST *request) DICT_VALUE *dv = dict_valbyattr(53, DHCP_MAGIC_VENDOR, vp->vp_integer); DEBUG("Trying sub-section dhcp %s {...}", dv->name ? dv->name : "<unknown>"); - rcode = module_post_auth(vp->vp_integer, request); + rcode = module_dhcp(vp->vp_integer, request); + if (rcode == RLM_MODULE_REJECT) { + DEBUG("Fallback, trying sub-section dhcp {...}"); + rcode = module_dhcp(0, request); + } } else { DEBUG("DHCP: Failed to find DHCP-Message-Type in packet!"); rcode = RLM_MODULE_FAIL; diff --git a/src/main/modules.c b/src/main/modules.c index 8984f92..7f0fa9b 100644 --- a/src/main/modules.c +++ b/src/main/modules.c @@ -84,12 +84,10 @@ static const section_type_value_t section_type_value[RLM_COMPONENT_COUNT] = { { "session", "Session-Type", PW_SESSION_TYPE }, { "pre-proxy", "Pre-Proxy-Type", PW_PRE_PROXY_TYPE }, { "post-proxy", "Post-Proxy-Type", PW_POST_PROXY_TYPE }, - { "post-auth", "Post-Auth-Type", PW_POST_AUTH_TYPE } -#ifdef WITH_COA - , + { "post-auth", "Post-Auth-Type", PW_POST_AUTH_TYPE }, { "recv-coa", "Recv-CoA-Type", PW_RECV_COA_TYPE }, - { "send-coa", "Send-CoA-Type", PW_SEND_COA_TYPE } -#endif + { "send-coa", "Send-CoA-Type", PW_SEND_COA_TYPE }, + { "dhcp", "DHCP-Type", PW_DHCP_TYPE }, }; @@ -804,7 +802,7 @@ static int load_subcomponent_section(modcallable *parent, CONF_SECTION *cs, DICT_VALUE *dval; const char *name2 = cf_section_name2(cs); - rad_assert(comp >= RLM_COMPONENT_AUTH); + rad_assert(comp >= 0); rad_assert(comp < RLM_COMPONENT_COUNT); /* @@ -1209,12 +1207,12 @@ static int load_byserver(CONF_SECTION *cs) if (!load_subcomponent_section(NULL, subcs, components, dattr->attr, - RLM_COMPONENT_POST_AUTH)) { + RLM_COMPONENT_DHCP)) { goto error; /* FIXME: memleak? */ } c = lookup_by_index(components, - RLM_COMPONENT_POST_AUTH, 0); - if (c) server->mc[RLM_COMPONENT_POST_AUTH] = c->modulelist; + RLM_COMPONENT_DHCP, 0); + if (c) server->mc[RLM_COMPONENT_DHCP] = c->modulelist; flag = 1; } } @@ -1714,3 +1712,10 @@ int module_send_coa(int send_coa_type, REQUEST *request) return indexed_modcall(RLM_COMPONENT_SEND_COA, send_coa_type, request); } #endif + +#ifdef WITH_DHCP +int module_dhcp(int dhcp_type, REQUEST *request) +{ + return indexed_modcall(RLM_COMPONENT_DHCP, dhcp_type, request); +} +#endif diff --git a/src/modules/rlm_policy/parse.c b/src/modules/rlm_policy/parse.c index 2992ab4..a6db9d7 100644 --- a/src/modules/rlm_policy/parse.c +++ b/src/modules/rlm_policy/parse.c @@ -1094,10 +1094,9 @@ const FR_NAME_NUMBER policy_component_names[] = { { "pre-proxy", RLM_COMPONENT_PRE_PROXY }, { "post-proxy", RLM_COMPONENT_POST_PROXY }, { "post-auth", RLM_COMPONENT_POST_AUTH }, -#ifdef WITH_COA { "recv-coa", RLM_COMPONENT_RECV_COA }, { "send-coa", RLM_COMPONENT_SEND_COA }, -#endif + { "dhcp", RLM_COMPONENT_DHCP }, { NULL, RLM_COMPONENT_COUNT } }; diff --git a/src/modules/rlm_sql/conf.h b/src/modules/rlm_sql/conf.h index d1faae0..aac2084 100644 --- a/src/modules/rlm_sql/conf.h +++ b/src/modules/rlm_sql/conf.h @@ -47,6 +47,7 @@ typedef struct sql_config { int max_queries; int connect_failure_retry_delay; char *postauth_query; + char *dhcp_query; char *allowed_chars; int query_timeout; diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index 0982363..bfe197c 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -109,6 +109,10 @@ static const CONF_PARSER module_config[] = { #endif {"postauth_query", PW_TYPE_STRING_PTR, offsetof(SQL_CONFIG,postauth_query), NULL, ""}, +#ifdef WITH_DHCP + {"dhcp_query", PW_TYPE_STRING_PTR, + offsetof(SQL_CONFIG,dhcp_query), NULL, ""}, +#endif {"safe-characters", PW_TYPE_STRING_PTR, offsetof(SQL_CONFIG,allowed_chars), NULL, "@abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_: /"}, @@ -1097,6 +1101,7 @@ static int rlm_sql_authorize(void *instance, REQUEST * request) pairxlatmove(request, &request->reply->vps, &reply_tmp); } pairxlatmove(request, &request->config_items, &check_tmp); + pairfree(&reply_tmp); } } @@ -1104,7 +1109,6 @@ static int rlm_sql_authorize(void *instance, REQUEST * request) * Clear out the pairlists */ pairfree(&check_tmp); - pairfree(&reply_tmp); /* * dofallthrough is set to 1 by default so that if the user information @@ -1175,7 +1179,7 @@ static int rlm_sql_authorize(void *instance, REQUEST * request) RDEBUG("User %s not found", sqlusername); return RLM_MODULE_NOTFOUND; } else { - return RLM_MODULE_OK; + return RLM_MODULE_UPDATED; } } @@ -1661,6 +1665,50 @@ static int rlm_sql_postauth(void *instance, REQUEST *request) { return RLM_MODULE_OK; } +#ifdef WITH_DHCP +static int rlm_sql_dhcp(void *instance, REQUEST * request) +{ + VALUE_PAIR *reply_tmp = NULL; + int rows; + SQLSOCK *sqlsocket; + SQL_INST *inst = instance; + char querystr[MAX_QUERY_LEN]; + + /* + * reserve a socket + */ + if (!(sqlsocket = sql_get_socket(inst))) + return RLM_MODULE_FAIL; + + /* + * After this point, ALL 'return's MUST release the SQL socket! + */ + + if (!radius_xlat(querystr, sizeof(querystr), inst->config->dhcp_query, request, sql_escape_func)) { + radlog_request(L_ERR, 0, request, "Error generating query; rejecting DHCP request"); + sql_release_socket(inst, sqlsocket); + return RLM_MODULE_FAIL; + } + rows = sql_getvpdata(inst, sqlsocket, &reply_tmp, querystr); + if (rows < 0) { + radlog_request(L_ERR, 0, request, "SQL query error; rejecting DHCP request"); + sql_release_socket(inst, sqlsocket); + pairfree(&reply_tmp); + return RLM_MODULE_FAIL; + } else if (rows > 0) + pairxlatmove(request, &request->reply->vps, &reply_tmp); + + sql_release_socket(inst, sqlsocket); + pairfree(&reply_tmp); + + if (rows > 0) + return RLM_MODULE_UPDATED; + + RDEBUG("DHCP query returned nothing"); + return RLM_MODULE_NOTFOUND; +} +#endif + /* globally exported name */ module_t rlm_sql = { RLM_MODULE_INIT, @@ -1684,6 +1732,13 @@ module_t rlm_sql = { #endif NULL, /* pre-proxy */ NULL, /* post-proxy */ - rlm_sql_postauth /* post-auth */ + rlm_sql_postauth, /* post-auth */ + NULL, /* recv-coa */ + NULL, /* send-coa */ +#ifdef WITH_DHCP + rlm_sql_dhcp, /* dhcp */ +#else + NULL, +#endif }, }; -- 1.7.5.4
This can't be added... for one, it removes the #ifdef WITH_COA stuff. There's no reason for that. It adds a RLM_COMPONENT_DHCP which is nice... but has some issues. The RLM_COMPONENT_DHCP code is *not* conditionally compiled. So it's still there even if you don't build with DHCP. Adding RLM_COMPONENT_DHCP means that *all* of the modules need a DHCP component. That's why the modules have entries for the COA code. It initializes the array entries so that the CoA entry exist if CoA enabled. Right now, most modules don't have a DHCP entry. This means that if someone accidentally lists a module in a DHCP section, the "modcall.c" code will walk off of the end of the module_t array. The real way to do this is to change the code to allow it to dynamically register callbacks. That requires a lot more work, unfortunately. For now, the DHCP code works by overloading the "post-auth" section. It's ugly, but it pretty much works. Stephen R. van den Berg wrote:
--- src/include/modules.h | 11 +++---- src/include/radius.h | 1 + src/lib/dhcp.c | 23 ++++++++++---- src/main/dhcpd.c | 6 +++- src/main/modules.c | 23 +++++++++------ src/modules/rlm_policy/parse.c | 3 +- src/modules/rlm_sql/conf.h | 1 + src/modules/rlm_sql/rlm_sql.c | 61 ++++++++++++++++++++++++++++++++++++++-- 8 files changed, 101 insertions(+), 28 deletions(-)
diff --git a/src/include/modules.h b/src/include/modules.h index dbb17f3..2dff528 100644 --- a/src/include/modules.h +++ b/src/include/modules.h @@ -28,11 +28,10 @@ enum { RLM_COMPONENT_PRE_PROXY, /* 5 */ RLM_COMPONENT_POST_PROXY, /* 6 */ RLM_COMPONENT_POST_AUTH, /* 7 */ -#ifdef WITH_COA RLM_COMPONENT_RECV_COA, /* 8 */ RLM_COMPONENT_SEND_COA, /* 9 */ -#endif - RLM_COMPONENT_COUNT /* 8 / 10: How many components are there */ + RLM_COMPONENT_DHCP, /* 10 */ + RLM_COMPONENT_COUNT /* 11: the number of components */ };
#define RLM_TYPE_THREAD_SAFE (0 << 0) @@ -79,9 +78,9 @@ int module_post_auth(int type, REQUEST *request); #ifdef WITH_COA int module_recv_coa(int type, REQUEST *request); int module_send_coa(int type, REQUEST *request); -#define MODULE_NULL_COA_FUNCS ,NULL,NULL -#else -#define MODULE_NULL_COA_FUNCS +#endif +#ifdef WITH_DHCP +int module_dhcp(int type, REQUEST *request); #endif int indexed_modcall(int comp, int idx, REQUEST *request);
diff --git a/src/include/radius.h b/src/include/radius.h index 3d9c221..76e4dcb 100644 --- a/src/include/radius.h +++ b/src/include/radius.h @@ -239,6 +239,7 @@ #define PW_MSCHAP_PASSWORD 1133 #define PW_PACKET_TRANSMIT_COUNTER 1134 #define PW_CACHED_SESSION_POLICY 1135 +#define PW_DHCP_TYPE 1136
/* * Integer Translations diff --git a/src/lib/dhcp.c b/src/lib/dhcp.c index 1bcaf44..971fac7 100644 --- a/src/lib/dhcp.c +++ b/src/lib/dhcp.c @@ -1022,18 +1022,26 @@ int fr_dhcp_encode(RADIUS_PACKET *packet, RADIUS_PACKET *original) }
} else if ((packet->code == PW_DHCP_NAK) || - ((dhcp->flags & 0x8000) != 0) || - (dhcp->ciaddr == htonl(INADDR_ANY))) { + ((dhcp->flags & 0x8000) != 0)) { /* * The kernel will take care of sending it to * the broadcast MAC. */ - packet->dst_ipaddr.ipaddr.ip4addr.s_addr = htonl(INADDR_BROADCAST); - + packet->dst_ipaddr.ipaddr.ip4addr.s_addr = lvalue; + } else if (dhcp->ciaddr == htonl(INADDR_ANY))) { + + vp = pairfind(packet->vps, 264, DHCP_MAGIC_VENDOR); /* Your IP address */ + if (vp) { + lvalue = vp->vp_ipaddr; + packet->offset = 0; + } else { + lvalue = htonl(INADDR_BROADCAST); + } + packet->dst_ipaddr.ipaddr.ip4addr.s_addr = lvalue; } else { /* - * It was broadcast to us: we need to - * broadcast the response. + * It was unicast to us: we need to + * unicast the response. */ if (packet->src_ipaddr.ipaddr.ip4addr.s_addr != dhcp->ciaddr) { packet->offset = 0; @@ -1286,7 +1294,8 @@ int fr_dhcp_encode(RADIUS_PACKET *packet, RADIUS_PACKET *original) for (i = 0; i < 14; i++) { vp = pairmake(dhcp_header_names[i], NULL, T_OP_EQ); if (!vp) { - fr_strerror_printf("Parse error %s", fr_strerror()); + fr_strerror_printf("%s Parse error %s", + fr_strerror(), dhcp_header_names[i]); return -1; }
diff --git a/src/main/dhcpd.c b/src/main/dhcpd.c index 975f159..7247e42 100644 --- a/src/main/dhcpd.c +++ b/src/main/dhcpd.c @@ -52,7 +52,11 @@ static int dhcp_process(REQUEST *request) DICT_VALUE *dv = dict_valbyattr(53, DHCP_MAGIC_VENDOR, vp->vp_integer); DEBUG("Trying sub-section dhcp %s {...}", dv->name ? dv->name : "<unknown>"); - rcode = module_post_auth(vp->vp_integer, request); + rcode = module_dhcp(vp->vp_integer, request); + if (rcode == RLM_MODULE_REJECT) { + DEBUG("Fallback, trying sub-section dhcp {...}"); + rcode = module_dhcp(0, request); + } } else { DEBUG("DHCP: Failed to find DHCP-Message-Type in packet!"); rcode = RLM_MODULE_FAIL; diff --git a/src/main/modules.c b/src/main/modules.c index 8984f92..7f0fa9b 100644 --- a/src/main/modules.c +++ b/src/main/modules.c @@ -84,12 +84,10 @@ static const section_type_value_t section_type_value[RLM_COMPONENT_COUNT] = { { "session", "Session-Type", PW_SESSION_TYPE }, { "pre-proxy", "Pre-Proxy-Type", PW_PRE_PROXY_TYPE }, { "post-proxy", "Post-Proxy-Type", PW_POST_PROXY_TYPE }, - { "post-auth", "Post-Auth-Type", PW_POST_AUTH_TYPE } -#ifdef WITH_COA - , + { "post-auth", "Post-Auth-Type", PW_POST_AUTH_TYPE }, { "recv-coa", "Recv-CoA-Type", PW_RECV_COA_TYPE }, - { "send-coa", "Send-CoA-Type", PW_SEND_COA_TYPE } -#endif + { "send-coa", "Send-CoA-Type", PW_SEND_COA_TYPE }, + { "dhcp", "DHCP-Type", PW_DHCP_TYPE }, };
@@ -804,7 +802,7 @@ static int load_subcomponent_section(modcallable *parent, CONF_SECTION *cs, DICT_VALUE *dval; const char *name2 = cf_section_name2(cs);
- rad_assert(comp >= RLM_COMPONENT_AUTH); + rad_assert(comp >= 0); rad_assert(comp < RLM_COMPONENT_COUNT);
/* @@ -1209,12 +1207,12 @@ static int load_byserver(CONF_SECTION *cs) if (!load_subcomponent_section(NULL, subcs, components, dattr->attr, - RLM_COMPONENT_POST_AUTH)) { + RLM_COMPONENT_DHCP)) { goto error; /* FIXME: memleak? */ } c = lookup_by_index(components, - RLM_COMPONENT_POST_AUTH, 0); - if (c) server->mc[RLM_COMPONENT_POST_AUTH] = c->modulelist; + RLM_COMPONENT_DHCP, 0); + if (c) server->mc[RLM_COMPONENT_DHCP] = c->modulelist; flag = 1; } } @@ -1714,3 +1712,10 @@ int module_send_coa(int send_coa_type, REQUEST *request) return indexed_modcall(RLM_COMPONENT_SEND_COA, send_coa_type, request); } #endif + +#ifdef WITH_DHCP +int module_dhcp(int dhcp_type, REQUEST *request) +{ + return indexed_modcall(RLM_COMPONENT_DHCP, dhcp_type, request); +} +#endif diff --git a/src/modules/rlm_policy/parse.c b/src/modules/rlm_policy/parse.c index 2992ab4..a6db9d7 100644 --- a/src/modules/rlm_policy/parse.c +++ b/src/modules/rlm_policy/parse.c @@ -1094,10 +1094,9 @@ const FR_NAME_NUMBER policy_component_names[] = { { "pre-proxy", RLM_COMPONENT_PRE_PROXY }, { "post-proxy", RLM_COMPONENT_POST_PROXY }, { "post-auth", RLM_COMPONENT_POST_AUTH }, -#ifdef WITH_COA { "recv-coa", RLM_COMPONENT_RECV_COA }, { "send-coa", RLM_COMPONENT_SEND_COA }, -#endif + { "dhcp", RLM_COMPONENT_DHCP }, { NULL, RLM_COMPONENT_COUNT } };
diff --git a/src/modules/rlm_sql/conf.h b/src/modules/rlm_sql/conf.h index d1faae0..aac2084 100644 --- a/src/modules/rlm_sql/conf.h +++ b/src/modules/rlm_sql/conf.h @@ -47,6 +47,7 @@ typedef struct sql_config { int max_queries; int connect_failure_retry_delay; char *postauth_query; + char *dhcp_query; char *allowed_chars; int query_timeout;
diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index 0982363..bfe197c 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -109,6 +109,10 @@ static const CONF_PARSER module_config[] = { #endif {"postauth_query", PW_TYPE_STRING_PTR, offsetof(SQL_CONFIG,postauth_query), NULL, ""}, +#ifdef WITH_DHCP + {"dhcp_query", PW_TYPE_STRING_PTR, + offsetof(SQL_CONFIG,dhcp_query), NULL, ""}, +#endif {"safe-characters", PW_TYPE_STRING_PTR, offsetof(SQL_CONFIG,allowed_chars), NULL, "@abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789.-_: /"}, @@ -1097,6 +1101,7 @@ static int rlm_sql_authorize(void *instance, REQUEST * request) pairxlatmove(request, &request->reply->vps, &reply_tmp); } pairxlatmove(request, &request->config_items, &check_tmp); + pairfree(&reply_tmp); } }
@@ -1104,7 +1109,6 @@ static int rlm_sql_authorize(void *instance, REQUEST * request) * Clear out the pairlists */ pairfree(&check_tmp); - pairfree(&reply_tmp);
/* * dofallthrough is set to 1 by default so that if the user information @@ -1175,7 +1179,7 @@ static int rlm_sql_authorize(void *instance, REQUEST * request) RDEBUG("User %s not found", sqlusername); return RLM_MODULE_NOTFOUND; } else { - return RLM_MODULE_OK; + return RLM_MODULE_UPDATED; } }
@@ -1661,6 +1665,50 @@ static int rlm_sql_postauth(void *instance, REQUEST *request) { return RLM_MODULE_OK; }
+#ifdef WITH_DHCP +static int rlm_sql_dhcp(void *instance, REQUEST * request) +{ + VALUE_PAIR *reply_tmp = NULL; + int rows; + SQLSOCK *sqlsocket; + SQL_INST *inst = instance; + char querystr[MAX_QUERY_LEN]; + + /* + * reserve a socket + */ + if (!(sqlsocket = sql_get_socket(inst))) + return RLM_MODULE_FAIL; + + /* + * After this point, ALL 'return's MUST release the SQL socket! + */ + + if (!radius_xlat(querystr, sizeof(querystr), inst->config->dhcp_query, request, sql_escape_func)) { + radlog_request(L_ERR, 0, request, "Error generating query; rejecting DHCP request"); + sql_release_socket(inst, sqlsocket); + return RLM_MODULE_FAIL; + } + rows = sql_getvpdata(inst, sqlsocket, &reply_tmp, querystr); + if (rows < 0) { + radlog_request(L_ERR, 0, request, "SQL query error; rejecting DHCP request"); + sql_release_socket(inst, sqlsocket); + pairfree(&reply_tmp); + return RLM_MODULE_FAIL; + } else if (rows > 0) + pairxlatmove(request, &request->reply->vps, &reply_tmp); + + sql_release_socket(inst, sqlsocket); + pairfree(&reply_tmp); + + if (rows > 0) + return RLM_MODULE_UPDATED; + + RDEBUG("DHCP query returned nothing"); + return RLM_MODULE_NOTFOUND; +} +#endif + /* globally exported name */ module_t rlm_sql = { RLM_MODULE_INIT, @@ -1684,6 +1732,13 @@ module_t rlm_sql = { #endif NULL, /* pre-proxy */ NULL, /* post-proxy */ - rlm_sql_postauth /* post-auth */ + rlm_sql_postauth, /* post-auth */ + NULL, /* recv-coa */ + NULL, /* send-coa */ +#ifdef WITH_DHCP + rlm_sql_dhcp, /* dhcp */ +#else + NULL, +#endif }, };
Alan DeKok wrote:
This can't be added... for one, it removes the #ifdef WITH_COA stuff. There's no reason for that.
Only in the definition of the "jumptable". This in order to ensure that the number of entries there doesn't shift up and down with WITH_COA being present or not.
It adds a RLM_COMPONENT_DHCP which is nice... but has some issues.
The RLM_COMPONENT_DHCP code is *not* conditionally compiled. So it's still there even if you don't build with DHCP.
Yes, on purpose, to keep the "binary" interface the same. It's extremely messy trying to debug compilation issues when the modules are not all properly rebuilt (yes, there are issues with the Makefiles too). The components being callable or not is solved elsewhere.
Adding RLM_COMPONENT_DHCP means that *all* of the modules need a DHCP component. That's why the modules have entries for the COA code. It initializes the array entries so that the CoA entry exist if CoA enabled. Right now, most modules don't have a DHCP entry. This means that if someone accidentally lists a module in a DHCP section, the "modcall.c" code will walk off of the end of the module_t array.
Didn't I add null values in every module that didn't have it? Hmmm..., maybe I didn't. What if I fixed this?
The real way to do this is to change the code to allow it to dynamically register callbacks. That requires a lot more work, unfortunately.
Yes, sounds like more work than I bargained for. Is the fix mentioned above enough to get it in?
For now, the DHCP code works by overloading the "post-auth" section. It's ugly, but it pretty much works.
Well, I tried it, but in combination with SQL it sort of didn't work. Was I too soon to judge? -- Stephen.
Stephen R. van den Berg wrote:
Only in the definition of the "jumptable". This in order to ensure that the number of entries there doesn't shift up and down with WITH_COA being present or not.
That's what #ifdef's are for...
The RLM_COMPONENT_DHCP code is *not* conditionally compiled. So it's still there even if you don't build with DHCP.
Yes, on purpose, to keep the "binary" interface the same. It's extremely messy trying to debug compilation issues when the modules are not all properly rebuilt (yes, there are issues with the Makefiles too).
I don't see why. src/main/rules.mak ensures that all of the modules depend on src/include/modules.h. So any changes to that will result in the modules being re-built.
Adding RLM_COMPONENT_DHCP means that *all* of the modules need a DHCP component. That's why the modules have entries for the COA code. It initializes the array entries so that the CoA entry exist if CoA enabled. Right now, most modules don't have a DHCP entry. This means that if someone accidentally lists a module in a DHCP section, the "modcall.c" code will walk off of the end of the module_t array.
Didn't I add null values in every module that didn't have it? Hmmm..., maybe I didn't. What if I fixed this?
No. *All* of the modules need to be updated. Every single last one. The patch changed only a select few.
The real way to do this is to change the code to allow it to dynamically register callbacks. That requires a lot more work, unfortunately.
Yes, sounds like more work than I bargained for. Is the fix mentioned above enough to get it in?
Not really, no.
For now, the DHCP code works by overloading the "post-auth" section. It's ugly, but it pretty much works.
Well, I tried it, but in combination with SQL it sort of didn't work. Was I too soon to judge?
The SQL code doesn't do dynamic IP allocation for DHCP. That requires some fairly small changes to sqlippool. That's pretty much it, IIRC. Alan DeKok.
Alan DeKok wrote:
Stephen R. van den Berg wrote:
Only in the definition of the "jumptable". This in order to ensure that the number of entries there doesn't shift up and down with WITH_COA being present or not.
That's what #ifdef's are for...
Sometimes #ifdefs are considered ugly. I'll adjust my policy for freeradius source code.
properly rebuilt (yes, there are issues with the Makefiles too).
I don't see why. src/main/rules.mak ensures that all of the modules depend on src/include/modules.h. So any changes to that will result in the modules being re-built.
Well, maybe I'll figure it out somewhere along the line. Suffice it to say that had cases where I modified files, but it didn't result in new binaries. Maybe the debian stuff has problems.
No. *All* of the modules need to be updated. Every single last one. The patch changed only a select few.
The real way to do this is to change the code to allow it to dynamically register callbacks. That requires a lot more work, unfortunately.
Yes, sounds like more work than I bargained for. Is the fix mentioned above enough to get it in?
Not really, no.
Suppose I update all modules, and put back the ifdefs, would that help? -- Stephen. "What do I do when I see someone *extremely gorgeous*? I stare, I smile, and when I get tired... I put the mirror down."
participants (2)
-
Alan DeKok -
Stephen R. van den Berg