Setting Client-IP-Address in rlm_preprocess

[John Morrissey]

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 at horde.net        _-< \_          /  \       ----  <  \,
www.horde.net/    __(_)/_(_)________/    \_______(_) /_(_)__