[Home]

Summary:ASTERISK-13607: Resolve remaining issues left over from 'kill-the-user'
Reporter:Leif Madsen (lmadsen)Labels:
Date Opened:2009-02-19 08:42:59.000-0600Date Closed:2009-03-04 15:09:14.000-0600
Priority:BlockerRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:(From Russell Bryant to the asterisk-dev mailing list)

Greetings,

It's getting to the point that Asterisk 1.6.1 is nearing release.  I'd
like to cover what we have left to address before Asterisk 1.6.1 can be
released.

First, the following page shows what mantis issues are marked as
blocking the release of 1.6.1.  Currently, merging some updates to the
timing API is the only thing on this list.

http://bugs.digium.com/roadmap_page.php

The other big thing on my list is to ensure that we are happy with the
state of the chan_sip user/peer/friend situation.  Oej's kill-the-user
patch went in for this release.  Some issues came up, and a number of
issues have already been fixed.  We need to determine if there is
anything left to do in this area before 1.6.1.

Oej has written a document that describes how user/peer/friend matching
should work here:

http://svn.digium.com/view/asterisk/team/oej/sip-compliance/sipobjects.txt?view=markup

I think that what is written here sounds good to me.  Does anyone else
have any comments?  Also, do we have code to write to get this to be the
reality in the 1.6.1 code base?


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

The mailing list discussion can be followed here:

http://lists.digium.com/pipermail/asterisk-dev/2009-February/036725.html
Comments:By: Leif Madsen (lmadsen) 2009-02-19 09:04:45.000-0600

I've just reviewed the document, and it seems pretty straight forward to me and makes sense. I have nothing additional to add.
By: Michiel van Baak (mvanbaak) 2009-02-21 04:49:34.000-0600

The document is very clear.
Also, the description in sip.conf.sample explains it clearly as well.
By: Russell Bryant (russell) 2009-02-23 14:52:14.000-0600

Some code is in team/group/sip-object-matching

Review is at http://reviewboard.digium.com/r/172/
By: Leif Madsen (lmadsen) 2009-02-26 16:30:00.000-0600

Working on testing this for match order and such, and found an edge case I believe.

If you define a peer and a user using the same [name], you don't get both objects in memory -- you get whichever one is defined closest to the bottom of the sip.conf file.

For example:

[leif]
type=peer
context=incoming
host=dynamic
nat=yes
secret=welcome

[leif]
type=user
secret=welcome
nat=yes


This gives you the 'leif' user when you do 'sip show users', but not when you do 'sip show peers'. Flipping the order causes the opposite effect.
By: Leif Madsen (lmadsen) 2009-02-26 16:56:14.000-0600

OK I have found another matching issue. This time it seems to deal with the way the objects remain in memory after a 'sip reload'.

I have for example the following sip.conf:

[jimmy-friend]                         ; match jimmy first
type=friend
context=incoming
host=dynamic
nat=yes
secret=welcome

[sam-user]                              ; match sam first
type=user
nat=yes
secret=welcome

[sam-peer]                              ; match this IP?
type=peer
host=dynamic
context=incoming
nat=yes
secret=welcome

[ip-catchall]              ; get calls have no matching username
type=peer
host=216.19.xxx.xxx
nat=yes
insecure=invite
secret=welcome
context=incoming


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

I had previously made a call with jimmy-friend. It was the first call I made on this system. The device had registered with Asterisk. All calls are coming from the same IP address, from the same device, regardless of which user (multi-line softphone).

After this registration, the astdb contained a SIP/Registry family with the jimmy-friend key and some information as normal.

Calls then that did not match on a user, would continue to match on this peer object, even though ip-catchall has the host defined statically, and is listed closer to the bottom of the file.

Deleting the SIP/Registry/jimmy-friend key did not change this behaviour. Either did removing it and then reloading chan_sip. I also commented out the jimmy-friend entry.

However, performing a restart of the system allowed the incoming call to match on the ip-catchall user as intended. Upon restart, then jimmy-friend entry continued to be commented out.
By: Leif Madsen (lmadsen) 2009-02-27 15:56:44.000-0600

It seems I'm unable to reproduce the last bit of my note here, but it may have just been a fluke as I didn't realize matching order has nothing to do with order in the file now.
By: Russell Bryant (russell) 2009-02-27 15:58:07.000-0600

The code on reviewboard has been updated to reflect changes made to address the first problem - both type=user and type=peer entries.
By: Digium Subversion (svnbot) 2009-03-04 15:01:06.000-0600

Repository: asterisk
Revision: 180261

U   trunk/channels/chan_sip.c

------------------------------------------------------------------------
r180261 | russell | 2009-03-04 15:01:05 -0600 (Wed, 04 Mar 2009) | 54 lines

Resolve object matching issues related to the removal of the sip_user object.

Previously, chan_sip had both sip_peer and sip_user objects in memory.  A
patch went in to remove sip_user to simplify the code, since everything
could be done with just sip_peer.  This patch resolves some regressions
found that were introduced by those changes.

This code comes from svn/asterisk/team/group/sip-object-matching/.

Here is a list of the changes that have been made:

1) When doing a match by name with the find_peer() function, make it much
  easier to specify which objects should be matched by having a parameter
  that specifies exactly which object types should be considered.  Also,
  update find_by_name() to handle this parameter.  Finally, update all
  code to use the new option values.

2) When looking up an object for an outbound request by name, consider
  peers only.  (create_addr())

3) Only match peers on an incoming registration request.

4) When doing authentication (except for SUBSCRIBE), look up users
  by name, instead of all objects by name.
 
5) When doing authentication (except for SUBSCRIBE), after looking for
  a user by name, look for a peer by IP address, instead of all objects
  by IP address.

6) When handling the SIP qualify CLI command or manager action, look for
  a peer by name, instead of any object by name.

7) When handling the SIP unregister CLI command, look for a peer by name,
  instead of any object by name.

9) In sip_do_debug_peer(), search for a peer by name, instead of any object
  by name.

9) When handling the SIPPEER() dialplan function, search for a peer by name,
  instead of any object by name.

10) In the following session timer related functions, st_get_se(),
   st_get_refresher(), and st_get_mode(), when looking for an object for a
   given sip_pvt using pvt->peername, look for a peer by name, instead of any
   object by name.

11) Fix build_peer() to properly handle the case where separate type=peer and
   type=user entries were specified in sip.conf.

(closes issue ASTERISK-13607)
Reported by: lmadsen

Review: http://reviewboard.digium.com/r/172/

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

http://svn.digium.com/view/asterisk?view=rev&revision=180261
By: Digium Subversion (svnbot) 2009-03-04 15:03:29.000-0600

Repository: asterisk
Revision: 180262

_U  branches/1.6.0/

------------------------------------------------------------------------
r180262 | russell | 2009-03-04 15:03:29 -0600 (Wed, 04 Mar 2009) | 61 lines

Blocked revisions 180261 via svnmerge

........
r180261 | russell | 2009-03-04 15:01:05 -0600 (Wed, 04 Mar 2009) | 54 lines

Resolve object matching issues related to the removal of the sip_user object.

Previously, chan_sip had both sip_peer and sip_user objects in memory.  A
patch went in to remove sip_user to simplify the code, since everything
could be done with just sip_peer.  This patch resolves some regressions
found that were introduced by those changes.

This code comes from svn/asterisk/team/group/sip-object-matching/.

Here is a list of the changes that have been made:

1) When doing a match by name with the find_peer() function, make it much
  easier to specify which objects should be matched by having a parameter
  that specifies exactly which object types should be considered.  Also,
  update find_by_name() to handle this parameter.  Finally, update all
  code to use the new option values.

2) When looking up an object for an outbound request by name, consider
  peers only.  (create_addr())

3) Only match peers on an incoming registration request.

4) When doing authentication (except for SUBSCRIBE), look up users
  by name, instead of all objects by name.
 
5) When doing authentication (except for SUBSCRIBE), after looking for
  a user by name, look for a peer by IP address, instead of all objects
  by IP address.

6) When handling the SIP qualify CLI command or manager action, look for
  a peer by name, instead of any object by name.

7) When handling the SIP unregister CLI command, look for a peer by name,
  instead of any object by name.

9) In sip_do_debug_peer(), search for a peer by name, instead of any object
  by name.

9) When handling the SIPPEER() dialplan function, search for a peer by name,
  instead of any object by name.

10) In the following session timer related functions, st_get_se(),
   st_get_refresher(), and st_get_mode(), when looking for an object for a
   given sip_pvt using pvt->peername, look for a peer by name, instead of any
   object by name.

11) Fix build_peer() to properly handle the case where separate type=peer and
   type=user entries were specified in sip.conf.

(closes issue ASTERISK-13607)
Reported by: lmadsen

Review: http://reviewboard.digium.com/r/172/

........

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

http://svn.digium.com/view/asterisk?view=rev&revision=180262
By: Digium Subversion (svnbot) 2009-03-04 15:09:13.000-0600

Repository: asterisk
Revision: 180263

_U  branches/1.6.1/
U   branches/1.6.1/channels/chan_sip.c

------------------------------------------------------------------------
r180263 | russell | 2009-03-04 15:09:13 -0600 (Wed, 04 Mar 2009) | 62 lines

Merged revisions 180261 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

........
r180261 | russell | 2009-03-04 15:01:05 -0600 (Wed, 04 Mar 2009) | 54 lines

Resolve object matching issues related to the removal of the sip_user object.

Previously, chan_sip had both sip_peer and sip_user objects in memory.  A
patch went in to remove sip_user to simplify the code, since everything
could be done with just sip_peer.  This patch resolves some regressions
found that were introduced by those changes.

This code comes from svn/asterisk/team/group/sip-object-matching/.

Here is a list of the changes that have been made:

1) When doing a match by name with the find_peer() function, make it much
  easier to specify which objects should be matched by having a parameter
  that specifies exactly which object types should be considered.  Also,
  update find_by_name() to handle this parameter.  Finally, update all
  code to use the new option values.

2) When looking up an object for an outbound request by name, consider
  peers only.  (create_addr())

3) Only match peers on an incoming registration request.

4) When doing authentication (except for SUBSCRIBE), look up users
  by name, instead of all objects by name.
 
5) When doing authentication (except for SUBSCRIBE), after looking for
  a user by name, look for a peer by IP address, instead of all objects
  by IP address.

6) When handling the SIP qualify CLI command or manager action, look for
  a peer by name, instead of any object by name.

7) When handling the SIP unregister CLI command, look for a peer by name,
  instead of any object by name.

9) In sip_do_debug_peer(), search for a peer by name, instead of any object
  by name.

9) When handling the SIPPEER() dialplan function, search for a peer by name,
  instead of any object by name.

10) In the following session timer related functions, st_get_se(),
   st_get_refresher(), and st_get_mode(), when looking for an object for a
   given sip_pvt using pvt->peername, look for a peer by name, instead of any
   object by name.

11) Fix build_peer() to properly handle the case where separate type=peer and
   type=user entries were specified in sip.conf.

(closes issue ASTERISK-13607)
Reported by: lmadsen

Review: http://reviewboard.digium.com/r/172/

........

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

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