Steve Murphy [Sat, 5 Apr 2008 01:33:13 +0000 (01:33 +0000)]
Found a little problem with the sip request handling that could lead to a quick crash of asterisk, and a road to a DOS attack if left unfixed.
Attaching to a running asterisk with "telnet hostname 5060", I would input "something", then hit return three times, and asterisk crashes.
I traced it to handle_request_do(), which zeroes out the data (an ast_str ptr) if the string is too short.
Instead of freeing the struct and nulling the pointer, it now just resets it, because this
ast_str is expected by the calling routine to still be there after handle_request_do() returns.
This appears to fix the crash. I assume that it was introduced with ast_str's being adopted. It's a subtle and easy-to-miss sort of problem.
I also found all the places where the req.data is freed, and made sure the ptr is Nulled out as well;
no good leaving bad ptrs laying around-- I didn't need to do this, but it seemed a good thing to do...
add a Zaptel timer check to verify the timer is responding when Zaptel support is compiled into Asterisk and Zaptel drivers are loaded. This will help people not waste their valuable time debugging side effects.
Use a 32k file buffer on recordings, which increases the efficiency of file recording.
(closes issue #11962)
Reported by: garlew
Patches:
recording.patch uploaded by garlew (license 376)
bug-11962.diff uploaded by snuffy (license 35)
Make MISDN generate channel rename events when the name changes.
(closes issue #11142)
Reported by: julianjm
Patches:
chan_misdn_tmpchan_trunk_v1.diff uploaded by julianjm (license 99)
Fix a race condition in the manager. It is possible that a new manager event
could be appended during a brief time when the manager is not waiting for input.
If an event comes during this period, we need to set an indicator that there is an
event pending so that the manager doesn't attempt to wait forever for an event that
already happened.
(closes issue #12354)
Reported by: bamby
Patches:
manager_race_condition.diff uploaded by bamby (license 430)
(comments added by me)
Since the SIP request structure gets reused multiple times with TCP handling we have to clear the debug state or else we will keep spitting out debug even after it has been turned off.
(closes issue #12169)
Reported by: pj
Patches:
12169-debugoff-2.diff uploaded by qwell (license 4)
Tested by: pj
Steve Murphy [Tue, 1 Apr 2008 22:45:10 +0000 (22:45 +0000)]
Bumped across another test set for the new exten pattern matcher, which revealed a problem with the CANMATCH/MATCHMORE modes. Direct matches were getting in the way. Fixed.
I have hopes that the changes made over the last few days will
finalize and solidify this code. While there are bound to be
small tweaks still needed, I feel that the job (at last) is
somewhat completed. Finally, I had a chance to comprehend how
the scoring of extension patterns was done in the previous
version, and I've come very close to using the exact same
criteria in the new pattern matching code. The left-right
sorting is now replicated in the trie structure itself, such
that the first match found will the 'best' match. Compared
the results against 1.4 for several extensions. Replicated
falves11's setup and it works. Used some devious patterns
provided by jsmith, supplemented with a few of my own.
Looks good.
Disable Packet2Packet bridging when we need to feed DTMF frames into the core. Some implementations do not like how we switch between things.
(closes issue #12212)
Reported by: bamby
Jeff Peeler [Tue, 1 Apr 2008 17:53:08 +0000 (17:53 +0000)]
This adds DNS SRV record support to DNS manager. If there is a SRV record for a given domain, the hostname and port listed in the SRV record will be used. If no SRV record exists or a SRV lookup is not attempted, the DNS lookup on the specified domain will be performed as normal. Chan_sip has been modified to take advantage of the new SRV support.
Do not pass audio until the remote side has indicated they are providing early media, or if the channel has been answered.
(closes issue #11823)
Reported by: SDamm
Initialize the __res_state structure used for dns purposes
to all 0's prior to using it. This is due to valgrind's complaints
on issue #12284 as well as an excerpt found in "Description" portion
of the online man page found here:
Ensure that we do not exceed the hold's maximum size with a single frame.
(closes issue #12047)
Reported by: fabianoheringer
Tested by: fabianoheringer
Note: While I did write this patch, I would not have found this if fossil
had not reported and fixed issue #12253. A huge thanks to him for helping
to (indirectly) find the problem here.
Mark Michelson [Fri, 28 Mar 2008 20:03:16 +0000 (20:03 +0000)]
This time the fix is proper for issue 12284. I have tested it thoroughly and found
that valgrind no longer complains and that calls do complete correctly.
The fix is along the same lines as before: Make sure the final null terminator gets copied
into the new sip_request's data pointer. Without it, parse_request will read and potentially
write past the end of the string, causing potential crashes.
(closes issue #12284...for real this time!)
reported by falves11
Jason Parker [Fri, 28 Mar 2008 19:08:35 +0000 (19:08 +0000)]
Replace magic number size from msgArray array with a define.
(same patch as before, I just split this part out)
(close issue #12326)
Reported by: travishein
Patches:
app_voicemail_code_documentation.patch uploaded by travishein (license 385)
Mark Michelson [Fri, 28 Mar 2008 16:36:59 +0000 (16:36 +0000)]
The copy_request function did not take into account the necessary null terminator
for the string to be copied into. This resulted in parse_request reading invalid
memory beyond the end of the string, and in some cases led to crashes. Thanks
to falves11 for providing the valgrind output which led to the closure of this issue.
Mark Michelson [Fri, 28 Mar 2008 00:12:52 +0000 (00:12 +0000)]
Fix a crash that would happen when attempting to unload the app_queue module.
The problem was that when the refcount on the queue hit 0, the destructor was
called, and inside the destructor, another function was called which would increase
the refcount back to 1 again and then decrease it again back to 0 for every member
in the queue. This meant that the destructor was being recursively called, leading
to a double free of the queue. This is now fixed by making sure to unlink the
queue from the queues container prior to the final unref of the queue.
For FreeBSD, at least, the ifa_addr element could be NULL.
(closes issue #12300)
Reported by: festr
Patches:
acl.c.patch uploaded by festr (license 443)
These small documentation updates made in response to a query in
asterisk-users, where a user was using Playback, but needed the
features of Background, and had no idea that Background existed,
or that it might provide the features he needed. I thought the
best way to avert these kinds of queries was to provide "See Also"
references in all three of "Background", "Playback", "WaitExten".
Perhaps a project to do this with all related apps is in order.
These changes will set a channel variable ~~EXTEN~~ just before generating code
for a switch, with the value of ${EXTEN}. The exten is marked as having a switch,
and ever after that, till the end of the exten, we substitute any ${EXTEN}
with ${~~EXTEN~~} instead in application arguments; (and the ${EXTEN: also).
The reason for this, is that because switches are coded using
separate extensions to provide pattern matching, and
jumping to/from these switch extensions messes up the ${EXTEN} value,
which blows the minds of users.
Remove excessive smoother optimization that was causing audio glitches (small "pops")
after (about 200ms later) an "incorrectly" sized frame was received.
While it would be very nice to keep this as optimized as possible, it makes no sense
for the smoother to be dropping random bits of audio like this. Isn't that the
whole point of a smoother?
Add a lock to the vm_state structure and use the lock around mail_open calls
to prevent concurrent access of the same mailstream. This, along with trunk's
ability to configure TCP timeouts for IMAP storage will help to prevent
crashes and hangs when using voicemail with IMAP storage.
Jason Parker [Wed, 26 Mar 2008 19:05:51 +0000 (19:05 +0000)]
Large cleanup of DSP code
Per comments from dimas:
1. The code now generates DTMF_BEGIN frames in addition to DTMF_END ones.
2. "quelching" rewritten - now each detector (MF/DTMF/generic tone) may mark fragment of a frame for suppression (squelching, muting) with a call to mute_fragment. Actual muting happens only once at the very end of ast_dsp_process where all marked fragments are zeroed. This way every detector sees original data in the frame without any piece of a frame being zeroed by a detector which was run before.
3. DTMF detector tries to "mute" one block before and one block after the block where actual tone was detected. Muting of previois block is something new for this patch. Obviously this operation is not always possible - if current frame does not contain data for previous block - it is too late. But at least we make our best.
Muting of next block was already done by the old code but it only affects part of the next block which is in the frame being processed. New code keeps this information in state structures so it will mute proper number of samples in the next frame(s) too.
4. Removed ast_dsp_digitdetect and ast_dsp_getdigits APIs because these are not used.
5. DSP API extended a bit - ast_dsp_was_muted() function added which returns true if DSP code was muting any fragment in the last frame. chan_zap uses this function to decide it needs to turn on confmute on the channel.
This is to replace AST_FRAME_DTMF 'm'/'u' (mute/unmute) functionality.
Add an option (transmit_silence) which transmits silence during both Record() and DTMF generation. The reason this is an option is that in order to transmit silence we have to setup a translation path. This may not be needed/wanted in all cases.
(closes issue #10058)
Reported by: tracinet
due to licensing restrictions, we cannot distribute the source code for iLBC encoding and decoding... so remove it, and add instructions on how the user can obtain it themselves
Mark Michelson [Wed, 26 Mar 2008 00:02:31 +0000 (00:02 +0000)]
This ensures that the manager interface is not enabled by default. Prior to this
change, it was possible to start Asterisk with the manager interface enabled, then
either comment out the enabled option or make manager.conf unopenable and the manager
interface would still be enabled.
Jeff Peeler [Tue, 25 Mar 2008 20:02:57 +0000 (20:02 +0000)]
This one line change makes an if inside a for loop (in realtime_peer) check all the ast_variables the loop was intending to test rather than just the first one.
Tilghman Lesher [Tue, 25 Mar 2008 17:40:28 +0000 (17:40 +0000)]
Update the sample configuration, to use Macro less (since it's now deprecated).
(closes issue #12293)
Reported by: pprindeville
Patches:
bugid-0012293.1.6.patch uploaded by pprindeville (license 347)
When reverting a commit, I accidentally left in this bit which was an experiment
to see what would happen. It passed the compile test, and I didn't notice I had
left this change in too.
Joshua Colp [Tue, 25 Mar 2008 15:18:41 +0000 (15:18 +0000)]
Add a special dialplan variable to chan_sip which will cause an audio file to be played upon completion of an attended transfer.
(closes issue #9239)
Reported by: sunder
Add an option (transmit_silence) which transmits silence during both Record() and DTMF generation. The reason this is an option is that in order to transmit silence we have to setup a translation path. This may not be needed/wanted in all cases.
(closes issue #10058)
Reported by: tracinet
Olle Johansson [Tue, 25 Mar 2008 10:54:07 +0000 (10:54 +0000)]
Use the "Server" header when responding to SIP requests.
(closes issue #12278)
Reported by: rjain
Patches:
chan_sip.c.diff uploaded by rjain (license 226)
Mark Michelson [Mon, 24 Mar 2008 20:14:07 +0000 (20:14 +0000)]
Remove the "Event: registration" header from Asterisk-generated
SIP REGISTER requests. rjain points out that RFC 3265 specifies
that the Event: header is not a valid header for REGISTER requests
and that the "registration" value is not defined at IANA.
This is a revert for revision 108288. The reason is that that revision
was not for an actual bug fix per se, and so it really should not have been in 1.4 in
the first place. Plus, people who compile with DO_CRASH are more likely
to encounter a crash due to this change. While I think the usage of DO_CRASH
in ast_sched_del is a bit absurd, this sort of change is beyond the scope of 1.4
and should be done instead in a developer branch based on trunk
so that all scheduler functions are fixed at once.
I also am reverting the change to trunk and 1.6 since they also suffer from
the DO_CRASH potential.
Joshua Colp [Mon, 24 Mar 2008 15:28:25 +0000 (15:28 +0000)]
Only print out the set_address_from_contact host verbose message if debugging is enabled on the dialog.
(closes issue #12280)
Reported by: rjain
Patches:
chan_sip.c.diff uploaded by rjain (license 226)
Russell Bryant [Thu, 20 Mar 2008 23:14:13 +0000 (23:14 +0000)]
Merged revisions 110395 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r110395 | russell | 2008-03-20 18:13:56 -0500 (Thu, 20 Mar 2008) | 9 lines
Shorten the ast_waitfor() timeout from 500 ms to 50 ms in the autoservice thread.
This really should not make a difference except in very rare cases. That case would
be that all of the channels in autoservice are not generating any frames. In that
case, this change reduces the potential amount of time that a thread waits in
ast_autoservice_stop() for the autoservice thread to wrap back around to the beginning
of its loop.