[Home]

Summary:ASTERISK-05165: [patch] Wrong auth on BYE and no update of noncecount with qop digest auth
Reporter:Tom Neville (tomn)Labels:
Date Opened:2005-09-27 15:48:01Date Closed:2011-06-07 14:03:11
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Interoperability
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_sip-correct.diff
( 1) noncecount.txt
( 2) SIPDEBUG-AuthFail.txt
( 3) SIPDEBUG-Failing.txt
( 4) SIPDEBUG-Working.txt
Description:When calling from *->Metaswitch->PSTN the call comes up normally.  When I try to end the call (before the far end party hangs up) the Metaswitch responds with 401.  (Even though I have valid auth information in the packet.)  The most obvious symptom of this is Metaswitch continually calls back until the end party hangs up.

In looking over the code, NC (nonce count)  is not being updated.  The comment in the code says:

       /* XXX We hard code the nonce-number to 1... What are the odds? Are we seriously going to keep
              track of every nonce we've seen? Also we hard code to "auth"...  XXX */

I'm not sure this is the right way to handle this.  Infact, the Metaswitch will NOT accept the auth if it has already seen the current nonce count.  (Preventing a replay attack.)  By incrementing the Nonce Count by 1 each use.. the Metaswitch will accept the BYE correctly.  (NC was included to prevent replay attacks as NC is included in the MD5 hash.)

I've included a patch aginst CVS HEAD as of around "2005-09-27 18:47:06 UTC".  The MS also requres the use of Authorization and not Proxy-Authorization, so I added another Auth line to the BYE message.  (I don' think that's aginst RFC?)  The second Auth line may not be needed for other users?

I disclaim any and all copyrights to the submitted code.  (Just faxed the form in as well.)


Comments:By: Mark Spencer (markster) 2005-09-28 01:03:07

I think we only should increment the nonce counter *if* the nonce is the same as the last nonce we saw...
By: Olle Johansson (oej) 2005-09-28 15:12:49

I want to see a SIP debug output of this, thank you.
By: Olle Johansson (oej) 2005-09-28 15:13:41

...and patches has to be created with "cvs diff -u" format... Please read the bug guidelines.
By: Tom Neville (tomn) 2005-09-28 16:03:19

Sorry about the -u.. Didn't notice that.  chan_sip-correct.diff is a cvs diff -u.  (I tried to delete chan_sip-new.diff but got permission denied.)  

I've attached two SIP DEBUG outputs. Failing is without the patch, and Working is with the patch.

How about instead of only incrementing on old nonces, we continue to increment on each use - and reset the noncecounter each time we get a new nonce?

I'm not sure I put the compare (from the nonce we have vs the nonce in the new auth request) in the right place.  I put it at line 12968.  I'm not very familiar with the source, but from the looks of things that's the only place it copies the nonce from the auth request.
By: Kevin P. Fleming (kpfleming) 2005-09-28 22:56:28

As far as "Authorization" vs. "Proxy-Authorization" that will depend on whether we received 401 or 407, and should already be handled properly. There is no reason why we should supply both headers in our responses.
By: Mark Spencer (markster) 2005-09-28 23:27:06

oej: can you confirm tha the nonce counter should only increment if the nonce itself is the same as what we've seen before?
By: Olle Johansson (oej) 2005-09-29 01:48:03

The patch is certainly incorrect in the way you send both authorization and proxy-authorization in the same packet for the same realm. Since the INVITE was challenged with WWW-authorization, we should not use Proxy-auth in the bye.

We should also be able to handle the 401 properly in the failing case.
By: Tom Neville (tomn) 2005-09-29 10:15:25

About the Porxy-Auth vs Auth.. Yeah, it's not right to send both.  That was mostly just a quick fix because I thought the Metaswitch was doing something wrong.  I should have taken that out before I submitted the patch and gone further in looking at it.

I just attached a new SIP DEBUG (AuthFail) that shows the MS sending a 401 to my INVITE.  On the BYE however, * sends back "Proxy-Authorization" not "Authorization".

Since this looks like two seperate issues.. Should I re-submit the patch without the "Authroization" line and submit a new bug for the Auth line?  I think figuring out how to fix the Auth problem is something someone else can probably fix faster than I can.  :)
By: Olle Johansson (oej) 2005-09-29 10:30:26

Give me a day to look into this. Thanks.
By: Tom Neville (tomn) 2005-09-29 10:30:27

According to RFC 2617.. nc should increment each time the client sends a request with the current nonce value.  Here's the section that I was reading:

The nc-value is the hexadecimal count of the number of requests (including the current request) that the client has sent with the nonce value in this request.   (From: http://www.faqs.org/rfcs/rfc2617.html)
By: Olle Johansson (oej) 2005-09-30 11:58:33

Yes we should increment if we are re-using the auth. I see that now. But as I said, there are several problems here.
By: Olle Johansson (oej) 2005-10-02 12:41:31

Attached patch tries to address both nonce value (hex code string) and re-using the same auth method. It's not a long-term beautiful solution, but a fix to solve this problem.

I can't test this, since I have no device that actually checks nonce-count on auth.

Disclaimer on file.
By: Olle Johansson (oej) 2005-10-02 12:41:57

tomn: Please test quickly and reply to the issue tracker. Thank you!
By: Olle Johansson (oej) 2005-10-05 14:12:28

I need feedback, please-
By: Kevin P. Fleming (kpfleming) 2005-10-05 17:38:04

oej: This looks fairly safe to apply, and appears to increase compatibility. Is it ready to commit?
By: Tom Neville (tomn) 2005-10-05 18:18:00

Sorry.. last couple of days have been really rough.  I did apply it and it fixes the Proxy-Auth vs Auth issue.  However, nonce count is not incrementing.  I've not had time to look over the patch and see if I can find the problem.. which is what I was trying to get done before reporting back.

I'll take a look at it later tonight and see what I can come up with.
By: Olle Johansson (oej) 2005-10-06 00:50:26

kevin: I think you can safely commit and we'll continue to track the noncecount.
By: Mark Spencer (markster) 2005-10-12 00:40:33

Fixed in CVS head rather differently.
By: Digium Subversion (svnbot) 2008-01-15 15:50:32.000-0600

Repository: asterisk
Revision: 6746

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r6746 | markster | 2008-01-15 15:50:31 -0600 (Tue, 15 Jan 2008) | 2 lines

Fix noncecount update (bug ASTERISK-5165, redone fix)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=6746