[reSIProcate] Re: ServerAuthManager ?
http://track.sipfoundry.org/browse/RSP-29
[Scott] We should be really careful about this. We should only issue a
407 if we haven't already done so for this request. If we don't do this
check we could get into an endless loop with poorly behaved UA's. ie.
UA's that always just send a bad realm, when challenged.
I guess we're damned if we do and damned if we don't - it is quite
possible for a SIP packet to have ProxyAuthorization headers that were
not meant for us, this should not be a reason to reject it. Is there
any other way we can detect a loop? I don't mind coding it if someone
can suggest the preferred approach to this issue.
If there is no definite answer, then maybe we can control this behaviour
with a setter method, e.g.
ServerAuthManager::setRejectUnrecognisedRealms(bool) so that the
application developer/end user can make the final decision?
It occurs to me that the current `requiresChallenge()' method gets
called twice - once when the original request arrives, and a second
time
when the request with ProxyAuthorization arrives. I might re-arrange
things a little to avoid calling requiresChallenge() until the
challenge
is just about to be issued. This will avoid duplicating all the
requests when operating in async mode.
[Scott] Sounds good. - In fact I will fix the double call today - since
repro is using this already.
I've already written this, but the patch is untested and still needs
some work. I've attached it so you can have a look, it will be finished
and tested in another few hours.
It also occured to me that the logic for ACK and CANCEL could
potentially be put into the requiresChallenge method in the base
class.
Would this be neater than the current logic?
[Scott] I don't really like this - it will force an inherited method to
ensure that the base method is called in order to avoid challenging
ACK's and CANCEL's. Not challenging these is part of the RFC behaviour
(section 22.1) - so they are probably best left out of that method.
That did occur to me too, I'm happy to leave it as is but thought it was
worth mentioning.
p.s. the attached patch is untested, do not use :)