Re: [reTurn-devel] [reSIProcate] reTurn authentication, async and threading issues
On 27/01/12 18:05, Scott Godin wrote:
> All your notes on H(A1) seem good to me. : )
>
One more thing on H(A1):
Kamailio and OpenSIPS are calculating two variations of H(A1) to
accommodate different UAs:
ha1 = md5(user:domain:password)
ha1b = md5(user@domain:domain:password)
http://kamailio.org/docs/modules/stable/modules_k/auth_db.html#id2545132
Another interesting point is that they support multiple domains (and
consequently they can dynamically choose the realm="" value in the
challenge). They can choose which realm to challenge with based on the
content of a SIP request (e.g. if they recognise From URI for their own
user)
reSIProcate also has such dynamic realm/challenge logic capabilities (I
believe both of us contributed bits of that):
http://list.resiprocate.org/archive/resiprocate-devel/msg05484.html
http://list.resiprocate.org/archive/resiprocate-devel/msg03405.html
For STUN/TURN, I don't believe the initial STUN request gives reTurn
enough information to dynamically choose a realm for the challenge -
although the RFC is a little ambiguous. It just says that realm can be
in a request or a response: it doesn't say whether a UA should send a
realm before it is challenged, for example, and I wouldn't assume UAs
would do that anyway.
It also doesn't give a specific format for the USERNAME, it just says it
can be 513 bytes, it doesn't endorse or rule out usernames that contain
a domain:
http://tools.ietf.org/html/rfc5389#section-15.3
How do you think it should work in that case? Some solutions:
- run different instances on different IPs or ports for each realm
- challenge with a single realm, but try to auth against all known realms
- challenge with a single realm, expect the user to send a domain in the
user field, extract that domain and use it to find the right realm to
auth against, e.g.
a) challenge realm="myrealm.net"
b) UA responds username="me@xxxxxxxxxxx"
c) reTurn sees myrealm.org in the username, and tries to use that
instead of myrealm.net when calculating/looking up the H(A1):
H(A1) = md5(me@xxxxxxxxxxx:myrealm.org:password)
My feeling is that it should work like Kamailio: expect the UA to send
username only, but be prepared for bad UAs that send
user@realm:realm:password
Have you come across any issues with the realm or the submitted form of
user/user@domain in practice?
>>Extra considerations:
>>- what impact will it have on TCP/TLS? Is there a risk of two threads
>>sending responses on the same socket at the same time and corrupting
> each other? Or is the send() method thread safe?
>>- if several packets arrive from the same user in quick succession, must
> avoid starting simultaneous threads to auth the same user
>
> I'm not big on the idea of spawning a thread for every auth request.
> Repro uses a thread pool to do this work - I sent some pointers to it
> in my last email. I think this is the way to go. ASIO itself also has
> mechanisms for dispatching work to threads that could be used.
>
I agree with the worker pool idea - but when the worker picks up a
request, before it starts to work on the request, it should check:
a) has another worker already retrieved that user's details while this
request was in the queue?
b) is another worker already actively looking up the same user?
> send() is not threadsafe - so you will need to take the auth response
> and queue it back to main ASIO/return thread (started in
> reTurnServer.cxx), via the ioService. This will be a little tricky and
> require some basic reading on how asio service, queues and threads work:
> http://think-async.com/Asio/asio-1.4.8/doc/asio/overview.html
Thanks for the feedback on that - I've done similar things in Java, so I
agree with this approach
Just to combine this with my comments on concurrent lookups for the same
user, I think that the `queue' should have one entry per distinct user
rather than one queue entry per STUN request. If a second STUN request
for the same user comes in while there is already a queued request for
auth data, then the second STUN request will not generate an extra queue
entry - it will just be woken up when the existing queue entry is processed.
>
> Scott
>
> On Thu, Jan 26, 2012 at 3:28 PM, Daniel Pocock <daniel@xxxxxxxxxxxxx
> <mailto:daniel@xxxxxxxxxxxxx>> wrote:
>
>
>
> On 22/01/12 19:30, Scott Godin wrote:
> > ...inline...
> >
> > On Sun, Jan 22, 2012 at 9:47 AM, Daniel Pocock
> <daniel@xxxxxxxxxxxxx <mailto:daniel@xxxxxxxxxxxxx>
> > <mailto:daniel@xxxxxxxxxxxxx <mailto:daniel@xxxxxxxxxxxxx>>> wrote:
> >
> >
> >
> >
> > I'm just looking at how to do authentication with reTurn
> >
> > The current state of things:
> >
> > - users are configured at compile time in ReTurnConfig.cxx
> >
> > - users are validated by RequestHandler.cxx /
> > RequestHandler::handleAuthentication
> > (which calls ReTurnConfig::isUserNameValid)
> >
> > - the authentication is all synchronous (so RequestHandler
> blocks if
> > there is database or network lookup)
> >
> >
> > Yup, that is all correct.
> >
> >
> > In resip/dum, we have async authentication (e.g. using the
> > RADIUSServerAuthManager and RADIUSDigestAuthenticator classes)
> >
> > I don't mind doing the work to integrate some of that into
> reTurn, but
> > I'd appreciate some feedback about the issue first:
> >
> > a) how should the async behavior be implemented?
> >
> > b) is TURN auth similar enough to SIP auth that we can use the
> > ServerAuthManager?
> >
> > c) if the answer to (b) is no, does that mean we need something
> > different for reTurn? Or could ServerAuthManager be further
> generalised
> > to meet the needs of TURN?
> >
> > d) or could the async behavior be achieved indirectly, e.g.
> creating a
> > new thread to handle each incoming STUN message (maybe create
> a thread
> > from UdpServer::onReceiveSuccess) and would this be a better
> strategic
> > solution?
> >
> >
> > [Scott] The authentication process is only similar to DUM's Server
> > Registration in that it needs to asynchronously lookup the password
> > associated to a username. I don't think using the DUM implementation
> > will make much sense. It is pretty tied into the DUM framework, and I
> > don't think the effort to decouple it will pay off. Most of the auth
> > work in reTurn is accomplished in
> > the RequestHandler::handleAuthentication method. When reTurn get's a
> > request with the correct auth headers filled in, it would need
> > to asynchronously check if the username is valid and if so,
> retrieve the
> > password for the username before the Message Integrity can be
> verified.
> > Since checking the message integrity is required on every TURN
> request,
> > we would also want to cache this information, so we aren't required to
> > go a backend DB to authenticate every request. Current API calls
> to do
> > these two things synchronously with hard coded values are:
> > getConfig().isUserNameValid and
> getConfig().getPasswordForUsername. I
> > like the idea of having a pool of worker threads that can lookup
> > authentication information. This concept is used in repro. See
> > WorkerThread and Dispatcher classes. It makes sense to me to move
> these
> > two classes down to rutil, so they can be used by other projects.
> Also,
> > see the repro DigestAuthenticator, and UserAuthGrabber classes for a
> > good reference implementation.
>
> Thanks for these details
>
> I've had a chance to look more closely now, just contemplating how to
> proceed, could you let me know if my assessment below is reasonable?
>
> http://tools.ietf.org/html/rfc5389#section-10.2.2 finishes with this
> comment:
> "Any response generated by the server (excepting the cases described
> above) MUST include the MESSAGE-INTEGRITY attribute, computed using
> the username and password utilized to authenticate the request."
>
> This is quite different to SIP: for SIP, the server (e.g. repro) never
> needs to be in possession of H(A1) or the raw password.
>
> rutil/RADIUSDigestAuthenticator works against the rlm_digest support in
> freeradius:
> http://wiki.freeradius.org/Rlm_digest
> and there is a nice example at the bottom here:
> http://tools.ietf.org/id/draft-sterman-aaa-sip-00.txt
>
> Basically, for SIP digest auth:
> - the SIP server extracts all the digest-related stuff from the message
> - it verifies that the nonce is valid
> - and then it just relays all of those goodies to the RADIUS server
> - the rlm_digest module within the RADIUS server does the actual
> calculation of H(A1), calculates what the Digest-Response should be, and
> then compares it to the Digest-Response from the RADIUS request.
> - the SIP server receives a yes/no answer (not the password or H(A1))
> from freeradius
>
> Obviously, this is quite a good method for security. However, in STUN,
> it means the STUN or TURN server won't have the H(A1) it needs to put a
> MESSAGE-INTEGRITY attribute in the response.
>
> To satisfy the STUN variation of the digest algorithm in RFC 5389, s15.4
> http://tools.ietf.org/html/rfc5389#section-15.4
> requires H(A1), as a bare minimum, to be returned. H(A1) could be
> cached too. It seems much better for reTurn to have the H(A1) and never
> see the real password.
> (in fact, this is not quite true - the whole STUN response could be sent
> on a detour to the RADIUS server, where MESSAGE-INTEGRITY can be
> calculated and sent back to reTurn for preparing the response. Then
> reTurn would never need H(A1) or the password. However, this would
> require creating a module within freeradius that can calculate STUN
> MESSAGE-INTEGRITY)
>
> Therefore, the direction to go forward:
>
> - all code that expects a password should be tweaked to work with H(A1),
> and H(A1) gets calculated once (in the config class for now)
>
> - any auth module (RADIUS or SQL based) needs to return H(A1) to reTurn
> (rather than just a yes/no answer)
>
> - rutil/RADIUSDigestAuthenticator needs to be broken up to separate the
> low-level RADIUS stuff into a superclass, to be used from reTurn, maybe
> we need two classes:
> rutil/RADIUSConnector (for low-level RADIUS code)
> resip/stack/RADIUSDigestAuthenticator (for rlm_digest SIP-specific
> logic)
>
> - reTurn needs to have some async behavior: there are 5 calls to
> getPasswordForUsername(). However, as every request must go through
> handleAuthentication(), that is the only place where it needs to be
> async - the other calls to getPasswordForUsername() should always use
> the H(A1) obtained during handleAuthentication()?
>
> - making handleAuthentication() async:
> 1. create some kind of auth listener class
> 2. the full request/response state is encapsulated in a new auth
> listener instance
> 3. the (RADIUS/SQL/whatever) auth mechanism is instantiated and given
> the listener instance
> 4. the auth mechanism starts a thread to do it's work
> 5. the thread wakes up when it has an answer, and it feeds that answer
> to the listener instance
> 6. the response is created and sent by the listener object running
> within the auth thread
> 7. the listener instance is destroyed
> 8. the thread stops
> 9. where cached data is available, the design could skip thread creation
> and just invoke the listener instance from the main thread
>
> Extra considerations:
> - what impact will it have on TCP/TLS? Is there a risk of two threads
> sending responses on the same socket at the same time and corrupting
> each other? Or is the send() method thread safe?
> - if several packets arrive from the same user in quick succession, must
> avoid starting simultaneous threads to auth the same user
>
> > Note: There is also currently no database use in reTurn - so that
> would
> > need to be added, or left as a TODO for implementer's that
> > require authentication from a database back end.
>
>
> Personally, I'm only likely to need to link with the client libs for
> MySQL or radiusclient-ng - just as I've done with dum
>
>