Summary:	ASTERISK-09766: [patch]: Added control over the spying party and ability to spy on calls both ways, including incoming on queue.
Reporter:	xmarksthespot (xmarksthespot)	Labels:
Date Opened:	2007-06-27 11:19:18	Date Closed:	2008-02-07 15:34:12.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_chanspy
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) app_chanspy.c.rej ( 1) bugfix+newfeature10072patchtotrunkrev102726.diff ( 2) chanspy_patch_to_rev_75078.diff ( 3) FIX_patch10072trunkrev102726.diff ( 4) patch10072trunkrev102726.diff ( 5) to145.diff ( 6) to145v2.diff ( 7) to145v3.diff ( 8) to145v4.diff ( 9) to145v5.diff (10) totrunk_app_chanspy.patch (11) totrunk_revision_75395.diff
Description:	Recap: app_chanspy had a couple of things that made it more or less functional. 1. The spying party could spy on whoever he wanted once he was in chanspy, by pressing the "*" key to change channels at will. If it was the CEO on the phone he could spy on him. Aside from some manager stuff, or by using a clever extension numbering scheme (i.e. all spyable extensions begin by the number two), it was impossible to control the spy. This is undeniably a problem. This is rectified by this patch. 2. Spying could not be made to happen on incoming calls! It would be impossible to monitor calls incoming on a queue, so whispering to your employees was not possible, aside from some manager hacking. This was also undeniably a problem. This is also rectified by this patch. 3. If you somehow managed to hack your way into the possibility of spying on incoming calls, you could not be sure to who you attached! For all you know you could have been whispering to the client! This is a huge problem. This is also fixed by this patch. So this is a sort of 3 for 1 deal. ****** STEPS TO REPRODUCE ****** Steps to Reproduce: How this patch works, is simply by adding an 'e' for 'enforced' option to app_chanspy, so that app_chanspy can be called as such: exten => 1234,1,Chanspy(SIP|bqwe(ext1:ext2:ext3:...)) This is a list of whatever the end of the channel name may be when you spy/whisper. So specifying e(101:102:203) would restrain to channels whose name after the SIP part is that. So you could spy on channels named sip/ext1, sip/ext2, etc. And you can ONLY spy on those channels, no more wandering into the bosses' conversations. This allows to fix all three previous issues. Thanks for your consideration of this patch. ****** ADDITIONAL INFORMATION ****** If it is lamely coded or otherwise, please be gentle and I will make the required changes. I have only had time to make it to 1.4.5 yet, I'll make to trunk later.
Comments:	By: xmarksthespot (xmarksthespot) 2007-06-27 15:15:11 Added a new version of the thingy after Corydon76's suggestions. By: xmarksthespot (xmarksthespot) 2007-06-28 13:15:48 Added to145v3.diff, fixes the test of the "core show application chanspy" which was offset wrong in to145v2.diff By: xmarksthespot (xmarksthespot) 2007-06-29 09:57:59 Tried to upload a to145v4.diff, which is the same as to145v3.diff, becuase v3 seemed to have been twilightzoned. And now v4 is twilightzoned too! By: xmarksthespot (xmarksthespot) 2007-06-29 09:59:36 to145v3.diff == to145v4.diff == to145v5.diff I did that because they all just constantly disappear! By: Russell Bryant (russell) 2007-07-01 15:06:21 All of the file uploads are still there.  Nobody can download them because the license has not yet been accepted.  I can see that they are there because I am an admin.  However, you should be able to see them, as well, since you uploaded them.  I'll have bbryant look at it this week. By: xmarksthespot (xmarksthespot) 2007-07-12 10:24:24 Finally added a patch to trunk! Whew. By: xmarksthespot (xmarksthespot) 2007-07-13 15:15:52 Putnopvut committed a small bugfix to app_chanspy which broke the patch to revisions of app_chanspy.c greater than to or equal to 75078. chanspy_patch_to_rev_75078.diff takes care of that. By: Michiel van Baak (mvanbaak) 2007-07-17 02:49:14 xmarksthespot: sorry I ran away from irc yesterday. The patch is not applying to a current trunk checkout. As soon as I'm home after work I'll contact you on irc or put the chan_spy.c.rej file to this ticket. (if I feel good enough I'll merge it by hand and upload new patch) By: Michiel van Baak (mvanbaak) 2007-07-17 15:00:27 this new patch applies fine. Tested with OpenBSD as asterisk server and xlite softphones and Debian etch as asterisk server and sjphone and ekiga softphones. works great. By: Mark Michelson (mmichelson) 2008-02-01 16:46:18.000-0600 I noticed this issue was in need of some attention. I've looked over the description as well as the code, and I see the benefits of the patch. That being said, there are a couple of problems. 1. ext = alloca(strlen(peer->name)); is unnecessary since you later use ast_strdupa to allocate space for ext. You can just remove this line. 2. The e option for chanspy takes a list of numbers separated by :. Since you pull the number out of the channel name of the peer channel, this can lead to ambiguity if more than one channel technology is being used on the system. For instance, if SIP and Skinny channels are used and SIP/1000 is a different entity from Skinny/1000, both channels are in use, and "1000" is specified in the list of extensions for the e option's arguments, then you will be able to spy on both of the channels. I think the option should be changed to take an interface instead of an extension, like e(SIP/5000:Sip/2000:Skinny/2000:Zap/1). Best of all, this will actually make the parsing easier. 3. If you change 2 like I stated, then you should also change strstr to strcasestr in case people use funky capitalization for channel technologies (like I did with the two SIP interfaces above). By: xmarksthespot (xmarksthespot) 2008-02-06 09:53:22.000-0600 Late but not forgotten! I'll pay some attention to it to correct those various aspects you commented on. I'll do it as soon as humanly possible, but give me a few days to have some time to spare, I need to clear up some work first. By: xmarksthespot (xmarksthespot) 2008-02-06 14:42:32.000-0600 This is a newer, future proof, version of the patch. I haven't tested it yet, use at your own risk. I will submit test results, and/or necessary corrections shortly. Thanks. By: xmarksthespot (xmarksthespot) 2008-02-07 11:42:06.000-0600 Whoa, I messed up real bad with the last patch, here's a working fix as far as I could test. I did realize that calling Chanspy(,qwbe(my:inter:faces:listed:here:as:such) like so (without chanspec), wouldn't attach to any channel at all. It's as if the chanspec supersedes everything, and it compares to a "\0" or something at first, and can't find any channel. For it to work properly, I had to call chanspy like so: Chanspy(SIP,qwbe(SIP/my:SIP/inter:SIP/faces:SIP/listed:SIP/like:SIP/so). By: xmarksthespot (xmarksthespot) 2008-02-07 14:01:16.000-0600 All right, so I added a new patch. This new patch: 1) Adds the e() option to chanspy, for the enforced mode. 2) Is a bugfix for the issue where chanspec does not work, and specifying a chanspec is the only way to actually be able to spy on channels. Now it seems to work ok, with or without the e() option (it's unrelated, I noticed it while testing). Seems ok to me now. Questions? Comments? Threats? Go nuts. By: Digium Subversion (svnbot) 2008-02-07 15:34:12.000-0600 Repository: asterisk Revision: 102933 U   trunk/apps/app_chanspy.c ------------------------------------------------------------------------ r102933 | mmichelson | 2008-02-07 15:34:11 -0600 (Thu, 07 Feb 2008) | 18 lines This is a combination new feature/bug fix for app_chanspy. New feature: Add the 'e' option, which takes as an argument a list of interfaces separated by colons. This way, you will only be able to spy on this limited list of interfaces. Bug fix: change some pointer checks to ast_strlen_zero so that spying would work properly even if no channel was specified as the first argument to chanspy. (closes issue ASTERISK-9766) Reported by: xmarksthespot Patches:      bugfix+newfeature10072patchtotrunkrev102726.diff uploaded by xmarksthespot (license 16)  Tested by: xmarksthespot, mvanbaak ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=102933