[Devel] Inaccurate Radius Accounting

Klaus Darilion klaus.mailinglists at pernau.at
Thu Jan 5 10:49:16 CET 2006


Bogdan-Andrei Iancu wrote:
> Hi Lenir, Hi Klaus,
> 
> sure - no problem integrating the functionality, but the patch needs to 
> be completely re-written as most of the newly functions added by Jan 
> already exists in openser - functions related to RR, like determining 
> the direction of a request.

Yes, the ftag parsing. Nevertheless, Jan' patch works, but probably 
there is lot of duplicated code.

regards
klaus

> 
> regards,
> bogdan
> 
> Klaus Darilion wrote:
> 
>> I'm on it - just testing at the moment.
>>
>> regards
>> klaus
>>
>> Lenir wrote:
>>
>>> Can you guys implement this patch (written by Jan Janak) for acc 
>>> radius to
>>> CVS?
>>> It's been implemented to SER CVS.
>>>
>>> Thanks,
>>>
>>> Lenir
>>>
>>> -----Original Message-----
>>> From: 'Jan Janak' [mailto:jan at iptel.org] Sent: Sunday, December 25, 
>>> 2005 12:56 PM
>>> To: Lenir
>>> Cc: 'Klaus Darilion'; serdev at iptel.org; serusers at iptel.org;
>>> devel at openser.org; users at openser.org
>>> Subject: Re: [Users] RE: [Serusers] Re: [Serdev] Inaccurate Radius
>>> Accounting
>>>
>>> Hello,
>>>
>>> Attached is a patch that implements "swap_direction" parameter of acc
>>> module. If you turn the parameter on in the configuration file:
>>>
>>> modparam("acc", "swap_direction", 1)
>>>
>>> then the acc module will swap Calling-Station-ID and Called-Station-ID
>>> values when necessary (in this case BYE comming from the callee).
>>>
>>> Put the patch in top level source directory (ser-0.9.x) and type:
>>> patch -p0 < swap.patch
>>>
>>> The patch should work with any 0.9.x release.
>>>
>>> Note that Alan DeKok is wrong in thinking that this is a bug in SER.
>>> This particular problem is a result of incomplete specification of
>>> RADIUS use with SIP. I will commit the patch in CVS so it will be
>>> included in future SER releases.
>>>
>>>    Jan.
>>>
>>> On 23-12-2005 17:10, Lenir wrote:
>>>
>>>> Please read the reply below from one of the maintainers of freeradius:
>>>>
>>>> "Lenir" <lenirsantiago at yahoo.com> wrote:
>>>>
>>>>> But if UserB hangs up on UserA: SER generates a stop-record where 
>>>>> the Calling-Station-Id is UserB and the Called-Station-Id is UserA, 
>>>>> this is the undesired and incorrect behavior.
>>>>
>>>>
>>>>
>>>>  It would appear to be a bug in SER.
>>>>
>>>>
>>>>> To me the Calling-Station-Id and the Called-Station-Id should be 
>>>>> the same for both start and stop records, am I right by thinking that?
>>>>
>>>>
>>>>
>>>>  Yes.
>>>>
>>>>
>>>>> According to the developers of SER/OpenSER, this is the correct 
>>>>> behavior, whoever sends the hangup signal (BYE or CANCEL) is 
>>>>> considered the Calling-Station-Id, and they are unwilling to modify 
>>>>> or create a patch to "fix" this.
>>>>
>>>>
>>>>
>>>>  What they do for something inside of SER is their business.  When they
>>>> generate RADIUS packets, they should follow RADIUS standards and
>>>> interoperability.  The expectation, as you said, is that the
>>>> Calling/Called-Station-Id doesn't change during a session.  If it does,
>>>
>>>
>>>
>>> it's
>>>
>>>> a bug and they should fix it.
>>>>
>>>>  Alan DeKok.
>>>> -
>>>> List info/subscribe/unsubscribe? See
>>>> http://www.freeradius.org/list/users.html
>>>>
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: 'Jan Janak' [mailto:jan at iptel.org] Sent: Thursday, November 
>>>> 17, 2005 10:33 AM
>>>> To: Lenir
>>>> Cc: 'Klaus Darilion'; serdev at iptel.org; serusers at iptel.org;
>>>> devel at openser.org; users at openser.org
>>>> Subject: Re: [Users] RE: [Serusers] Re: [Serdev] Inaccurate Radius
>>>> Accounting
>>>>
>>>> On 17-11-2005 10:21, Lenir wrote:
>>>>
>>>>> In this case the radius proxy wont work, because you never can
>>>>
>>>>
>>>
>>> anticipate
>>>
>>>>> who hangs up the call, thus radius wont know who hung up the
>>>>
>>>>
>>>>
>>>> call...Besides
>>>>
>>>>> all other voice applications/hardware (SIP and H323) that use 
>>>>> radius do
>>>>
>>>>
>>>>
>>>> not
>>>>
>>>>> behave like that, the Called-Station-ID ALWAYS remains the same, as 
>>>>> with
>>>>
>>>>
>>>>
>>>> the
>>>>
>>>>> Calling-Station-ID.
>>>>
>>>>
>>>>
>>>>  Could you name those SIP applications that behave the way you 
>>>> describe ?
>>>>
>>>>    Jan.
>>>>
>>>>
>>>>
>>>> ------------------------------------------------------------------------ 
>>>>
>>>>
>>>> RCS file: /cvsroot/ser/sip_router/modules/acc/Attic/acc.c,v
>>>> retrieving revision 1.26.2.1
>>>> diff -u -r1.26.2.1 acc.c
>>>> --- modules/acc/acc.c    14 Jun 2005 20:25:19 -0000    1.26.2.1
>>>> +++ modules/acc/acc.c    25 Dec 2005 17:35:19 -0000
>>>> @@ -49,6 +49,9 @@
>>>> #include "acc_mod.h"
>>>> #include "acc.h"
>>>> #include "dict.h"
>>>> +#include "../../parser/parse_rr.h"
>>>> +#include "../../trim.h"
>>>> +#include "../../parser/parse_uri.h"
>>>> #ifdef RAD_ACC
>>>> #  ifdef RADIUSCLIENT_NG_4
>>>> #    include <radiusclient.h>
>>>> @@ -71,6 +74,8 @@
>>>> #define M_NAME    "acc"
>>>> #endif
>>>>
>>>> +extern int swap_dir;
>>>> +
>>>> #define ATR(atr)  atr_arr[cnt].s=A_##atr;\
>>>>                 atr_arr[cnt].len=A_##atr##_LEN;
>>>>
>>>> @@ -101,6 +106,94 @@
>>>> #endif
>>>>
>>>>
>>>> +static int check_ftag(struct sip_msg* msg, str* uri)
>>>> +{
>>>> +    param_hooks_t hooks;
>>>> +    param_t* params;
>>>> +    char* semi;
>>>> +    struct to_body* from;
>>>> +    str t;
>>>> +
>>>> +    t = *uri;
>>>> +    params = 0;
>>>> +    semi = q_memchr(t.s, ';', t.len);
>>>> +    if (!semi) {
>>>> +        DBG("No ftag parameter found\n");
>>>> +        return -1;
>>>> +    }
>>>> +   +    t.len -= semi - uri->s + 1;
>>>> +    t.s = semi + 1;
>>>> +    trim_leading(&t);
>>>> +   +    if (parse_params(&t, CLASS_URI, &hooks, &params) < 0) {
>>>> +        LOG(L_ERR, "Error while parsing parameters\n");
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    if (!hooks.uri.ftag) {
>>>> +        DBG("No ftag parameter found\n");
>>>> +        goto err;
>>>> +    }
>>>> +
>>>> +    from = get_from(msg);
>>>> +
>>>> +    if (!from || !from->tag_value.len || !from->tag_value.s) {
>>>> +        DBG("No from tag parameter found\n");
>>>> +        goto err;
>>>> +    }
>>>> +
>>>> +    if (from->tag_value.len == hooks.uri.ftag->body.len &&
>>>> +        !strncmp(from->tag_value.s, hooks.uri.ftag->body.s, 
>>>> hooks.uri.ftag->body.len)) {
>>>> +        DBG("Route ftag and From tag are same\n");
>>>> +        free_params(params);
>>>> +        return 0;
>>>> +    } else {
>>>> +        DBG("Route ftag and From tag are NOT same\n");
>>>> +        free_params(params);
>>>> +        return 1;
>>>> +    }
>>>> +
>>>> + err:
>>>> +    if (params) free_params(params);
>>>> +    return -1;
>>>> +}
>>>> +
>>>> +static int get_direction(struct sip_msg* msg)
>>>> +{
>>>> +    int ret;
>>>> +    if (parse_orig_ruri(msg) < 0) {
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    if (!msg->parsed_orig_ruri_ok) {
>>>> +        LOG(L_ERR, "Error while parsing original Request-URI\n");
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    ret = check_self(&msg->parsed_orig_ruri.host, +             
>>>> msg->parsed_orig_ruri.port_no ? msg->parsed_orig_ruri.port_no : 
>>>> SIP_PORT, 0);/* match all protos*/
>>>> +    if (ret < 0) return -1;
>>>> +    if (ret > 0) {
>>>> +             /* Route is in ruri */
>>>> +        return check_ftag(msg, &msg->first_line.u.request.uri);
>>>> +    } else {
>>>> +        if (msg->route) {
>>>> +            if (parse_rr(msg->route) < 0) {
>>>> +                LOG(L_ERR, "Error while parsing Route HF\n");
>>>> +                return -1;
>>>> +            }
>>>> +                ret = check_ftag(msg, 
>>>> &((rr_t*)msg->route->parsed)->nameaddr.uri);
>>>> +            if (msg->route->parsed) 
>>>> free_rr((rr_t**)&msg->route->parsed);
>>>> +            return ret;
>>>> +        } else {
>>>> +            DBG("No Route headers found\n");
>>>> +            return -1;
>>>> +        }
>>>> +    }
>>>> +}
>>>> +
>>>> +
>>>> static inline struct hdr_field *valid_to( struct cell *t, 
>>>>                 struct sip_msg *reply)
>>>> {
>>>> @@ -156,9 +249,10 @@
>>>>     static str mycode;
>>>>     str *cr;
>>>>     struct cseq_body *cseq;
>>>> +    int dir;
>>>>
>>>>     cnt=tl=al=0;
>>>> -
>>>> +    dir = -2;
>>>>     /* we don't care about parsing here; either the function
>>>>      * was called from script, in which case the wrapping function
>>>>      * is supposed to parse, or from reply processing in which case
>>>> @@ -218,10 +312,19 @@
>>>>                 }
>>>>                 /* fallback to from-uri if digest unavailable ... */
>>>>             case 'F': /* from-uri */
>>>> -                if (rq->from && (from=get_from(rq))
>>>> -                            && from->uri.len) {
>>>> +
>>>> +                if (swap_dir && dir == -2) dir = get_direction(rq);
>>>> +                if (dir <= 0) {
>>>> +                    if (rq->from && (from=get_from(rq))
>>>> +                        && from->uri.len) {
>>>>                         val_arr[cnt]=&from->uri;
>>>> -                } else val_arr[cnt]=&na;
>>>> +                    } else val_arr[cnt]=&na;
>>>> +                } else {
>>>> +                    if (rq->to && (pto=get_to(rq))
>>>> +                        && pto->uri.len) {
>>>> +                        val_arr[cnt]=&pto->uri;
>>>> +                    } else val_arr[cnt]=&na;
>>>> +                }
>>>>                 ATR(FROMURI);
>>>>                 break;
>>>>             case '0': /* from user */
>>>> @@ -255,10 +358,19 @@
>>>>                 ATR(TOTAG);
>>>>                 break;
>>>>             case 'T': /* to-uri */
>>>> -                if (rq->to && (pto=get_to(rq))
>>>> -                            && pto->uri.len) {
>>>> +
>>>> +                if (swap_dir && dir == -2) dir = get_direction(rq);
>>>> +                if (dir <= 0) {
>>>> +                    if (rq->to && (pto=get_to(rq))
>>>> +                        && pto->uri.len) {
>>>>                         val_arr[cnt]=&pto->uri;
>>>> -                } else val_arr[cnt]=&na;
>>>> +                    } else val_arr[cnt]=&na;
>>>> +                } else {
>>>> +                    if (rq->from && (from=get_from(rq))
>>>> +                        && from->uri.len) {
>>>> +                        val_arr[cnt]=&from->uri;
>>>> +                    } else val_arr[cnt]=&na;
>>>> +                }
>>>>                 ATR(TOURI);
>>>>                 break;
>>>>             case '1': /* to user */ Index: modules/acc/acc_mod.c
>>>> ===================================================================
>>>> RCS file: /cvsroot/ser/sip_router/modules/acc/Attic/acc_mod.c,v
>>>> retrieving revision 1.39.2.3
>>>> diff -u -r1.39.2.3 acc_mod.c
>>>> --- modules/acc/acc_mod.c    20 Sep 2005 16:03:14 -0000    1.39.2.3
>>>> +++ modules/acc/acc_mod.c    25 Dec 2005 17:35:20 -0000
>>>> @@ -115,6 +115,7 @@
>>>> int radius_flag = 0;
>>>> int radius_missed_flag = 0;
>>>> static int service_type = -1;
>>>> +int swap_dir = 0;
>>>> void *rh;
>>>> struct attr attrs[A_MAX];
>>>> struct val vals[V_MAX];
>>>> @@ -206,6 +207,7 @@
>>>>     {"radius_flag",                INT_PARAM, 
>>>> &radius_flag            },
>>>>     {"radius_missed_flag",        INT_PARAM, 
>>>> &radius_missed_flag        },
>>>>     {"service_type",         INT_PARAM, &service_type },
>>>> +    {"swap_direction",             INT_PARAM, &swap_dir},
>>>> #endif
>>>> /* DIAMETER    */
>>>> #ifdef DIAM_ACC
>>>> @@ -414,7 +416,7 @@
>>>>      * don't be worried about parsing outcome -- if it failed,      
>>>> * we will report N/A
>>>>      */
>>>> -    parse_headers(rq, HDR_CALLID| HDR_FROM| HDR_TO, 0 );
>>>> +    parse_headers(rq, HDR_CALLID| HDR_FROM| HDR_TO| HDR_ROUTE, 0 );
>>>>     parse_from_header(rq);
>>>>
>>>>     if (strchr(log_fmt, 'p') || strchr(log_fmt, 'D')) {
>>>> Index: modules/acc/acc_mod.h
>>>> ===================================================================
>>>> RCS file: /cvsroot/ser/sip_router/modules/acc/Attic/acc_mod.h,v
>>>> retrieving revision 1.14
>>>> diff -u -r1.14 acc_mod.h
>>>> --- modules/acc/acc_mod.h    24 Aug 2004 08:58:23 -0000    1.14
>>>> +++ modules/acc/acc_mod.h    25 Dec 2005 17:35:21 -0000
>>>> @@ -87,6 +87,7 @@
>>>> extern char* acc_totag_col;
>>>> extern char* acc_fromtag_col;
>>>>
>>>> +extern int swap_dir;
>>>>
>>>> #endif /* SQL_ACC */
>>>>
>>>> Index: parser/parse_param.c
>>>> ===================================================================
>>>> RCS file: /cvsroot/ser/sip_router/parser/parse_param.c,v
>>>> retrieving revision 1.21
>>>> diff -u -r1.21 parse_param.c
>>>> --- parser/parse_param.c    1 Sep 2004 12:50:40 -0000    1.21
>>>> +++ parser/parse_param.c    25 Dec 2005 17:35:26 -0000
>>>> @@ -144,6 +144,15 @@
>>>>             _h->uri.maddr = _p;
>>>>         }
>>>>         break;
>>>> +
>>>> +    case 'f':
>>>> +    case 'F':
>>>> +        if ((_p->name.len == 4) &&
>>>> +            (!strncasecmp(_p->name.s + 1, "tag", 3))) {
>>>> +            _p->type = P_FTAG;
>>>> +            _h->uri.ftag = _p;
>>>> +        }
>>>> +        break;
>>>>     }
>>>> }
>>>>
>>>> @@ -475,6 +484,7 @@
>>>>     case P_MADDR:     type = "P_MADDR";     break;
>>>>     case P_TTL:       type = "P_TTL";       break;
>>>>     case P_RECEIVED:  type = "P_RECEIVED";  break;
>>>> +    case P_FTAG:      type = "P_FTAG";      break;
>>>>     default:          type = "UNKNOWN";     break;
>>>>     }
>>>>     Index: parser/parse_param.h
>>>> ===================================================================
>>>> RCS file: /cvsroot/ser/sip_router/parser/parse_param.h,v
>>>> retrieving revision 1.11
>>>> diff -u -r1.11 parse_param.h
>>>> --- parser/parse_param.h    1 Sep 2004 11:56:59 -0000    1.11
>>>> +++ parser/parse_param.h    25 Dec 2005 17:35:26 -0000
>>>> @@ -53,6 +53,7 @@
>>>>     P_R2,        /* URI: r2 parameter (ser specific) */
>>>>     P_MADDR,     /* URI: maddr parameter */
>>>>     P_TTL,       /* URI: ttl parameter */
>>>> +    P_FTAG       /* URI: ftag parameter */
>>>> } ptype_t;
>>>>
>>>>
>>>> @@ -98,6 +99,7 @@
>>>>     struct param* r2;        /* r2 parameter */
>>>>     struct param* maddr;     /* maddr parameter */
>>>>     struct param* ttl;       /* ttl parameter */
>>>> +    struct param* ftag;      /* ftag parameter */
>>>> };
>>>>
>>>>
>>>>
>>>>
>>>> ------------------------------------------------------------------------ 
>>>>
>>>>
>>>> _______________________________________________
>>>> Devel mailing list
>>>> Devel at openser.org
>>>> http://openser.org/cgi-bin/mailman/listinfo/devel
>>>
>>>
>>
>> _______________________________________________
>> Devel mailing list
>> Devel at openser.org
>> http://openser.org/cgi-bin/mailman/listinfo/devel
>>
> 
> 




More information about the Devel mailing list