[Home]

Summary:ASTERISK-05182: [patch] The SIP channel prevents placing non-telco phone numbers in the caller-ID number information.
Reporter:Robert Christian (thetatag)Labels:
Date Opened:2005-09-28 20:54:25Date Closed:2008-01-15 15:51:00.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_sip.c.diff
Description:The SIP channel driver (chan_sip.c) contains a line which checks to see if the number being sent to a SIP device is a valid telco phone number.  This prevents sending various sip username/hostname or other textual (yet not necessarily descriptive as in the CID name) to a remote SIP phone.  The irony is that, as the code goes, if you supply a textual caller-ID number, the SIP channel driver rejects it and replaces it with the built-in default "asterisk", which is obviously textual in itself, and although I have only ever tested it on my Grandstream GXP-2000 phones, it seems to display just fine.  From repeated digging in the sources, I do not see any reason to limit the CID-number to telco numbers, and I see this as very limiting to a channel type that is typically NOT utilizing a telco phone number for originating calls.  A cursory examination of the other channel drivers suggests that SIP is the only one that imposes this limitation.

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

I recommend the following patch to chan_sip.c to correct this issue, unless I am overlooking some logical reason why this restriction exists (and yet why it is okay for the textual word "asterisk" to be in the caller-id number).  Note that I also removed the check to see if the default callerid contained anything, as it is initialized in the chan_sip.c file itself and therefore will always contain the word "asterisk".
Proposed patch follows:

--- chan_sip.c.orig      2005-09-28 17:23:39.000000000 -0800
+++ chan_sip.c  2005-09-28 17:23:54.000000000 -0800
@@ -4628,7 +4628,7 @@
               l = p->owner->cid.cid_num;
               n = p->owner->cid.cid_name;
       }
-       if (!l || (!ast_isphonenumber(l) && default_callerid[0]))
+       if (!l)
                       l = default_callerid;
       /* if user want's his callerid restricted */
       if ((p->callingpres & AST_PRES_RESTRICTION) != AST_PRES_ALLOWED) {
Comments:By: Robert Christian (thetatag) 2005-09-28 21:21:46

I realize I should have put [patch] in the description.  I apologize.  This is my first patch submission on this project.
By: Russell Bryant (russell) 2005-09-28 21:36:40

Patches are also generally attached as files in "cvs diff -u" format.
By: Robert Christian (thetatag) 2005-09-28 21:44:34

Thank you for the pointer, drumkilla.  I have corrected that situation as well and uploaded an appropriate patch file.
By: Kevin P. Fleming (kpfleming) 2005-10-04 21:48:05

We restrict the CLID to containing only phone numbers since we are trying to interoperate with the PSTN. Why 'asterisk' is the default I have no idea... I'll try to get that changed (I don't like it either).

It is necessary to check the contents of default_callerid because it can be changed via the config file and it could be made to be empty.
By: Tilghman Lesher (tilghman) 2005-10-04 23:09:37

Shouldn't restricting callerid best be left as a policy decision by the local administrator, rather than forcing numeric CID on everybody?
By: Olle Johansson (oej) 2005-10-05 09:19:58

THis is one of my favourite topics and a reason why we have to rewrite a lot of the caller id num/name functions in 1.3...

ast_shrink... is also a culprit.
By: Olle Johansson (oej) 2005-10-05 09:21:44

I would propose a bad fix for now. Documenting like this in sip.conf

"Asterisk does not currently support all characters in the SIP standard for the username part of the SIP uri. The support is limited to a-z and 0-9. Some characters like "." and "()" will be removed from the URI while producing a caller ID."
By: Robert Christian (thetatag) 2005-10-05 11:57:22

In response to kpfleming's note:

In that case, I can see the advantage of checking to see if there is a default caller id specified.  However, as I read it, IF the local variable "l" is NULL or it doesn't point to a phone number and there is a default defined, then "l" is replaced with the default.

So what happens if "l" is NULL and there is no default defined?  In the case of the original code, "l" will be left NULL, which could cause NULL pointer referencing and possible seg faults.

However, "default_callerid" should never be null, even if it points only to a single-byte NULL string terminator if undefined.  Therefore, it seems appropriate and safer to always use the default when "l" is NULL.
By: Robert Christian (thetatag) 2005-10-05 12:09:02

I have to agree with Corydon76.  One of the attractive aspects of Asterisk (one of many) is its flexibility for operation in a variety of environments.  It seems to me that if the SIP channel is restricting the CID to telco numbers simply for interoperability with the PSTN, it should be the PSTN channel that makes the restriction.  It seems inappropriate to impose the limitations of one channel type onto other channels that do not inherently have those restrictions.

Perhaps chan_zap and any other PSTN channels should check the CID and convert it to a form it can handle.  I don't think this is something the SIP channel should do just in case the call proceeds to a PSTN connection (which it very well may not).
By: Olle Johansson (oej) 2005-10-05 12:54:17

If you open this can of worms, we have to start supporting UTF8 all around asterisk, since the username part of the SIP uri is UTF8. This will require a significant change in chan_sip, which we do not have any chance of doing before 1.2. Let's fix immediate bugs and keep the existing functionality.

I have earlier published a proposal called "alphaextensions" that suggest a way to handle this for 1.3 and forward.
By: Robert Christian (thetatag) 2005-10-05 13:08:43

Prior to such an overhaul, I would still encourage adoption of my proposed patch.  As far as I can tell (and I am using it in a production environment), it doesn't break anything.  It may require the administrator to be more aware of what is going on.  But as I pointed out initially, I don't see the other channel drivers making the same checks.

So, until UTF8 can be fully implemented, I encourage it to be left to the administrator to determine if alpha characters invalidate the CID number.
By: lancey (lancey) 2005-10-07 04:35:46

I believe this bug has some relation with bug ASTERISK-5355405, i will further investigate that.


By: Kevin P. Fleming (kpfleming) 2005-10-13 19:48:21

Committed to CVS HEAD with minor mods, thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:51:00.000-0600

Repository: asterisk
Revision: 6778

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r6778 | kpfleming | 2008-01-15 15:51:00 -0600 (Tue, 15 Jan 2008) | 2 lines

don't force CLID to be a phone-number-looking-thingie (issue ASTERISK-5182)

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

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