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/ __(_)/_(_)________/ \_______(_) /_(_)__