[Home]

Summary:ASTERISK-00771: [patch] Contact: header problems
Reporter:muckl (muckl)Labels:
Date Opened:2004-01-10 01:16:24.000-0600Date Closed:2011-06-07 14:04:42
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_sip.c.optipoint400.diff
Description:This is now a bug report for all problems with how Asterisk handles contact headers in registration /OEJ
-------
Original description:
Siemens optiPoint 400 thinks it isnt registered, Asterisk thinks it is --> cannot make calls from optipoint



****** ADDITIONAL INFORMATION ******

see:
http://lists.digium.com/pipermail/asterisk-users/2003-May/012998.html

diff attached
Comments:By: zoa (zoa) 2004-01-10 08:23:48.000-0600

see also 0000778

By: jrollyson (jrollyson) 2004-01-11 01:17:50.000-0600

Anybody able to confirm this/test this patch?

By: muckl (muckl) 2004-01-11 14:51:54.000-0600

traces / debug outputs can be found here:

http://bembel.homeip.net/~johannes/asterisk/

"bad" means: CVS snapshot 2004-01-11, 14:20 CST
"good" means: same snapshot + patches 777 and 778

edited on: 01-11-04 14:39

By: willi (willi) 2004-02-03 15:44:04.000-0600

The above patch is ok and assures, that the
original COntact header is given in the
register acknowledge.
Picky phones as the OptiPoint then can
complete their registration.
I and some others use this patch since
May of last year

Willi

By: Brian West (bkw918) 2004-02-03 19:22:54.000-0600

Please attach the good and the bad sip trace to the bug notes.

By: Brian West (bkw918) 2004-02-03 19:22:55.000-0600

Please attach the good and the bad sip trace to the bug notes.

By: lwc (lwc) 2004-02-19 10:45:20.000-0600

This fix works.
The traces at the web lik given above are what i'd expect.
The only reason * ever worked is 'cos most UAs are not picky about the contact: address the Pegistrar returns.
The Optipoint (or other phones with the Mediatrix stack) IS picky, thus a latent bug becomes a pain in the fingers - I have to apply a functionally identical hack every time I get a fresh copy of *.

The current * code is incorrect, so please can we roll this in now?

By: Brian West (bkw918) 2004-02-19 19:00:19.000-0600

The question is what does the RFC say.  If it confirms this then it will go in.  Otherwise it wont.

By: Olle Johansson (oej) 2004-03-21 17:34:47.000-0600

Maybe this fixes bug 0000732 as well.

By: lwc (lwc) 2004-03-21 21:05:40.000-0600

Yup - it will.
Problem seems to be that we are getting the Asterisk's (i.e. my_contact) in the REGISTER 200 OK response rather than the phone's contact as sent in the REGISTER request.

BTW, do folks *really* want me to quote chapter(s) and verse from 3261 on REGISTER and the correct response for contact?
The only problem I can see with this patch is if the Proxy/Registrar insists on a lower expires than the registrant wants; given that my phone can't have a higher registration expires time than 1 hour, I don't think that will occur.
I'm not sure that * deals with the case of "de-registration of all contacts" properly, so until that code's done this is OK, IMHO.

By: Olle Johansson (oej) 2004-03-22 06:14:36.000-0600

Didn't follow you on the expire problem - is that related to this patch or another SIP channel problem? Can't see how this patch is related to expiration.

By: lwc (lwc) 2004-03-22 06:59:47.000-0600

It isn't - it merely copies back the contact: passed by the Registrant in the REGISTER request.
The patch is fine as is.

However, the Registrant may have put an ;expires=3600 parameter onto the end of their contact: header in the REGISTER request.

If they have, *and* If Asterisk decides that it wants only a maximim REGISTER expiry of 1800,
then
- you might argue that the contact: should be changed to replace the passed parameter (with the 3600) with a replacement (with 1800).

This is obscure (only happens with an expires= parameter in the passed contact: header), and unlikely to be a problem (Asterisk doesn't normally care what is the re-register expires value).

By: Olle Johansson (oej) 2004-03-22 07:09:22.000-0600

I think the normal behaviour is to keep everything *within* the < and > and skip the rest. So a ;-parameter within <> should be kept (SNOM bug) but everything outside may be skipped. Can't find a reference to this anywhere in the RFC, please help me find it if you can.

By: stredicke (stredicke) 2004-03-22 11:50:16.000-0600

On a formal side RFC3261 says:

Contact        =  ("Contact" / "m" ) HCOLON
                 ( STAR / (contact-param *(COMMA contact-param)))
contact-param  =  (name-addr / addr-spec) *(SEMI contact-params)
name-addr      =  [ display-name ] LAQUOT addr-spec RAQUOT
addr-spec      =  SIP-URI / SIPS-URI / absoluteURI
SIP-URI          =  "sip:" [ userinfo ] hostport
                   uri-parameters [ headers ]
uri-parameters    =  *( ";" uri-parameter)
uri-parameter     =  transport-param / user-param / method-param
                    / ttl-param / maddr-param / lr-param / other-param

In § 10.2.1: "The Contact header field values of the request typically consist of SIP or SIPS URIs that identify particular SIP endpoints..." See also § 19.1.4 where bindings are explicitly subject to the URI comparision rules.

That means, if the UAC likes, it can even register email addresses.

I think you are making your life too hard. Don't parse the contact when registering, just store whatever you get as a string (between the < > if present). Parse it when you send the request (after you copied it into the request). That's it! It's no rocket science.

By: Olle Johansson (oej) 2004-03-22 12:01:41.000-0600

Christian, thank you for the clarification.

By: twisted (twisted) 2004-04-29 08:47:42

Is this something that is still needed to be open?  Or has everything been taken care of?

By: Olle Johansson (oej) 2004-04-29 11:51:07

Not fixed in CVS, but a trial fix in chan_sip2. No response from reporter on wheather this works or not. Closed while waiting for patches. Will reopen when we have a patch. /O

By: bdegel (bdegel) 2004-07-23 06:59:03

Reminder sent to oej

This bug still exists in Asterisk 0.9.0
I'm running Asterisk with 2 Siemens Optipoint 400 Standard SIP (version 2.3.14) and I still have to add

copy_header(resp, req, "Contact");

after line 2419 for getting the Optipoints registered with Asterisk. This is working perfectly and has no negative effects to other clients we're running (Budgetone, X-Lite, Firefly).
If you need more input on this, please drop me an email.
I really would appreciate to get this fix within the next release so that I don't have to patch each new stable version of Asterisk to get my Optipoints up and running again.


By: bdegel (bdegel) 2004-07-23 07:16:54

Reminder sent to oej

Same procedure with Asterisk-1.0-RC1:
The Optipoint 400 thinks it isn't registered (message "no server") while Asterisk thinks it is.

When adding the copy_header line after line 3072 in chan_sip.c everthing's fine again.

By: Olle Johansson (oej) 2004-07-23 16:36:09

Still a problem.

By: Olle Johansson (oej) 2004-07-23 16:39:49

The conclusion is this:

When client registers, he/she/it sends a contact: header we should save and send back *verbatim* when we invite.

If there's a "<" and ">" only save what's between the brackets.

In chan_sip2, I'm saving this data and show it in an extended version of "sip show peers" called "sip show active", so we can check the Contact header of registered peers. Do you find that useful?

By: bdegel (bdegel) 2004-07-26 11:01:37

Reminder sent to oej

I never played with chan_sip2 before.
Is it possible to use chan_sip2 with a stable version of Asterisk ?
I was trying to compile it following your comments in bug ASTERISK-753 but I wasn't succesful ?

Are there any more infos available on chan_sip2 ?

By: Olle Johansson (oej) 2004-07-26 11:09:19

Yes, chan_sip2 compiles on Asterisk CVS head.

There is *NO* stable version, only CVS head nowadays.

If you have problems, find me on IRC - I'm "oej" in asterisk-dev

By: Mark Spencer (markster) 2004-07-27 00:20:38

So what exactly is the story here?

By: lwc (lwc) 2004-07-28 04:29:31

In RFC 3261, REGISTER is covered in section 10.2, starting at page 56.

Section 10.2.1, 4th paragraph on page 59 is:

  Once a client has established bindings at a registrar, it MAY send
  subsequent registrations containing new bindings or modifications to
  existing bindings as necessary.  The 2xx response to the REGISTER
  request will contain, in a Contact header field, a complete list of
  bindings that have been registered for this address-of-record at this
  registrar.

* is not a forking proxy, so a peer/friend can only have One current
binding.

What is currently being sent (inside the respprep routine) for a
REGISTER response is a contact header with p->our_contact.
That's built from the IP address of *, NOT the IP address of the
phone/UAC making the registration.

Many SIP phones ignore the contact header in the response, assuming that
the Registrar will of course send responses with the phone's contact -
that's what the RFC says. However, some are strict, and ignore a
response with someone else's contact address. This is paranoid, IMHO,
but there are some interesting attacks that could be done by a man in
the middle playing with REGISTER and its response.

Thus it's a GOOD IDEA to send the correct address in a response, as some
systems or phones will reject/ignore a response with someone else's
contact address.

Section 10.3 starting on page 63 covers a Registrar processing a
REGISTER request. It has a number of steps; step 8 on page 65 is:

     8. The registrar returns a 200 (OK) response.  The response MUST
        contain Contact header field values enumerating all current
        bindings.  Each Contact value MUST feature an "expires"
        parameter indicating its expiration interval chosen by the
        registrar.  The response SHOULD include a Date header field.

Again, a current binding is from a SIP "Address of Record" to the
phone's contact address - thus the response should include the phone's
contact address, NOT *'s.

SO... what do we do to fix this fault?

The current quick hack that we apply is to change the add_header(resp,
"Contact", contact) to copy_header(resp, req, "Contact");

This works as long as the phone has set a sane expires parameter into
its REGISTER request.

I propose to replace the current line:

snprintf(contact, sizeof(contact), "%s;expires=%d", p->our_contact, p->expiry);

with the lines between the "change" comments:

respprep fragment:

if (p->expiry) {
/* For registration responses, we also need expiry and
  contact info */
char contact[256];
char tmp[256];

// start change

char * c, * n;
strncpy(tmp,get_header(req, "Contact"), sizeof(tmp) - 1);
c = ditch_braces(tmp); //strip all outside braces
n = strchr(c, ';'); //zap trailing parameters
if (n)
*n = '\0';
snprintf(contact, sizeof(contact), "%s;expires=%d", c, p->expiry);

//end change

snprintf(tmp, sizeof(tmp), "%d", p->expiry);
add_header(resp, "Expires", tmp);
add_header(resp, "Contact", contact);
} else {

---- ----
I've tested this with Opti400 standards and the new phones (thanks to Siemens for the loan), and this works.

Appologies to folk - I'm clueless when it comes to running patch/diff
- I hope we all agree that this is a trivial change.

atb,  Lawrence

By: Olle Johansson (oej) 2004-07-28 04:51:15

The basic rule is to resend everything we get in the REGISTER - the stuff between the < > brackets. In chan_sip2 I save it in a "fullcontact" variable in the peer structure. When calling, I use it in the INVITE. Also, I'm showing the contact header in SIP show peer - very useful for debugging. SNOM phones use an extra variable in the Contact: to indicate line, middle boxes add extra stuff there to signify session, so it's important.

I'll rip out what I have in chan_sip2 and add it as a patch later today or tomorrow.

By: bdegel (bdegel) 2004-07-30 04:05:03

Reminder sent to lwc

I tested the code and it's working perfectly in my environment (Optipoint 400 standard, Budgetone, X-Lite, Firefly) when patching the 1.0-RC1 code.

Hope you put this into the final release.

By: lwc (lwc) 2004-07-30 12:32:14

I stand corrected: one needs the whole triangle-braced contact data so the changed lines should be:
   char * c;
   strncpy(tmp,get_header(req, "Contact"), sizeof(tmp) - 1);
   c = ditch_braces(tmp); //strip all outside braces
   snprintf(contact, sizeof(contact), "%s;expires=%d", c, p->expiry);
(i.e. lose 3 lines of code and the * n declaration).

I've downloaded sip2Ab.c from bug 759, and I don't see how respprep is different from what we have already in chan_sip.c. I must be missing something, so I await the counter-proposal with interest.

By: Olle Johansson (oej) 2004-07-30 15:21:39

Added patch "sipcontact.txt"
This code is taken from chan_sip2.c with small corrections and additions
* Saves the Contact: <text here>
* Uses the text saved on invite
* Show text in "sip show peer <name>"

* Also adds comments as explanations on peer and user structure

Please test if this works with optipoint. I've tested with Snom.

By: Mark Spencer (markster) 2004-07-31 00:18:18

Should be fixed in CVS with an especially simple patch...

By: Mark Spencer (markster) 2004-07-31 16:36:13

Assuming this is fixed.

By: bdegel (bdegel) 2004-08-05 01:21:50

Reminder sent to oej

ok, I've tested the "sipcontact.txt" patch, everything seems to be fine now.

Thanks a lot for your help !

By: Olle Johansson (oej) 2004-08-09 02:51:08

In my setup, Mark's fix did not fix the problem, I'm still getting an URI built by Asterisk, not the URI sent in the Contact: header at registration. Any others that can verify, so I'm not mistaken?

By: Mark Spencer (markster) 2004-08-09 15:50:29

I really just don't think you're on latest CVS, but if you are just find me on IRC and i'll look at it.

By: Brian West (bkw918) 2004-08-11 23:23:52

Please repsond.

By: Olle Johansson (oej) 2004-08-12 04:36:29

I'm using CVS, of course.

By: Mark Spencer (markster) 2004-08-12 11:42:29

Then can you find me on IRC so I can login then?

By: Brian West (bkw918) 2004-08-22 23:13:44

Please find mark and bug him to fix this ASAP :P

bkw

By: Mark Spencer (markster) 2004-08-23 10:04:58

OEJ's issue has nothing to do with what this bug was placed about.

By: Olle Johansson (oej) 2004-08-24 00:44:04

Mark,
If you read the report and the pointer to the mail, you'll see that
* The optipoint requires the contact header as is
* I've merged several bug # concering keeping the Contact: header as is.

So my patch is concerning this bug report!

/O :-)

By: willi (willi) 2004-08-24 06:21:21

Hi Oej Mark,
since a couple of CVS versions I'm now using Asterisk and the
OptiPoint 400 (V 2.3.14) without any of my previous patches.
The last version was compiled 2 days ago and the OptiPoint
receives the correct contact header during registration.
... no cheating or hidden patches, promise!
it simply works.

The same applies for Lawrence who uses a similar setup in UK.

OeJ unles you have another case, this report shall be closed.
For other cases, I would recommend to open a new bug report

Willi

By: willi (willi) 2004-08-24 06:22:13

Hi Oej Mark,
since a couple of CVS versions I'm now using Asterisk and the
OptiPoint 400 (V 2.3.14) without any of my previous patches.
The last version was compiled 2 days ago and the OptiPoint
receives the correct contact header during registration.
... no cheating or hidden patches, promise!
it simply works.

The same applies for Lawrence who uses a similar setup in UK.

OeJ unles you have another case, this report shall be closed.
For other cases, I would recommend to open a new bug report

Willi

By: Olle Johansson (oej) 2004-08-24 07:48:33

Ok, moving back to ASTERISK-726

By: Olle Johansson (oej) 2004-08-24 07:49:29

Think I confused too many by combining similar bug reports into one. Consolidation is a mystery :-)