[Home]

Summary:ASTERISK-13456: [patch] Calls are not matched to correct peer when using callbackextension parameter
Reporter:nick_lewis (nick_lewis)Labels:
Date Opened:2009-01-27 04:03:05.000-0600Date Closed:2012-02-08 15:35:19.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) callbackextension_peer_trunk.diff
( 1) chan_sip.c-callbackmatch.patch
( 2) issue14340_callbackextension_trunk.diff
( 3) patch-match_by_callbackexten.diff
Description:If there are a number of peers with different callbackextension parameters the incoming calls are not matched to the right peers

for example with two peers as follows
[trunk1]
host=sip.myitsp.com
callbackextension=9991
...
[trunk2]
host=sip.myitsp.com
callbackextension=9992
...

incoming calls to 9991 and to 9992 are both matched to the peer for 9992

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

Reviewboard link: https://reviewboard.asterisk.org/r/344/
Comments:By: Leif Madsen (lmadsen) 2009-01-28 13:02:11.000-0600

Is this the same issue as 14250 (essentially?)
By: Olle Johansson (oej) 2009-01-28 13:18:42.000-0600

This is the same issue as reported in the other bug report. It's the same mechanism as a register= line, Nick. Nothing else. We still need new code.
By: Leif Madsen (lmadsen) 2009-01-28 20:00:34.000-0600

Closing as this appears to be a duplicate.
By: nick_lewis (nick_lewis) 2009-08-18 10:15:03

This one is a bit different from ASTERISK-13376 in that ASTERISK-13376 was proposing a match on peername. That proposal was as per RFC3261 but at odds with extensions and the way they are implemented in Asterisk.

This issue relates to the callbackextension specified in the peer config and ensuring that incoming calls with different callback extensions do not match incorrectly. It is not contentious - if a peer is specified with a callback extension then calls that match the peer will be to that extension.
By: nick_lewis (nick_lewis) 2009-08-18 10:17:09

Please find attached a patch that may address this issue
By: Olle Johansson (oej) 2009-09-03 14:53:10

This is not considered a bug, since no registrations currently match correctly, but use default matching on calls from the location server, based on the registration.

If we change this, we need to consider the architecture properly and make sure we don't break anything unexpected and that we get what we want.
By: Olle Johansson (oej) 2009-09-03 14:54:51

Note that SIPS: calls are allowed over UDP.

Also check formatting.
By: Olle Johansson (oej) 2009-09-03 14:59:59

I am bit concerned about hashing on IP. That will only solve a few of all cases. If you register to a provider with separate reg and proxy servers, you won't receive the call back from the IP address you had resolved to when building the hash. Generating just a random hash is a better way, that will work in more cases.

Limiting on IP is what we use ACLs for, we should not mix that in this algorithm.
By: nick_lewis (nick_lewis) 2009-09-04 03:18:42

Re SIPS over UDP, the attached patch made use of the old parse_uri function. I assume that when reworked for the new parse_uri function this will be ok.

Re hashing on IP, this is the current behaviour. The callbackexten check is in addition not instead of the IP check. Since many currently use the matching as a form of security it is important not to relax the criteria for matching. Any such change would need a different peer type.
By: nick_lewis (nick_lewis) 2009-09-04 04:06:02

oops - the matching also needs a step with SIP_INSECURE_PORT for ao2_find in peers_by_callback table.

(i) ao2_find in peers_by_callback table (callback && ip && port)
(+ii) ao2_find in peers_by_callback table with SIP_INSECURE_PORT (callback && ip)
(iii) ao2_find in peers_by_ip table (ip && port)
(iv) ao2_find in peers_by_ip table with SIP_INSECURE_PORT (ip)
By: David Vossel (dvossel) 2009-09-09 16:01:49

I uploaded a patch I made a few days ago.  I've had it up on reviewboard for a bit, but it may have gotten over looked.  I tried to take the general idea Nick_Lewis had and implement it without the use of a separate hash table.  What are your thoughts?
By: Leif Madsen (lmadsen) 2009-09-09 16:17:34

What is the link to reviewboard? (for historical purposes)
By: David Vossel (dvossel) 2009-09-09 16:38:31

https://reviewboard.asterisk.org/r/344/
By: Leif Madsen (lmadsen) 2009-09-30 09:43:02

I'm marking this as Ready for Testing as I don't see what information we're waiting for anymore. Moving out of Feedback.
By: nick_lewis (nick_lewis) 2009-09-30 11:29:31

The proposed patch uses a process of finding a peer with matching ip and then trying to find one with matching both ip and callbackextension. If the tighter match is found it tosses the previously found peer. This is not an efficient approach. There is always a double search whether or not there is a tight peer match.

It is better to go straight to the search for the tighter match and only do the ip match if it fails. In that way if there is a tight match (ip and callbackextension) then the ip search is not required.
By: David Vossel (dvossel) 2009-10-02 14:24:43

There is not always a double search.  If the first peer found by address has a callback that matches the URI, the "tighter" search is not done.  Depending on the configuration, having the "tighter" search before the ip search may be slightly more efficient, but in other config's it may be the other way around.  The only time we are guaranteed a double lookup regardless of which comes first is when no callbackextension is used at all, and that will probably happen more often than not.  I don't believe this is done frequently enough for this to really be an issue... I could be wrong, but isn't this just done once at the beginning of a dialog during authentication?
By: nick_lewis (nick_lewis) 2009-10-05 03:38:00

I agree that it is infrequent and in no sense a big deal either way but there are some other advantages doing the tighter search first:
- no need for a peer2 or unrefing
- less code and simpler

In what config is the efficiency the other way round? In all cases that a callback extension is defined, the best match is one that (i) has both matching ip address and matching callback extension if it exists or (ii) has just an ip match if a tight match does not exist. It is not obvious to me how the most efficient way to find this best match might depend on the config.

Another query: What is the correct behaviour of peer_match_callbackextension_cb when peer2->callback is unused. Should it match any callback?
By: Michael L. Young (elguero) 2010-07-06 19:09:15

Brought patch up to date with trunk in order to test it.

Tested the callbackextension functionality and I can confirm that it is working as it should.
By: David Vossel (dvossel) 2010-07-07 10:49:00

This should have been committed a long time ago.  I've restarted the discussion on reviewboard to hopefully get this moving again.

https://reviewboard.asterisk.org/r/344/
By: bas (bas) 2010-12-07 16:26:55.000-0600

I ported patch to 1.8.0 and tested it.
Patch uploaded, waiting license.
By: bas (bas) 2010-12-09 12:03:57.000-0600

Patch successfully applies to 1.8.1
By: bas (bas) 2011-01-13 09:01:49.000-0600

Patch successfully working with 1.8.1.1

Is any chance to any progress?
By: Niccolò Belli (darkbasic) 2011-04-14 09:30:23

Does this patch fix this issue when using register instead of callbackextension?
By: bas (bas) 2011-04-15 01:35:48

Should, if you specify extension in register string.
By: bas (bas) 2011-04-15 01:36:22

Patch successfully working with 1.8.3.2 :)
By: Niccolò Belli (darkbasic) 2011-04-15 05:25:07

I want calls going to the right context without having to specify an extension.
Example:

[trunk1]
host=sip.myitsp.com
context=trunk1-context
...
[trunk2]
host=sip.myitsp.com
context=trunk2-context

Is it possible?
By: bas (bas) 2011-05-03 04:23:19

darkbasic, and how do you suggest to differentiate them?

p.s. bump, patch works with 1.8.3.3 :)