Summary:	ASTERISK-13376: [patch] Incoming calls from registrations and peer matching
Reporter:	nick_lewis (nick_lewis)	Labels:
Date Opened:	2009-01-15 05:58:28.000-0600	Date Closed:	2011-06-07 14:08:22
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) chan_sip.c-namematch.patch
Description:	If there are multiple sip trunks with the same ITSP then an incoming call is arbitarily matched to the last peer with the same host IP address. This is not a serious problem because the DID is still correct but it does have many insidious effects due to the incorrect channel name
Comments:	By: nick_lewis (nick_lewis) 2009-01-15 06:41:23.000-0600 Example register=myaccount1@sip.myitsp.com/line1 register=myaccount2@sip.myitsp.com/line2 [line1] type=peer username=myaccount1 host=sip.myitsp.com [line2] type=peer username=myaccount2 host=sip.myitsp.com If sip.myitsp.com directs a call to asterisk with a request line of: INVITE line1@mybindaddr SIP/2.0 then it is matched to the line2 peer whereas it would probably be better matched to the line1 peer By: Olle Johansson (oej) 2009-01-15 07:09:38.000-0600 THis is a well-known issue that has been around for years and there are well documented solutions to this, like I mentioned in another bug report with the To: header. By: nick_lewis (nick_lewis) 2009-01-15 12:20:50.000-0600 The dial plan can compensate for a lot but as far as I am aware it cannot compensate for the channel name. The channel name is assigned by ast_channel_alloc in sip_new. The channel name is set to SIP/<peerusername>-<instance>. If the wrong peer is matched it gets the wrong channel name. The effects of this are subtle but things that rely on channel state manager events such as the call state on the flash operator panel are affected. I am going to upload a patch that resolves this problem in many cases. It will certainly not be to everyone's liking. It uses the approach of ordinary UACs, such as sip phones, of matching the exten in the request URI with the peername. It only does this after IP based authentication. If there is no peername match then it drops back to the initial IP based match so it is fully backward compatible. By: Olle Johansson (oej) 2009-01-15 12:33:14.000-0600 Ok, that was a new angle - that you can't find the proper peer in manager actions. You did not say that originally. I will look at your patch when you have it, but I ask you to understand that we have to be very careful so we don't break the current architecture - like it or not. Again, don't call IP matching "authentication". It's not. It's just IP matching and it's very insecure. By: Leif Madsen (lmadsen) 2009-01-20 12:35:37.000-0600 There is a patch here ready for your review oej. If you can't get to this in a reasonable time, please reassign this to someone else. Thanks! By: Olle Johansson (oej) 2009-01-23 02:26:08.000-0600 This patch is not approved as it breaks a lot of the architecture, not only mixing the extension namespace and the peer namespace. We can't do that.  And we can't change the internal workings and break the current architecture, which a lot of people have configurations that rely on. I do however agree that there is a problem that needs to be solved at some point with the matching of SIP trunks coming in, regardless if we register or not. We need to match on the domain of the sender and implement a new type of object (not a user or a peer) that matches on domains. That way we can match one providers cluster of servers -regardless of IP. If we want to restrict IP access, we can use ACLs. I would happily start coding on that, provided I can find funding somewhere. That has to be seen as a feature request though. The problem with peer matching on registrations is well-known and there are several working workarounds published on the net. I do realize that these doesn't really work well with AMI clients, which puts more pressure on finding a good solution. By: nick_lewis (nick_lewis) 2009-01-23 07:39:38.000-0600 The patch does not break existing usage but augments it so that an asterisk sip trunk can operate as a proper UAC as defined in RFC3261. It only matches the request uri to the peername if all other matches fail so it is fully backward compatible. The OEJ proposal to match on domain does not address this issue. This issue occurs when multiple sip trunks have the same source ip and the same domain. For example an ITSP may provide two sip lines each connected to a different PSTN number. Asterisk is currently unable to distinguish between incoming calls on these lines. Adding matching to the domain in addition to the source ip address would leave the problem unresolved. A simple two line sip phone has no problems with supporting two sip lines from one ITSP and nor should asterisk. This patch fixes the problem. By: Olle Johansson (oej) 2009-01-23 07:41:24.000-0600 You still mix namespaces by assuming that the extension given in the register= is the name of a peer, which is a very bad architecture solution. We have to work on another solution to this issue than this patch. By: Olle Johansson (oej) 2009-01-23 07:43:09.000-0600 Btw, I do have code to separate two registrations in a branch. That code fixes this issue, but funding disappeared so I never got around to fix it properly. So there are better solutions for exactly this problem, matching a call to one peer or one registration. By: nick_lewis (nick_lewis) 2009-01-23 08:36:47.000-0600 Re "You still mix namespaces": You have chosen to interpret the AOR in the request line of an INVITE from a UAC as an "extension" and you have chosen to interpret the Contact in the request line of an INVITE from a UAS also as an "extension". This interpretation receives no support from RFC3261. They may both be request lines in INVITES but the one from a UAS contains different objects to the one from a UAC. The AOR from a UAC and the Contact from a UAS do not need to be interpreted in the same way as each other and can be mapped to different asterisk namespaces without abusing any architectural principles. By: Olle Johansson (oej) 2009-01-23 08:41:58.000-0600 It's the way we do implement it in Asterisk, like it or not. By: Leif Madsen (lmadsen) 2009-01-27 14:30:07.000-0600 Where do we go from here? By: Olle Johansson (oej) 2009-01-28 02:48:14.000-0600 The patch is rejected and I'll look for the code I have somewhere. Discussion on the mailing list now as well. I think we need to consolidate a lot of these bug reports about peer/user matching into one and register= stuff into one. By: nick_lewis (nick_lewis) 2009-01-28 03:02:55.000-0600 The contact in the request line of a trunk INVITE must actually contact the peer. A match needs to be made between the contact info and something in the peer. If the peername cannot be used because the contact is set to callbackextension then the match must be with the callbackextension. All trunk peer definitions will need to contain a callbackextension parameter for invite matching purposes irrespective of whether they use integrated registration or use a registration string in the common section. The existence of a callbackextension parameter cannot imply integrated registration so a register=yes/no parameter is required to set integrated registration. By: Olle Johansson (oej) 2009-01-28 03:18:56.000-0600 Nick, again. This is not a bug, what you're saying is a "must" is actually a feature request, outlining changes that I have documented that we need a long time ago. We have never supported what you say is a must and it requires a lot of work. My proposal was to create a new type, called type=service. That's what you request here. Look at http://www.codename-pineapple.org/newtypes.shtml, which is a few years old. By: nick_lewis (nick_lewis) 2009-01-28 03:37:54.000-0600 Ok - it is a feature request to better comply with RFC3261. (Only if compliance with RFC3261 is claimed is it a bug). I accept that to "do it all" is a lot of work but to amend my patch to match on callbackextension instead of peername is relatively straightforward. I think it would be of benefit but I understand that  you may rather I hold off. I think your proposal is fantastic but it may be possible to do the same thing with [peername] type=peer register=yes By: Olle Johansson (oej) 2009-01-28 03:49:21.000-0600 I don't see where we are not compliant with RFC3261. It's our implementation that is a mess, on the wire we do everything right. We register a contact and route the call based on the INVITE request URI. No problem with that. The whole user/peer/friend implementation was a poor design from start, inherited from chan_iax with some patches that made it worse and this is something I've gradually changed for many years. Now is the time to move forward and make good designs instead of patching and making it even worse. We can't have two parallell architectures. By: nick_lewis (nick_lewis) 2009-01-28 05:30:50.000-0600 I am glad that now is the time to make a good design for sip trunks Re "No problem with that" I do have some niggles (i) In an INVITE from a UAS the request URI represents the sip termination point. Asterisk is currently getting the wrong one. Remedial action outside sip does not mend sip behaviour (ii) In an INVITE from a UAS the request URI does not contain an Address of Record so it should not be used to route a call in the dialplan. Any routing of trunk calls in the dialplan should be based only on the context of the sip termination point. RFC3261 shows the difference between UAC and UAS invites in the examples: INVITE sip:bob@biloxi.com SIP/2.0 (an Address of Record from UAC to UAS) INVITE sip:bob@192.0.2.4 SIP/2.0  (a Contact from UAS to UAC) and perhaps it would have been even clearer as INVITE sip:robertgeldof@biloxi.com SIP/2.0 INVITE sip:bob@192.0.2.4 SIP/2.0 It is possible to regard the way a UAS Registrar translates AOR to Contact as similar to the way the dialplan translates Exten to Dial(res_tech/res_name). The Contact is a more physical resource like res_name whereas the AOR is signalling like Exten Anyway I have wittered on too much. I will await the new design By: nick_lewis (nick_lewis) 2009-01-28 05:40:58.000-0600 Once the system design for trunks is specified I will be happy to assist in its implementation as proper sip trunk functionality supports our objectives By: Russell Bryant (russell) 2009-09-10 22:05:54 I'm closing this out since there does not appear to be anything else to do here.  It is clear that there are architecture issues to be worked on.  However, we should work on those outside of a bug report. Thanks guys.