My 2 cents worth: Also we experienced the problem of SIP-Rpid suddenly not working. (It took some time and some code reading to figure out that the problem was to be found in the auth_db module, even though auth_radius also was updated.) It was Martin Koenig's serdev posting regarding CVS HEAD that pointed me in the right direction.
IMHO, the committed changes done by Bogdan did not only fix Rpid issues, but also introduced new functionality in the auth_radius module (i.e. ability to load SIP-AVPs as part of a radius auth). Before the discussion started on these mailing lists, I was surprised that such functionality was introduced so close to release. I did not really expect such an extensive code upgrade, and admittedly I would prefer not to get surprises like this... ;-) The loading of SIP-Rpid as part of auth and having to use a separate function in avp_radius for loading SIP-AVPs for caller has seemed a bit odd and makes it a bit difficult on the RADIUS side (we use a commerical RADIUS server). So, I agree with Bogdan that introducing SIP-AVP loading in authentication is a clean up that makes sense. However, I believed that the avp_radius module was a way to introduce avp loading in 0.9.0 without changing the code of the auth modules?! Why not address this earlier?
My personal opinion: Despite the latest commits, AVP loading from RADIUS is still in transition from being an add-on (avp_radius module) to being integrated in auth_radius and uri_radius. I would love to get rid of one auth (we have three today for each INVITE).
As long as the backwards SIP-Rpid compatibility is there, I'm fine with the commits done.
So, some comments to how I view the RADIUS-based avpair loading in ser-0.9.0: It looks like the loading of SIP-AVPs in auth_radius load all attributes without the "caller_" prefix, while our scripts use caller_* and callee_* prefixed AVPs. This results in the need for renaming all ser.cfg avp references when moving from avp_load_radius("caller") to loading through radius_proxy_authorize().
Also, it seems that both radius_www_authorize and radius_proxy_authorize will load the same set of SIP-AVPs. Both use SIP-Session as Service-Type and it is thus not possible to avoid returning SIP-AVPs for a REGISTER, but do so for INVITEs. Though not directly a critical problem, our RADIUS servers do complex configuration evaluations and use Session-Type to filter out which evaluations and thus avpairs to return. A REGISTER is a plain authentication + authorization on the SIP service, while an INVITE is a more complex evaluation (that is: if SIP-AVPs are to be returned.) Another question is if it is always so that SIP-AVPs should be returned when Session-Type=SIP-Session. The avp_radius module uses SIP-Caller-AVPs and SIP-Callee-AVPs, thus giving a great deal of flexibility.
Ok, finally, loading SIP-AVPs as part of the auth makes sense, but from a user point of view it would be natural to phase out the avp_radius module when doing so. Now avp_load_radius("callee") is still left (radius_does_uri_exist seems to be the natural replacement). This gives another question: Should Session-Type=Call-check always indicate that SIP-AVPs for callee should be returned, or is there other scenarios? (I don't really know, we haven't, but others may?)
g-)
Bogdan-Andrei Iancu wrote:
Juha Heinanen wrote:
Bogdan-Andrei Iancu writes:
What you miss here - or I haven't made it clear enough - is that the auth* commit doesn't backport any new features, improvements, etc, but backports a *cleanup* of these modules, cleanup which fixes a lot of inconsistencies - which in my opinion goes into fixing area.
as i said, auth commit was not backwards compatible with what we had before the commit. more specifically, before the commit, rpid was returned from radius in SIP-RPID reply attribute to authentication query, whereas it now is returned in SIP-AVP attribute. thus all proxies that use radius for authentication stopped working after the commit.
in order to maintain backwards compatibility at radius level, perhaps you could add a check for existence of SIP-RPID attribute in the reply and if it exists, assign its value to rpid avp?
I have to agree with you are this point. I failed to notice this since it is rather a incompatibility at Radius server level - it's about how exactly the RPID value is sent: old version, as standalone A_SIP_RPID Radius attr, now as A_SIP_AVP Radius attr in "rpid:value" format. As far as I found out, this is the only issue, right? And can be easyly fixed - before loading all general Radius attributes from A_SIP_AVP, also look for A_SIP_RPID for backward compat. (maybe configurable).. Do you find this acceptable from all point of view - compatibility and cvs ruling?
bogdan
Serusers mailing list serusers@lists.iptel.org http://lists.iptel.org/mailman/listinfo/serusers