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(a)lists.iptel.org
http://lists.iptel.org/mailman/listinfo/serusers