Setting Client-IP-Address in rlm_preprocess
Client-IP-Address is dynamically set by xlat_packet(). AFAICT, there's no way to trigger this in rlm_perl, so Client-IP-Address is never available in rlm_perl handlers. This leads to some interesting unlang to set Client-IP-Address in the request so rlm_perl can access it: authorize { update request { Client-IP-Address := "%{Client-IP-Address}" } } rlm_preprocess sets NAS-IP-Address if it's not set. Alan, would you accept a patch to add similar behavior for Client-IP-Address? john -- John Morrissey _o /\ ---- __o jwm@horde.net _-< \_ / \ ---- < \, www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__
On Wed, Sep 02, 2009 at 07:37:04PM -0400, John Morrissey wrote:
Client-IP-Address is dynamically set by xlat_packet(). AFAICT, there's no way to trigger this in rlm_perl, so Client-IP-Address is never available in rlm_perl handlers.
This leads to some interesting unlang to set Client-IP-Address in the request so rlm_perl can access it:
authorize { update request { Client-IP-Address := "%{Client-IP-Address}" } }
rlm_preprocess sets NAS-IP-Address if it's not set. Alan, would you accept a patch to add similar behavior for Client-IP-Address?
I forked the github FreeRADIUS tree a while ago and made this change. I sent a pull request for this commit, but haven't seen it pulled into the canonical git tree, so maybe it's preferable to post it to -devel? john commit 0778ffe67b78c56014029f95dce22222e94a110a Author: John Morrissey <jwm@horde.net> Date: Tue Sep 8 16:04:21 2009 +0000 set client address in the request if it's not present, similar to the treatment of nas ip address diff --git a/src/modules/rlm_preprocess/rlm_preprocess.c b/src/modules/rlm_preprocess/rlm_preprocess.c index 5c25569..7fe91da 100644 --- a/src/modules/rlm_preprocess/rlm_preprocess.c +++ b/src/modules/rlm_preprocess/rlm_preprocess.c @@ -455,6 +455,40 @@ static int add_nas_attr(REQUEST *request) return 0; } +static int add_client_attr(REQUEST *request) +{ + VALUE_PAIR *client_addr; + + switch (request->packet->src_ipaddr.af) { + case AF_INET: + client_addr = pairfind(request->packet->vps, PW_CLIENT_IP_ADDRESS); + if (!client_addr) { + client_addr = radius_paircreate(request, &request->packet->vps, + PW_CLIENT_IP_ADDRESS, + PW_TYPE_IPADDR); + client_addr->vp_ipaddr = request->packet->src_ipaddr.ipaddr.ip4addr.s_addr; + } + break; + + case AF_INET6: + client_addr = pairfind(request->packet->vps, PW_CLIENT_IPV6_ADDRESS); + if (!client_addr) { + client_addr = radius_paircreate(request, &request->packet->vps, + PW_CLIENT_IPV6_ADDRESS, + PW_TYPE_IPV6ADDR); + memcpy(client_addr->vp_strvalue, + &request->packet->src_ipaddr.ipaddr, + sizeof(request->packet->src_ipaddr.ipaddr)); + } + break; + + default: + radlog(L_ERR, "Unknown address family for packet"); + return -1; + } + + return 0; +} /* * Initialize. @@ -560,6 +594,10 @@ static int preprocess_authorize(void *instance, REQUEST *request) return RLM_MODULE_FAIL; } + if (add_client_attr(request) < 0) { + return RLM_MODULE_FAIL; + } + hints_setup(data->hints, request); /* @@ -626,6 +664,10 @@ static int preprocess_preaccounting(void *instance, REQUEST *request) return RLM_MODULE_FAIL; } + if (add_client_attr(request) < 0) { + return RLM_MODULE_FAIL; + } + hints_setup(data->hints, request); if ((r = huntgroup_access(request, -- John Morrissey _o /\ ---- __o jwm@horde.net _-< \_ / \ ---- < \, www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__
John Morrissey wrote:
rlm_preprocess sets NAS-IP-Address if it's not set. Alan, would you accept a patch to add similar behavior for Client-IP-Address?
I forked the github FreeRADIUS tree a while ago and made this change. I sent a pull request for this commit, but haven't seen it pulled into the canonical git tree, so maybe it's preferable to post it to -devel?
I'd like to replace Client-IP-Address with Packet-Src-IP-Address, as it's a better name. There's also matching Packet-Dst-IP-Address, IPv6 equivalents, and equivalents for src/dst port. I'd be more comfortable adding *all* of them, rather than just one. Alan DeKok.
On Thu, Oct 22, 2009 at 08:14:06AM +0200, Alan DeKok wrote:
John Morrissey wrote:
rlm_preprocess sets NAS-IP-Address if it's not set. Alan, would you accept a patch to add similar behavior for Client-IP-Address?
I forked the github FreeRADIUS tree a while ago and made this change. I sent a pull request for this commit, but haven't seen it pulled into the canonical git tree, so maybe it's preferable to post it to -devel?
I'd like to replace Client-IP-Address with Packet-Src-IP-Address, as it's a better name. There's also matching Packet-Dst-IP-Address, IPv6 equivalents, and equivalents for src/dst port.
I'd be more comfortable adding *all* of them, rather than just one.
Sure, below. I wasn't sure what to make of the memcpy() for the IPv6 case. VALUE_PAIR_DATA has an ipv6addr field and there's talk in src/include/libradius.h (for vp_ipaddr, but not vp_ipv6addr) of: * These are left as lvalue until we audit the source for code * that prints to vp_strvalue for integer/ipaddr/date types. The surrounding rlm_preprocess code used memcpy() into the strvalue, so I left it that way. john diff --git a/src/modules/rlm_preprocess/rlm_preprocess.c b/src/modules/rlm_preprocess/rlm_preprocess.c index 5c25569..7b1a21d 100644 --- a/src/modules/rlm_preprocess/rlm_preprocess.c +++ b/src/modules/rlm_preprocess/rlm_preprocess.c @@ -417,41 +417,103 @@ static int huntgroup_access(REQUEST *request, PAIR_LIST *huntgroups) } /* - * If the NAS wasn't smart enought to add a NAS-IP-Address - * to the request, then add it ourselves. + * Set NAS-IP-Address, packet source/destination address and port + * attributes if not already set. */ -static int add_nas_attr(REQUEST *request) +static int add_generated_attrs(REQUEST *request) { - VALUE_PAIR *nas; + VALUE_PAIR *vp; switch (request->packet->src_ipaddr.af) { case AF_INET: - nas = pairfind(request->packet->vps, PW_NAS_IP_ADDRESS); - if (!nas) { - nas = radius_paircreate(request, &request->packet->vps, - PW_NAS_IP_ADDRESS, - PW_TYPE_IPADDR); - nas->vp_ipaddr = request->packet->src_ipaddr.ipaddr.ip4addr.s_addr; + vp = pairfind(request->packet->vps, PW_NAS_IP_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_NAS_IP_ADDRESS, + PW_TYPE_IPADDR); + vp->vp_ipaddr = request->packet->src_ipaddr.ipaddr.ip4addr.s_addr; + } + + vp = pairfind(request->packet->vps, PW_PACKET_SRC_IP_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_SRC_IP_ADDRESS, + PW_TYPE_IPADDR); + vp->vp_ipaddr = request->packet->src_ipaddr.ipaddr.ip4addr.s_addr; } break; case AF_INET6: - nas = pairfind(request->packet->vps, PW_NAS_IPV6_ADDRESS); - if (!nas) { - nas = radius_paircreate(request, &request->packet->vps, - PW_NAS_IPV6_ADDRESS, - PW_TYPE_IPV6ADDR); - memcpy(nas->vp_strvalue, + vp = pairfind(request->packet->vps, PW_NAS_IPV6_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_NAS_IPV6_ADDRESS, + PW_TYPE_IPV6ADDR); + memcpy(vp->vp_strvalue, &request->packet->src_ipaddr.ipaddr, sizeof(request->packet->src_ipaddr.ipaddr)); } + + vp = pairfind(request->packet->vps, PW_PACKET_SRC_IPV6_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_SRC_IPV6_ADDRESS, + PW_TYPE_IPV6ADDR); + memcpy(vp->vp_strvalue, + &request->packet->src_ipaddr.ipaddr, + sizeof(request->packet->src_ipaddr.ipaddr)); + } + break; + + default: + radlog(L_ERR, "Unknown address family for packet source."); + return -1; + } + + switch (request->packet->dst_ipaddr.af) { + case AF_INET: + vp = pairfind(request->packet->vps, PW_PACKET_DST_IP_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_DST_IP_ADDRESS, + PW_TYPE_IPADDR); + vp->vp_ipaddr = request->packet->dst_ipaddr.ipaddr.ip4addr.s_addr; + } + break; + + case AF_INET6: + vp = pairfind(request->packet->vps, PW_PACKET_DST_IPV6_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_DST_IPV6_ADDRESS, + PW_TYPE_IPV6ADDR); + memcpy(vp->vp_strvalue, + &request->packet->dst_ipaddr.ipaddr, + sizeof(request->packet->dst_ipaddr.ipaddr)); + } break; default: - radlog(L_ERR, "Unknown address family for packet"); + radlog(L_ERR, "Unknown address family for packet destination."); return -1; } + vp = pairfind(request->packet->vps, PW_PACKET_SRC_PORT); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_SRC_PORT, + PW_TYPE_INTEGER); + vp->vp_integer = request->packet->src_port; + } + + vp = pairfind(request->packet->vps, PW_PACKET_DST_PORT); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_DST_PORT, + PW_TYPE_INTEGER); + vp->vp_integer = request->packet->dst_port; + } + return 0; } @@ -556,7 +618,7 @@ static int preprocess_authorize(void *instance, REQUEST *request) * the Request-Src-IP-Address to be used for huntgroup * comparisons. */ - if (add_nas_attr(request) < 0) { + if (add_generated_attrs(request) < 0) { return RLM_MODULE_FAIL; } @@ -619,10 +681,7 @@ static int preprocess_preaccounting(void *instance, REQUEST *request) alvarion_vsa_hack(request->packet->vps); } - /* - * Ensure that we log the NAS IP Address in the packet. - */ - if (add_nas_attr(request) < 0) { + if (add_generated_attrs(request) < 0) { return RLM_MODULE_FAIL; } -- John Morrissey _o /\ ---- __o jwm@horde.net _-< \_ / \ ---- < \, www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__
On Mon, Oct 26, 2009 at 09:51:47AM -0400, John Morrissey wrote:
On Thu, Oct 22, 2009 at 08:14:06AM +0200, Alan DeKok wrote:
John Morrissey wrote:
rlm_preprocess sets NAS-IP-Address if it's not set. Alan, would you accept a patch to add similar behavior for Client-IP-Address?
I forked the github FreeRADIUS tree a while ago and made this change. I sent a pull request for this commit, but haven't seen it pulled into the canonical git tree, so maybe it's preferable to post it to -devel?
I'd like to replace Client-IP-Address with Packet-Src-IP-Address, as it's a better name. There's also matching Packet-Dst-IP-Address, IPv6 equivalents, and equivalents for src/dst port.
I'd be more comfortable adding *all* of them, rather than just one.
Sure, below.
Any comments on this patch, Alan? Don't mean to be pushy, but it's been a couple weeks and wanted to make sure this didn't get dropped on the floor. john
I wasn't sure what to make of the memcpy() for the IPv6 case. VALUE_PAIR_DATA has an ipv6addr field and there's talk in src/include/libradius.h (for vp_ipaddr, but not vp_ipv6addr) of:
* These are left as lvalue until we audit the source for code * that prints to vp_strvalue for integer/ipaddr/date types.
The surrounding rlm_preprocess code used memcpy() into the strvalue, so I left it that way.
john
diff --git a/src/modules/rlm_preprocess/rlm_preprocess.c b/src/modules/rlm_preprocess/rlm_preprocess.c index 5c25569..7b1a21d 100644 --- a/src/modules/rlm_preprocess/rlm_preprocess.c +++ b/src/modules/rlm_preprocess/rlm_preprocess.c @@ -417,41 +417,103 @@ static int huntgroup_access(REQUEST *request, PAIR_LIST *huntgroups) }
/* - * If the NAS wasn't smart enought to add a NAS-IP-Address - * to the request, then add it ourselves. + * Set NAS-IP-Address, packet source/destination address and port + * attributes if not already set. */ -static int add_nas_attr(REQUEST *request) +static int add_generated_attrs(REQUEST *request) { - VALUE_PAIR *nas; + VALUE_PAIR *vp;
switch (request->packet->src_ipaddr.af) { case AF_INET: - nas = pairfind(request->packet->vps, PW_NAS_IP_ADDRESS); - if (!nas) { - nas = radius_paircreate(request, &request->packet->vps, - PW_NAS_IP_ADDRESS, - PW_TYPE_IPADDR); - nas->vp_ipaddr = request->packet->src_ipaddr.ipaddr.ip4addr.s_addr; + vp = pairfind(request->packet->vps, PW_NAS_IP_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_NAS_IP_ADDRESS, + PW_TYPE_IPADDR); + vp->vp_ipaddr = request->packet->src_ipaddr.ipaddr.ip4addr.s_addr; + } + + vp = pairfind(request->packet->vps, PW_PACKET_SRC_IP_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_SRC_IP_ADDRESS, + PW_TYPE_IPADDR); + vp->vp_ipaddr = request->packet->src_ipaddr.ipaddr.ip4addr.s_addr; } break;
case AF_INET6: - nas = pairfind(request->packet->vps, PW_NAS_IPV6_ADDRESS); - if (!nas) { - nas = radius_paircreate(request, &request->packet->vps, - PW_NAS_IPV6_ADDRESS, - PW_TYPE_IPV6ADDR); - memcpy(nas->vp_strvalue, + vp = pairfind(request->packet->vps, PW_NAS_IPV6_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_NAS_IPV6_ADDRESS, + PW_TYPE_IPV6ADDR); + memcpy(vp->vp_strvalue, &request->packet->src_ipaddr.ipaddr, sizeof(request->packet->src_ipaddr.ipaddr)); } + + vp = pairfind(request->packet->vps, PW_PACKET_SRC_IPV6_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_SRC_IPV6_ADDRESS, + PW_TYPE_IPV6ADDR); + memcpy(vp->vp_strvalue, + &request->packet->src_ipaddr.ipaddr, + sizeof(request->packet->src_ipaddr.ipaddr)); + } + break; + + default: + radlog(L_ERR, "Unknown address family for packet source."); + return -1; + } + + switch (request->packet->dst_ipaddr.af) { + case AF_INET: + vp = pairfind(request->packet->vps, PW_PACKET_DST_IP_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_DST_IP_ADDRESS, + PW_TYPE_IPADDR); + vp->vp_ipaddr = request->packet->dst_ipaddr.ipaddr.ip4addr.s_addr; + } + break; + + case AF_INET6: + vp = pairfind(request->packet->vps, PW_PACKET_DST_IPV6_ADDRESS); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_DST_IPV6_ADDRESS, + PW_TYPE_IPV6ADDR); + memcpy(vp->vp_strvalue, + &request->packet->dst_ipaddr.ipaddr, + sizeof(request->packet->dst_ipaddr.ipaddr)); + } break;
default: - radlog(L_ERR, "Unknown address family for packet"); + radlog(L_ERR, "Unknown address family for packet destination."); return -1; }
+ vp = pairfind(request->packet->vps, PW_PACKET_SRC_PORT); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_SRC_PORT, + PW_TYPE_INTEGER); + vp->vp_integer = request->packet->src_port; + } + + vp = pairfind(request->packet->vps, PW_PACKET_DST_PORT); + if (!vp) { + vp = radius_paircreate(request, &request->packet->vps, + PW_PACKET_DST_PORT, + PW_TYPE_INTEGER); + vp->vp_integer = request->packet->dst_port; + } + return 0; }
@@ -556,7 +618,7 @@ static int preprocess_authorize(void *instance, REQUEST *request) * the Request-Src-IP-Address to be used for huntgroup * comparisons. */ - if (add_nas_attr(request) < 0) { + if (add_generated_attrs(request) < 0) { return RLM_MODULE_FAIL; }
@@ -619,10 +681,7 @@ static int preprocess_preaccounting(void *instance, REQUEST *request) alvarion_vsa_hack(request->packet->vps); }
- /* - * Ensure that we log the NAS IP Address in the packet. - */ - if (add_nas_attr(request) < 0) { + if (add_generated_attrs(request) < 0) { return RLM_MODULE_FAIL; }
-- John Morrissey _o /\ ---- __o jwm@horde.net _-< \_ / \ ---- < \, www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__
John Morrissey wrote:
Any comments on this patch, Alan? Don't mean to be pushy, but it's been a couple weeks and wanted to make sure this didn't get dropped on the floor.
I'm still hesitant. Part of the reason is that there is *already* code to handle this in src/main/xlat.c. The reason you're proposing new code is that the xlat functionality isn't integrated into rlm_perl. It *is* integrated into the rest of the server... It may be better (but a lot more work) to use "tie": http://perldoc.perl.org/functions/tie.html That way, the underlying arrays can be accessed directly, and seamlessly. They won't have to be mangled to/from Perl hashes, as they will *always* be in the RADIUS data structures. The only problem is that it's a lot more work to implement "tie". Alan DeKok.
On Tue, Nov 17, 2009 at 02:51:29PM +0100, Alan DeKok wrote:
John Morrissey wrote:
Any comments on this patch, Alan? Don't mean to be pushy, but it's been a couple weeks and wanted to make sure this didn't get dropped on the floor.
I'm still hesitant. Part of the reason is that there is *already* code to handle this in src/main/xlat.c. The reason you're proposing new code is that the xlat functionality isn't integrated into rlm_perl. It *is* integrated into the rest of the server...
It's not integrated into rlm_python, either. Or anything else that wants this value and isn't explicitly filtering values through the xlat code. john -- John Morrissey _o /\ ---- __o jwm@horde.net _-< \_ / \ ---- < \, www.horde.net/ __(_)/_(_)________/ \_______(_) /_(_)__
Alan DeKok wrote:
John Morrissey wrote:
rlm_preprocess sets NAS-IP-Address if it's not set. Alan, would you accept a patch to add similar behavior for Client-IP-Address? I forked the github FreeRADIUS tree a while ago and made this change. I sent a pull request for this commit, but haven't seen it pulled into the canonical git tree, so maybe it's preferable to post it to -devel?
I'd like to replace Client-IP-Address with Packet-Src-IP-Address, as it's a better name. There's also matching Packet-Dst-IP-Address, IPv6 equivalents, and equivalents for src/dst port.
I'd be more comfortable adding *all* of them, rather than just one.
Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/devel.html
Thats the same place I add Client-Short-Name
participants (3)
-
Alan DeKok -
Joe Maimon -
John Morrissey