Richard Mudgett [Mon, 10 Aug 2015 18:43:19 +0000 (13:43 -0500)]
chan_dahdi.c: Flush the DAHDI write buffer after starting DTMF.
Pressing DTMF digits on a phone to go out on a DAHDI channel can result in
the digit not being recognized or even heard by the peer.
Phone -> Asterisk -> DAHDI/channel
Turns out the DAHDI behavior with DTMF generation (and any other generated
tones) is exposed by the "buffers=" setting in chan_dahdi.conf. When
Asterisk requests to start sending DTMF then DAHDI waits until its write
buffer is empty before generating any samples for the DTMF tones. When
Asterisk subsequently requests DAHDI to stop sending DTMF then DAHDI
immediately stops generating the DTMF samples. As a result, the more
samples there are in the DAHDI write buffer the shorter the time DTMF
actually gets sent on the wire. If there are more samples in the write
buffer than the time DTMF is supposed to be sent then no DTMF gets sent on
the wire. With the "buffers=12,half" setting and each buffer representing
20 ms of samples then the DAHDI write buffer is going to contain around
120 ms of samples. For DTMF to be recognized by the peer the actual sent
DTMF duration needs to be a minimum of 40 ms. Therefore, the intended
duration needs to be a minimum of 160 ms for the peer to receive the
minimum DTMF digit duration to recognize it.
A simple and effective solution to work around the DAHDI behavior is for
Asterisk to flush the DAHDI write buffer when sending DTMF so the full
duration of DTMF is actually sent on the wire. When someone is going to
send DTMF they are not likely to be talking before sending the tones so
the flushed write samples are expected to just contain silence.
* Made dahdi_digit_begin() flush the DAHDI write buffer after requesting
to send a DTMF digit.
Richard Mudgett [Wed, 5 Aug 2015 19:21:50 +0000 (14:21 -0500)]
chan_dahdi.c: Lock private struct for ast_write().
There is a window of opportunity for DTMF to not go out if an audio frame
is in the process of being written to DAHDI while another thread starts
sending DTMF. The thread sending the audio frame could be past the
currently dialing check before being preempted by another thread starting
a DTMF generation request. When the thread sending the audio frame
resumes it will then cause DAHDI to stop the DTMF tone generation. The
result is no DTMF goes out.
* Made dahdi_write() lock the private struct before writing to the DAHDI
file descriptor.
Richard Mudgett [Mon, 10 Aug 2015 23:23:02 +0000 (18:23 -0500)]
res_pjsip.c: Fix crash from corrupt saved SUBSCRIBE message.
If the saved SUBSCRIBE message is not parseable for whatever reason then
Asterisk could crash when libpjsip tries to parse the message and adds an
error message to the parse error list.
* Made ast_sip_create_rdata() initialize the parse error rdata list. The
list is checked after parsing to see that it remains empty for the
function to return successful.
Mark Michelson [Thu, 6 Aug 2015 17:48:07 +0000 (12:48 -0500)]
res_pjsip_pubsub: More accurately persist packet.
The pjsip_rx_data structure has a pkt_info.packet field on it that is
the packet that was read from the transport. For datagram transports,
the packet read from the transport will correspond to the SIP message
that arrived. For streamed transports, however, it is possible to read
multiple SIP messages in one packet.
In a recent case, Asterisk crashed on a system where TCP was being used.
This is because at some point, a read from the TCP socket resulted in a
200 OK response as well as an incoming SUBSCRIBE request being stored in
rdata->pkt_info.packet. When the SUBSCRIBE was processed, the
combination 200 OK and SUBSCRIBE was saved in persistent storage. Later,
a restart of Asterisk resulted in the crash because the persistent
subscription recreation code ended up building the 200 OK response
instead of a SUBSCRIBE request, and we attempted to access
request-specific data.
The fix here is to use the pjsip_msg_print() function in order to
persist SUBSCRIBE requests. This way, rather than using the raw socket
data, we use the parsed SIP message that PJSIP has given us. If we
receive multiple SIP messages from a single read, we will be sure only
to save off the relevant SIP message. There also is a safeguard put in
place to make sure that if we do end up reconstructing a SIP response,
it will not cause a crash.
Joshua Colp [Tue, 4 Aug 2015 21:12:59 +0000 (18:12 -0300)]
res_pjsip: Ensure sanitized XML is NULL terminated.
The ast_sip_sanitize_xml function is used to sanitize
a string for placement into XML. This is done by examining
an input string and then appending values to an output
buffer. The function used by its implementation, strncat,
has specific behavior that was not taken into account.
If the size of the input string exceeded the available
output buffer size it was possible for the sanitization
function to write past the output buffer itself causing
a crash. The crash would either occur because it was
writing into memory it shouldn't be or because the resulting
string was not NULL terminated.
This change keeps count of how much remaining space is
available in the output buffer for text and only allows
strncat to use that amount.
Since this was exposed by the res_pjsip_pidf_digium_body_supplement
module attempting to send a large message the maximum allowed
message size has also been increased in it.
A unit test has also been added which confirms that the
ast_sip_sanitize_xml function is providing NULL terminated
output even when the input length exceeds the output
buffer size.
A multi-asterisk box setup with direct media enabled would occasionally
crash when two re-INVITE collisions on a call leg happen in a row.
The re-INVITE logic only had one timer struct to defer the re-INVITE.
When the second collision happens the timer struct is overwritten and put
into the timer heap again. Resources for the first timer are leaked and
the heap has two positions occupied by the same timer struct. Now the
heap ordering is potentially corrupted, the timer will fire twice, and any
resources allocated for the second timer will be released twice.
* The solution is to put the collided re-INVITE into the delayed requests
queue with all the other delayed requests and cherry pick the next request
that can come off the queue when an event happens.
* Changed to put delayed BYE requests at the head of the delayed queue.
There is no sense in processing delayed UPDATEs and re-INVITEs when a BYE
has been requested.
* Made the start of a BYE request flush the delayed requests queue to
prevent a delayed request from overlapping the BYE transaction. I saw a
few cases where a delayed re-INVITE got started after the BYE transaction
started.
* Changed the delayed_request struct to use an enum instead of a string
for the request method. Cherry picking the queue is easier with an enum
than string comparisons and the compiler can warn if a switch statement
does not cover all defined enum values.
* Improved the debug output to give more information. It helps to know
which channel is involved with an endpoint. Trunks can have many channels
associated with the endpoint at the same time.
Mark Michelson [Fri, 16 Jan 2015 22:12:25 +0000 (22:12 +0000)]
Fix problem where a hung channel could occur on a failed blind transfer.
Different clients react differently to being told that a blind transfer
has failed. Some will simply send a BYE and be done with it. Others will
attempt to reinvite themselves back onto the call.
In the latter case, we were creating a new channel and then leaving it to
sit forever doing nothing. With this code change, that new channel will
not be created and the dialog with the transferring channel will be cleaned
up properly.
pjsip: Add rtp_timeout and rtp_timeout_hold endpoint options.
This change adds support for the 'rtp_timeout' and 'rtp_timeout_hold'
endpoint options. These allow the channel to be hung up if RTP
is not received from the remote endpoint for a specified number of
seconds.
Mark Michelson [Thu, 9 Jul 2015 19:17:53 +0000 (14:17 -0500)]
res_pjsip: Add rtp_keepalive endpoint option.
This adds an "rtp_keepalive" option for PJSIP endpoints. Similar to the
chan_sip option, this specifies an interval, in seconds, at which we
will send RTP comfort noise frames. This can be useful for keeping RTP
sessions alive as well as keeping NAT associations alive during lulls.
chan_pjsip: Don't change formats when frame of unsupported format is received.
Receipt of an RTP packet currently causes the formats on an PJSIP channel to
change to the format of the RTP packet. In some off-nominal cases it's possible
for this to be a format that has not been configured or negotiated. This change
makes it so only formats explicitly configured on the endpoint are allowed.
Richard Mudgett [Wed, 15 Jul 2015 20:40:32 +0000 (15:40 -0500)]
strings.h: Fix issues with escape string functions.
Fixes for issues with the ASTERISK-24934 patch.
* Fixed ast_escape_alloc() and ast_escape_c_alloc() if the s parameter is
an empty string. If it were an empty string the functions returned NULL
as if there were a memory allocation failure. This failure caused the AMI
VarSet event to not get posted if the new value was an empty string.
* Fixed dest buffer overwrite potential in ast_escape() and
ast_escape_c(). If the dest buffer size is smaller than the space needed
by the escaped s parameter string then the dest buffer would be written
beyond the end by the nul string terminator. The num parameter was really
the dest buffer size parameter so I renamed it to size.
* Made nul terminate the dest buffer if the source string parameter s was
an empty string in ast_escape() and ast_escape_c().
* Updated ast_escape() and ast_escape_c() doxygen function description
comments to reflect reality.
* Added some more unit test cases to /main/strings/escape to cover the
empty source string issues.
ASTERISK-25255 #close
Reported by: Richard Mudgett
Richard Mudgett [Tue, 14 Jul 2015 19:36:42 +0000 (14:36 -0500)]
res_parking: Fix crash if ATTENDEDTRANSFER set empty before Park.
setup_park_common_datastore() was assuming that a non-NULL string returned
for the ATTENDEDTRANSFER and BLINDTRANSFER channel variables are not empty
strings. Things got crashy as a result.
* Made setup_park_common_datastore() treat the channel variable values the
same whether they are NULL or empty for ATTENDEDTRANSFER and
BLINDTRANSFER.
ASTERISK-25254 #close
Reported by: Richard Mudgett
bridge_native_rtp.c: Don't start native RTP bridging after attended transfer.
The bridge_native_rtp module adds a frame hook to channels which are in
a native RTP bridge. This frame hook is used to intercept when a hold
or unhold frame traverses the bridge so native RTP can be stopped or
started as appropriate. This is expected but exposes a specific bug
when attended transfers are involved.
Upon completion of an attended transfer an unhold frame is queued up
to take one of the channels involved off hold. After this is done
the channel is moved between bridges.
When the frame hook is involved in this case for the unhold it
releases the channel lock and acquires the bridge lock. This
allows the bridge core to step in and move the channel
(potentially changing the bridging techology) from another thread.
Once completed the bridge lock is released by the bridge core.
The frame hook is then able to acquire the bridge lock and
wrongfully starts native RTP again, despite the channel no longer
being in the bridge or needing to start native RTP. In fact at
this point the frame hook is no longer attached to the channel.
This change makes it so the native RTP bridge data is available to
the frame hook when it is invoked. Whether the frame hook has
been detached or not is stored on the native RTP bridge data and
is checked by the frame hook before starting or stopping native
RTP bridging. If the frame hook has been detached it does nothing.
res_sorcery_memory_cache: Execute stale unit test last.
In Jenkins there is currently a sporadic test failure of a
variable number of sorcery memory cache unit tests. I have not
been able to reproduce this on the build agents themselves or
on my development machine.
My working theory is that the stale unit test is causing a
sorcery instance to persist longer than expected, causing subsequent
tests to fail when setting up and initializing the next
sorcery instance.
To see if this is the case this change moves the stale unit test
to execute last so no subsequent unit tests can have issues
initializing their sorcery instance.
Joshua Colp [Tue, 2 Jun 2015 15:20:00 +0000 (12:20 -0300)]
test_sorcery_memory_cache_thrash: Add unit tests for thrashing the memory cache.
This change adds a CLI command which can perform memory cache thrashing as well
as unit tests which perform thrashing under the following configurations:
1. Low number of unique objects that go stale after 1 second
2. Low number of unique objects that expire after 1 second
3. Low number of unique objects which are constantly updated
4. Large number of unique objects which exceed a defined cache size
5. Large number of unique objects which exceed a defined cache size
that also expire and go stale rapidly
6. Large number of unique objects which expire and go stale rapidly
7. Large number of unique objects
For all of the above there are a large number of threads constantly
attempting to retrieve random objects and each test runs for a few
seconds.
This change implements the expire_on_reload option for memory caches.
If enabled and a reload is performed all objects within the cache
will be expired and the cache emptied.
Joshua Colp [Thu, 4 Jun 2015 10:33:30 +0000 (07:33 -0300)]
res_sorcery_memory_cache: Add test event when a refresh occurs.
This change adds a testsuite event for when a refresh occurs.
This is useful as it provides a guaranteed mechanism of knowing when
it has occurred instead of waiting an arbitrary amount of time.
These allow both examination and manipulation of sorcery memory
caches from external sources.
Cached objects can be explicitly expired from a cache or marked
as stale. If expired they are immediately removed. If marked as
stale they will be background refreshed when next retrieved.
Mark Michelson [Tue, 26 May 2015 18:01:24 +0000 (13:01 -0500)]
res_sorcery_memory_cache: Add support for refreshing stale objects.
This change introduces a check of object_lifetime_stale when retrieving
cached objects. If the amount of time the object has been in the cache
exceeds the lifetime, then a task is scheduled to update the cached
object based on an object retrieved from other sorcery wizards instead.
To prevent the cached object from being retrieved during a refresh,
thread-local storage is used to mark the thread as being a stale object
update. This results in the cache returning no object, leading to
sorcery querying other wizards for the object instead.
A test has been added for stale objects as well. This test ensures that
stale objects are retrieved the same as freshly-cached objects. The test
also ensures that after an object is stale, changes in the backend are
reflected in the cache, to include if the object has been deleted from
the backend.
Joshua Colp [Wed, 20 May 2015 22:35:54 +0000 (19:35 -0300)]
res_sorcery_memory_cache: Add support for object_lifetime_maximum.
This makes the "object_lifetime_maximum" option operational.
On the addition of an object to an empty memory cache a scheduled
task is created which, when invoked, expires objects from the cache
which have exceeded their lifetime. If more objects have been added
the remaining life of the oldest object is used to schedule the
next invocation of the scheduled task.
If the oldest object is removed from the cache before it can be
expired automatically the scheduled task is cancelled, if possible,
and the lifetime of the next oldest is used to schedule the task.
If during these two operations no additional objects exist in the
cache then no task is scheduled.
An additional unit test has been added which verifies this
functionality.
Mark Michelson [Wed, 20 May 2015 20:19:27 +0000 (15:19 -0500)]
res_sorcery_memory_cache: Add support for maximum_objects.
This makes the "maximum_objects" option operational.
A heap has been added alongside the hash table in the cache. When
objects are added to the cache, they are also added to the heap.
Similarly, when objects are removed from the cache, they are removed
from the heap.
The heap's use comes into play when an item is to be added to a "full"
cache. When the cache is full, the oldest item is removed from the
cache, using the heap to determine the oldest item.
A unit test has been added that verifies that the maximum_objects option
works as expected and that the oldest object is removed from the cache
when an object beyond the maximum is added.
This change adds a basic res_sorcery_memory_cache module which implements
configuration option parsing, configuration file parsing for threading,
sorcery interface implementation, and unit tests.
Objects can be added, updated, deleted, and retrieved from the memory
cache. Automatic expiration and stale handling will be added in the
future.
Note that unit tests exist within the module itself in case the
threading done as a result of expiration results in asynchronous
actions (which it likely will). Providing access and a notification
mechanism for an external test module would be complicated and
not worth it.
Jonathan Rose [Thu, 2 Jul 2015 22:03:51 +0000 (17:03 -0500)]
app: Add functions to swap vm function table
This patch adds function-mocking methods for testing voicemail
features in external modules. It is being pulled over from r432556
on SVN because DPMA won't presently compile with TEST_FRAMEWORK
set in Asterisk 13.1 certified.
George Joseph [Wed, 22 Apr 2015 21:22:10 +0000 (15:22 -0600)]
res/res_corosync: Always decline module load, instead of failing
Returns a 'failure' from the module load routine indicates to Asterisk
that it should abort loading completely. This is rarely - in fact,
really, never - a good option. Aborting load of Asterisk from a dynamic
module implies that the core, and the rest of the dynamic modules, don't
matter: we should abandon all processing.
res_corosync is really not that important.
This patch updates the module such that, if it fails to load, it
politely declines (emitting ERROR messages along the way), and allows
Asterisk to continue to function.
Note that this issue was keeping Asterisk unit tests from running on
certain build agents.
Mark Michelson [Mon, 29 Jun 2015 17:45:02 +0000 (12:45 -0500)]
res_sorcery_realtime: Fix leak of sorcery object type.
This prevents a leak of a sorcery object type when realtime sorcery
objects are retrieved by fields or when multiple objects are retrieved.
The extent of this leak is that sorcery object types would be leaked.
These are allocated whenever an object type is registered with sorcery,
meaning that on module shutdown, these objects would be leaked. This
could be problematic if many reloads were performed, but it is not as
severe as if every sorcery object retrieved from realtime were being
leaked.
Mark Michelson [Fri, 26 Jun 2015 21:12:33 +0000 (16:12 -0500)]
res_pjsip_nat: Adjust when contact should be rewritten.
A previous change made the contact only get rewritten if the dialog's
route set was not marked frozen. Unfortunately, while the intent of this
is correct, the dialog's route set actually gets marked as frozen
earlier than expected, especially for UAS dialogs.
Instead, the idea is that the contact needs to not be rewritten if there
is a pre-existing route set on the dialog. This is now accomplished by
checking the dialog's route set list instead of checking if the route
set is frozen.
Doing this causes some broken tests to begin passing again.
res_pjsip_refer will attempt to add Referred-By or Replaces headers to
outbound INVITEs at times. If the INVITE gets challenged for
authentication, then we will resend the INVITE. Prior to this patch, the
Referred-By or Replaces header would be re-added to the outbound INVITE,
resulting in duplicated headers.
Mark Michelson [Tue, 23 Jun 2015 22:43:31 +0000 (17:43 -0500)]
res_pjsip_nat: Rewrite route set when required.
When performing some provider testing, the rewrite_contact option was
interfering with proper construction of a route set when sending an ACK
after receiving a 200 OK response to an INVITE.
The initial INVITE was sent to address sip:foo. The 200 OK had a Contact
header with URI sip:bar. In addition, the 200 OK had Record-Route
headers for sip:baz and sip:foo, in that order. Since the Record-Route
headers had the lr parameter, the result should have been:
* Set R-URI of the ACK to sip:bar.
* Add Route headers for sip:foo and sip:baz, in that order.
However, the rewrite_contact option resulted in our rewriting the
Contact header on the 200 OK to sip:foo. The result was:
* R-URI remained sip:foo.
* We added Route headers for sip:foo and sip:baz, in that order.
The result was that sip:bar was not indicated in the ACK at all, so the
far end never received our ACK. The call eventually dropped.
The intention of rewrite_contact is to rewrite the most immediate
destination of our SIP request to be the same address on which we
received a request or response. In the case of processing a SIP response
with Record-Route headers, this means that instead of rewriting the
Contact header, we should instead rewrite the bottom-most Record-Route
header. In the case of processing a SIP request with Record-Route
headers, this means we rewrite the top-most Record-route header.
Like when we rewrite the Contact header, we also ensure to update
the dialog's route set if it exists.
Joshua Colp [Wed, 17 Jun 2015 10:04:39 +0000 (07:04 -0300)]
res_pjsip_mwi: Set up unsolicited MWI upon registration.
The res_pjsip_mwi previously required a reload to set up the proper
subscriptions to allow unsolicited MWI to work. This change
makes it so the act of registering will also cause this to occur.
This is particularly useful if realtime is involved as no reload
needs to occur within Asterisk to cause the MWI information
to get sent.
Joshua Colp [Wed, 10 Jun 2015 23:28:26 +0000 (20:28 -0300)]
bridge: When performing a blonde transfer update connected line information.
When performing a blonde transfer the code uses the old masquerade
mechanism to move a channel around. As a result of this certain information,
such as connected line, is moved between the channels involved. Upon
completion of the move a frame is queued which is supposed to update the
connected line information on the channel. This does not occur as the
code considers it a redundant update since the masquerade operation
updated the channel (but did not inform it of the new connected line
information). The code also does not queue a connected line update
to be handled by the thread handling the channel. Without this any
other channel that may be loosely involved does not know it is
talking to a different caller.
This change does the following to resolve this:
1. The indicated connected line information is cleared upon
completion of the masquerade operation when doing a blonde transfer.
This prevents the connected line update from being considered
redundant.
2. A connected line update frame is now queued upon the completion
of the masquerade operation so any other channel loosely involved
knows that there is a different caller.
Richard Mudgett [Thu, 11 Jun 2015 19:39:45 +0000 (14:39 -0500)]
app_directory: Fix crash when using the alias option 'a'.
The voicemail.conf mailbox key/value pair is defined as:
<mailbox>=[<password>[,<full-name>[,<email>[,<pager>[,<options>]]]]]
Where all fields in the value including the field values are optional.
Since the parsing code for the mailbox key/value pair is sloppy, this
patch tightens the parsing for the directory information.
* Renamed the 'pos' and 'bufptr' variables to 'name' and 'options'
respectively in search_directory_sub(). Those names make more sense.
* Made sure that search_directory_sub() is dealing with the voicemail.conf
mailbox options field if it even exists when looking for the 'hidefromdir'
and 'alias' options.
* Fix crash if a voicemail.conf mailbox is just
<mailbox>=<password>,<name> when the 'a' option is used. If there were no
fields after the name then the 'options' pointer was not checked for NULL.
* Fix users.conf alias processing if the 'a' option is used. The wrong
variable was used.
Kevin Harwell [Mon, 8 Jun 2015 14:43:53 +0000 (09:43 -0500)]
AMI: Escape string values.
So this issue is a bit complicated. Since it is possible to pass values to AMI
that contain a '\r\n' (or other similar sequences) these values need to be
escaped. One way to solve this is to escape the values and then pass the escaped
values to the AMI variable parameter string building function. However, this
puts the onus on the pre-build function to escape all string values. This
potentially requires a fair amount of changes along with a lot of string
allocations/freeing for all values.
Surely there is a way to push this complexity down a level into the string
building function itself? This of course is possible, but ends up requiring a
way to distinguish between strings that need to be escaped and those that don't.
The best way to handle this is by introducing a new format specifier in the
format string. For instance a %s (no escape) and %S (escape). However, that is
a bit weird and unexpected.
So faced with those possibilities this patch implements a limited version of the
first option. Instead of attempting to escape all string values this patch only
escapes those values that make sense. This approach limits the number of changes
and doesn't suffer from the odd format specifier problem.
Mark Michelson [Wed, 3 Jun 2015 22:41:23 +0000 (17:41 -0500)]
res_pjsip: Prevent access of NULL channels.
It is possible to receive incoming requests or responses after the channel
on an ast_sip_session has been destroyed and NULLed out. Handlers of these
sorts of requests or responses need to be prepared for the possibility
that the channel is NULL or else they could cause a crash.
While several places have been amended to deal with NULL channels, there
were still a couple of places that needed updating.
res_pjsip_dtmf_info.c: When handling incoming INFO requests, we need to
return early if there is no channel on the session.
res_pjsip_session.c: When handling a 302 response, we need to stop the
redirecting attempt if there is no channel on the session.
Richard Mudgett [Tue, 17 Feb 2015 15:34:10 +0000 (15:34 +0000)]
res_pjsip_refer: Fix crash from a REFER and BYE collision.
Analyzing a one-off crash on a busy system showed that processing a REFER
request had a NULL session channel pointer. The only way I can think of
that could cause this is if an outgoing BYE transaction overlapped the
incoming REFER transaction in a collision. Asterisk sends a BYE while the
phone sends a REFER to complete an attended transfer.
* Made check the session channel pointer before processing an incoming
REFER request in res_pjsip_refer.
* Fixed similar crash potential for res_pjsip supplement incoming request
processing for res_pjsip_sdp_rtp INFO, res_pjsip_caller_id INVITE/UPDATE,
res_pjsip_messaging MESSAGE, and res_pjsip_send_to_voicemail REFER
messages.
* Made res_pjsip_messaging respond to a message body too large with a 413
instead of ignoring it.
Kevin Harwell [Mon, 6 Apr 2015 19:23:57 +0000 (19:23 +0000)]
res_pjsip: config option 'timers' can't be set to 'no'
When setting the configuration option 'timers' equal to 'no' the bit flag was
not properly negated. This patch clears all associated flags and only sets the
specified one. pjsip will handle any necessary flag combinations. Also went
ahead and did similar for the '100rel' option.
ASTERISK-24910 #close
Reported by: Ray Crumrine
Review: https://reviewboard.asterisk.org/r/4582/
........
Merged revisions 434131 from http://svn.asterisk.org/svn/asterisk/branches/13
Richard Mudgett [Tue, 26 May 2015 18:56:42 +0000 (13:56 -0500)]
res_pjsip_session: Fix in-dialog authentication.
When the remote peer requires authentication for in-dialog requests then
re-INVITEs to the peer cause the call to be disconnected and other
in-dialog requests to the peer like MESSAGE just don't go through.
* Made session_inv_on_tsx_state_changed() handle in-dialog authentication
for re-INVITEs and other methods. Initial INVITEs cannot be handled here
because the INVITE transaction must be restarted earlier.
* Pulled needed code from res/res_pjsip/pjsip_outbound_auth.c in
preparation for removing the file. The generic outbound authentication
code did not work as well as anticipated.
* Created outbound_invite_auth() to only handle initial outbound INVITEs.
Re-INVITEs cannot be handled here. The re-INVITE transaction is still in
progress and the PJSIP library cannot handle the overlapping INVITE
transactions. Other method types should not be handled here as this code
only works on outgoing calls and we need to handle incoming and outgoing
calls.
ASTERISK-25131 #close
Reported by: Richard Mudgett
Jonathan Rose [Tue, 12 May 2015 22:45:09 +0000 (17:45 -0500)]
app_voicemail: fix moving when old messages full
When completing voicemail playback of a message in the 'INBOX', the
message gets moved to the 'Old' messages folder. Without this patch, if
the 'Old' folder is already at its set limit, then the 'INBOX' message will
simply be deleted. With this patch, the flag to delete the message will be
removed if the save_to_folder function indicates that the message could
not be moved due to a full folder.
ASTERISK-25082 #close
Reported by: Jonathan Rose
Review: https://gerrit.asterisk.org/#/c/448/
Richard Mudgett [Tue, 12 May 2015 22:34:45 +0000 (17:34 -0500)]
chan_dahdi/sig_pri: Fix crash on ISDN call hangup collision.
If an ISDN call is hungup by both sides at the same time a crash could
happen.
* Added missing NULL checks for the owner channel after calling
pri_queue_pvt_cause_data() in two places. Code after those calls need to
check the owner channel pointer for NULL before use because
pri_queue_pvt_cause_data() needs to do deadlock avoidance to lock the
owner and the owner may get hung up.
ASTERISK-21893 #close
Reported by: Alexandr Gordeev
Kevin Harwell [Thu, 16 Apr 2015 15:51:50 +0000 (10:51 -0500)]
bridge.c: NULL app causes crash during attended transfer
Due to a race condition there was a chance that during an attended transfer the
channel's application would return NULL. This, of course, would cause a crash
when attempting to access the memory. This patch retrieves the channel's app
at an earlier time in processing in hopes that the app name is available.
However, if it is not then "unknown" is used instead. Since some string value
is now always present the crash can no longer occur.
Joshua Colp [Wed, 6 May 2015 18:24:29 +0000 (15:24 -0300)]
res_pjsip_exten_state: Fix race condition between sending NOTIFY and termination
The res_pjsip_exten_state module currently has a race condition between
processing the extension state callback from the PBX core and processing
the subscription shutdown callback from res_pjsip_pubsub. There is currently
no synchronization between the two. This can present a problem as while
the SIP subscription will remain valid the tree it points to may not.
This is in particular a problem as a task to send a NOTIFY may get queued
which will try to use the tree that may no longer be valid.
This change does the following to fix this problem:
1. All access to the subscription tree is done within the task that
sends the NOTIFY to ensure that no other thread is modifying or
destroying the tree. This task executes on the serializer for the
subscriptions.
2. A reference to the subscription serializer is kept to ensure it
remains valid for the lifetime of the extension state subscription.
3. The NOTIFY task has been changed so it will no longer attempt
to send a NOTIFY if the subscription has already been terminated.
Joshua Colp [Mon, 19 Jan 2015 13:18:32 +0000 (13:18 +0000)]
res_pjsip / res_pjsip_multihomed: Use the correct transport and addressing information on UAS sessions.
The first thing this patch fixes is UAS dialogs. Previously if a transport was
configured on an endpoint and an inbound session was created there was no guarantee
that requests sent on the dialog would use the correct transport and address
information. This has now been fixed so an explicitly configured transport
is taken into account.
The second thing this patch fixes is res_pjsip_multihomed. The res_pjsip_multihomed
module attempts to determine what transport a message should go out on and what
addressing information should go into the message itself. In a scenario where
multiple transports exist bound to the same IP address but a different port the
code would incorrectly alter the transport and change the message to the wrong
transport. This change makes the res_pjsip_multihomed module smarter so it will
only change the transport and address information in the message when it is
possible and makes sense.
Joshua Colp [Mon, 4 May 2015 17:16:24 +0000 (14:16 -0300)]
stasis: Fix dial masquerade datastore lifetime
A recent change went into Asterisk which added reference counts to the
channels stored in a dial masquerade datastore. Unfortunately this
included a reference to the caller in a dialing operation. While all
of the dialed targets have the datastore removed from them upon dialing
completion this did not occur for the caller, causing it to have a
reference to itself that could go never go away (as it depended on
the destruction of the datastore which only happened when the channel
was destroyed). This resulted in the caller channel remaining on the
system despite it having hung up.
This change does the following to fix this issue:
1. The dial masquerade datastore is now removed from the caller upon
dialing completion, just like the dialed targets.
2. Upon destruction of the caller all the dialed targets are also
removed from the dial masquerade datastore (just in case).
3. The reference to the caller has been removed as it should not be
possible for the datastore to now be valid/useful after the lifetime
of the caller has ended.
Richard Mudgett [Wed, 29 Apr 2015 19:29:10 +0000 (14:29 -0500)]
chan_dahdi: Add the chan_dahdi.conf force_restart_unavailable_chans option.
Some telco switches occasionally ignore ISDN RESTART requests. The fix
for ASTERISK-19608 added an escape clause for B channels in the restarting
state if the telco ignores a RESTART request. If the telco fails to
acknowledge the RESTART then Asterisk will assume the telco acknowledged
the RESTART on the second call attempt requesting the B channel by the
telco. The escape clause is good for dealing with RESTART requests in
general but it does cause the next call for the restarting B channel to be
rejected if the telco insists the call must go on that B channel.
chan_dahdi doesn't really need to issue a RESTART request in response to
receiving a cause 44 (Requested channel not available) code. Sending the
RESTART in such a situation is not required (nor prohibited) by the
standards. I think chan_dahdi does this for historical reasons to deal
with buggy peers to get channels unstuck in a similar fashion as the
chan_dahdi.conf resetinterval option.
* Add the chan_dahdi.conf force_restart_unavailable_chans compatability
option that when disabled will prevent chan_dahdi from trying to RESTART
the channel in response to a cause 44 code.
ASTERISK-25034 #close
Reported by: Richard Mudgett
Mark Michelson [Thu, 30 Apr 2015 20:20:43 +0000 (15:20 -0500)]
Prevent potential crash on blond transfer.
Scenario:
Alice calls Bob. Bob performs a blond transfer to Carol. Carol rejects
the incoming call (or some other immediate circumstance causes Carol not
to answer the call)
What occurs in this case is that when the bridge between Alice and Bob
breaks, Alice is told to masquerade into Bob's channel that had placed
the call to Carol. The actual masquerade goes down without a hitch.
However, a channel fixup callback that attempts to publish dial events
over Stasis has a crash. The reason for this crash is that the datastore
on Bob's channel that placed the outbound call to Carol only had a bare
pointer to Carol's channel. Since Carol rejected the incoming call,
Carol's channel has been hung up and freed, meaning accessing her
channel results in a crash.
The fix here is simple. The dial fixup code has been altered to hold
references to the involved channels and to drop those references when
freeing data.
The Asterisk 13 version of the fix for outbound registration was missing
a key component that set the outbound authenticator's callback that
creates an authenticated request based on an old request. This was
picked up by some outbound registration tests failing in the testsuite.
res_pjsip_outbound_registration: Fix double unref on error return.
When the PJSIP pjsip_regc_send function is invoked and an error
status returned the caller currently decrements the reference count
of the client state that it just incremented, assuming the
registration callback would not have been invoked. In practice
this is not correct. If the failure happens after the transaction
has been set up the callback will still be invoked. This will
cause the reference count to be incorrectly decremented twice, once
by the registration callback and second by the caller of
pjsip_regc_send.
This change makes it so that whether the callback is invoked or
not is known by the caller of pjsip_regc_send. Depending on
this it can know whether it is responsible for decrementing the
reference count of the client state or not.
Mark Michelson [Mon, 27 Apr 2015 21:56:31 +0000 (16:56 -0500)]
res_pjsip_outbound_registration: Don't fail on delayed processing: 13.
This is the Asterisk 13 version of a change to master that allows for
registration responses to be processed successfully potentially after
the original transaction has timed out. The main difference between this
and the master change is that the master version has API changes that
are unacceptable for 13. For 13, this is worked around by adding a new
API call that the outbound registration code uses instead.
The following is the text from the master version of this commit:
Odd behaviors have been observed during outbound registrations. The most
common problem witnessed has been one where a request with
authentication credentials cannot be created after receiving a 401
response. Other behaviors include apparently processing an incorrect SIP
response.
Inspecting the code led to an apparent issue with regards to how we
handle transactions in outbound registration code. When a response to a
REGISTER arrives, we save a pointer to the transaction and then push a
task onto the registration serializer. Between the time that we save the
pointer and push the task, it's possible for the transaction to be
destroyed due to a timeout. It's also possible for the address to be
reused by the transaction layer for a new transaction.
To allow for authentication of a REGISTER request to be authenticated
after the transaction has timed out, we now also hold a reference to the
original REGISTER request instead of the transaction. The function for
creating a request with authentication has been altered to take the
original request instead of the transaction where the original request
was sent.
When problems occur regarding outbound registrations, it currently
is difficult to debug. Most off-nominal paths had warning messages,
but sometimes we want to know what's going on before hitting the
off-nominal path. This patch adds lots of debugging output that
should give a clearer picture of what is happening with regards
to outbound registrations.
Matthew Jordan [Sat, 11 Apr 2015 15:10:34 +0000 (15:10 +0000)]
res/res_pjsip_t38: Add missing initialization of t38faxmaxdatagram
Prior to this patch, the far_max_datagram value on the UDPTL structure would
remain -1 if the remote endpoint fails to provide the SDP media attribute
T38FaxMaxDatagram. This can result in the INVITE request being rejected. With
this patch, we will now properly initialize the value with either the default
value or with the value provided by pjsip.conf's t38_udptl_maxdatagram
parameter.
Mark Michelson [Thu, 23 Apr 2015 17:54:30 +0000 (12:54 -0500)]
res_pjsip_t38: Don't crash on authenticated reinvite after originated T.38 FAX.
When Asterisk originates a channel to an application, the channel is
hung up once the application finishes executing. When the application
in question is SendFax, the Asterisk PJSIP code will attempt to reinvite
the T.38 session to audio after the FAX completes. The hangup of the
channel happens in the midst of this reinvite transaction. In most
circumstances, this works out okay because the BYE is delayed until the
reinvite transaction can complete.
However, if the reinvite that Asterisk sends receives a 401/407
response, then Asterisk's attempt to re-send the reinvite with
authentication will fail. This is because the session supplement in
res_pjsip_t38 makes the assumption that the channel on the session will
always be non-NULL. Since the channel has been hung up, though, the
channel is now NULL. Attempting to operate on the channel causes a
crash.
This patch fixes the issue by ensuring that the channel on the session
is not NULL before attempting to mess with the T.38 framehook.
This patch also contains some corrections for comments that were
incorrect and really confused me when I first started looking at the
code.
Mark Michelson [Wed, 15 Apr 2015 15:38:02 +0000 (10:38 -0500)]
Detect potential forwarding loops based on count.
A potential problem that can arise is the following:
* Bob's phone is programmed to automatically forward to Carol.
* Carol's phone is programmed to automatically forward to Bob.
* Alice calls Bob.
If left unchecked, this results in an endless loops of call forwards
that would eventually result in some sort of fiery crash.
Asterisk's method of solving this issue was to track which interfaces
had been dialed. If a destination were dialed a second time, then
the attempt to call that destination would fail since a loop was
detected.
The problem with this method is that call forwarding has evolved. Some
SIP phones allow for a user to manually forward an incoming call to an
ad-hoc destination. This can mean that:
* There are legitimate use cases where a device may be dialed multiple
times, or
* There can be human error when forwarding calls.
This change removes the old method of detecting forwarding loops in
favor of keeping a count of the number of destinations a channel has
dialed on a particular branch of a call. If the number exceeds the
set number of max forwards, then the call fails. This approach has
the following advantages over the old:
* It is much simpler.
* It can detect loops involving local channels.
* It is user configurable.
The only disadvantage it has is that in the case where there is a
legitimate forwarding loop present, it takes longer to detect it.
However, the forwarding loop is still properly detected and the
call is cleaned up as it should be.
Address review feedback on gerrit.
* Correct "mfgium" to "Digium"
* Decrement max forwards by one in the case where allocation of the
max forwards datastore is required.
* Remove irrelevant code change from pjsip_global_headers.c
Matt Jordan [Mon, 13 Apr 2015 14:54:18 +0000 (09:54 -0500)]
build_tools/make_version: Update version parsing for Git migration
External systems - such as the Asterisk Test Suite - require knowledge of the
upstream branch. Unfortunately, after moving to Git, the Asterisk version
currently consists of only a 'GIT" prefix followed by an object blob,
e.g., GIT-as08d7. This makes it difficult for such systems to know what
features are available in a particular check out of Asterisk.
This patch fixes this by hardcoding the branch in a variable in the
make_version script. Since the mainline branches are not changed often -
typically only once a year - this is a reasonable approach to solving
the problem, and is more reliable than parsing the output of 'git branch
-vv'. Branches that track off of an upstream primary branch will then get the
benefit of knowing which mainline branch they are currently based off
of.
Matt Jordan [Sun, 12 Apr 2015 17:59:22 +0000 (12:59 -0500)]
git migration: Remove support for file versions
Git does not support the ability to replace a token with a version
string during check-in. While it does have support for replacing a
token on clone, this is somewhat sub-optimal: the token is replaced
with the object hash, which is not particularly easy for human
consumption. What's more, in practice, the source file version was often
not terribly useful. Generally, when triaging bugs, the overall version
of Asterisk is far more useful than an individual SVN version of a file.
As a result, this patch removes Asterisk's support for showing source file
versions.
Specifically, it does the following:
* main/asterisk:
- Refactor the file_version structure to reflect that it no longer
tracks a version field.
- Alter the "core show file version" CLI command such that it always
reports the version of Asterisk. The file version is no longer
available.
* main/manager: The Version key now always reports the Asterisk version.
* UPGRADE: Add notes for:
- Modification to the ModuleCheck AMI Action.
- Modification of the "core show file version" CLI command.
Matt Jordan [Sun, 12 Apr 2015 04:22:59 +0000 (23:22 -0500)]
.gitignore: Ignore tarballs (*.gz)
This patch updates the root .gitignore file to ignore files with a .gz
extension. This will cause git to ignore downloaded sound tarballs in
the the sounds/ directory.
res_pjsip_mwi: Send unsolicited MWI NOTIFY on startup and when endpoint registers.
Currently the res_pjsip_mwi module only sends an unsolicited MWI NOTIFY upon
a mailbox state change (such as a new message being left, or one being deleted).
In practice this is not sufficient to keep clients aware of the current MWI status.
This change makes the module send unsolicited MWI NOTIFY on startup so that
clients are guaranteed to have the most up to date MWI information. It also makes
clients receive an unsolicited MWI NOTIFY upon registration so if they are unaware
of the current MWI status they receive it.
Jonathan Rose [Wed, 8 Apr 2015 18:19:26 +0000 (18:19 +0000)]
res_pjsip_t38: Fix FAX failures when using PJSIP with authentication
Without this patch, if a PJSIP endpoint with udptl enabled and authentication
set attempted to use sendFax, the FAX session would fail during setup. This
was because the invite issued in response to being auth challenged would cause
the PJSIP channel performing the FAX to receive a second T38 framehook and
this would cause frames to be consumed in an inappropriate manner.
ASTERISK-24933 #close
Reported by: Jonathan Rose
Review: https://reviewboard.asterisk.org/r/4577/
........
Merged revisions 434425 from http://svn.asterisk.org/svn/asterisk/branches/13
Jonathan Rose [Wed, 8 Apr 2015 17:24:23 +0000 (17:24 +0000)]
Security/tcptls: MitM Attack potential from certificate with NULL byte in CN.
When registering to a SIP server with TLS, Asterisk will accept CA signed
certificates with a common name that was signed for a domain other than the
one requested if it contains a null character in the common name portion of
the cert. This patch fixes that by checking that the common name length
matches the the length of the content we actually read from the common name
segment. Some certificate authorities automatically sign CA requests when
the requesting CN isn't already taken, so an attacker could potentially
register a CN with something like www.google.com\x00www.secretlyevil.net
and have their certificate signed and Asterisk would accept that certificate
as though it had been for www.google.com - this is a security fix and is
noted in AST-2015-003.
ASTERISK-24847 #close
Reported by: Maciej Szmigiero
Patches:
asterisk-null-in-cn.patch submitted by mhej (license 6085)
........
Merged revisions 434337 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 434338 from http://svn.asterisk.org/svn/asterisk/branches/11
........
Merged revisions 434384 from http://svn.asterisk.org/svn/asterisk/branches/13
Outbound SIP MESSAGEs had the potential to be sent out
of order from how they were specified in a set of
dialplan steps.
This change creates a serializer for sending outbound
MESSAGE requests on. This ensures that the MESSAGEs are
sent by Asterisk in the same order that they were sent
from the dialplan.
Mark Michelson [Wed, 1 Apr 2015 20:32:52 +0000 (20:32 +0000)]
core: avoid possible asterisk -r crash from long id
When connecting to the remote console, an id string
is first provided that consts of the hostname, pid,
and version. This is parsed by the remote instance
using a buffer that may be too short, and can allow
a buffer overrun because it is not terminated. This
patch adds termination and a larger buffer.
Review: https://reviewboard.asterisk.org/r/4182/
AFS-254
........
Merged revisions 429223 from http://svn.asterisk.org/svn/asterisk/branches/13
Ashley Sanders [Tue, 31 Mar 2015 22:34:48 +0000 (22:34 +0000)]
stasis: set a channel variable on websocket disconnect error
When an error occurs while writing to a web socket, the web socket is
disconnected and the event is logged. A side-effect of this, however, is that
any application on the other side waiting for a response from Stasis is left
hanging indefinitely (as there is no mechanism presently available for
notifying interested parties about web socket error states in Stasis).
To remedy this scenario, this patch introduces a new channel variable:
STASISSTATUS.
The possible values for STASISSTATUS are:
SUCCESS - The channel has exited Stasis without any failures
FAILED - Something caused Stasis to croak. Some (not all) possible
reasons for this:
- The app registry is not instantiated;
- The app requested is not registered;
- The app requested is not active;
- Stasis couldn't send a start message
ASTERISK-24802
Reported By: Kevin Harwell
Review: https://reviewboard.asterisk.org/r/4519/
........
Merged revisions 433839 from http://svn.asterisk.org/svn/asterisk/branches/13
Mark Michelson [Fri, 27 Mar 2015 20:55:41 +0000 (20:55 +0000)]
Add stateful PJSIP response API call, and use it for out-of-dialog responses.
Asterisk had an issue where retransmissions of MESSAGE requests resulted in
Asterisk processing the retransmission as if it were a new MESSAGE request.
This patch fixes the issue by creating a transaction in PJSIP on the incoming
request. This way, if a retransmission arrives, the PJSIP transaction layer
will resend the response and Asterisk will not ever see the retransmission.
Richard Mudgett [Fri, 20 Mar 2015 19:57:58 +0000 (19:57 +0000)]
Audit ast_pjsip_rdata_get_endpoint() usage for ref leaks.
Valgrind found some memory leaks associated with
ast_pjsip_rdata_get_endpoint(). The leaks would manifest when sending
responses to OPTIONS requests, processing MESSAGE requests, and
res_pjsip supplements implementing the incoming_request callback.
* Fix ast_pjsip_rdata_get_endpoint() endpoint ref leaks in
res/res_pjsip.c:supplement_on_rx_request(),
res/res_pjsip/pjsip_options.c:send_options_response(),
res/res_pjsip_messaging.c:rx_data_to_ast_msg(), and
res/res_pjsip_messaging.c:send_response().
* Eliminated RAII_VAR() use with ast_pjsip_rdata_get_endpoint() in
res/res_pjsip_nat.c:nat_on_rx_message().
* Fixed inconsistent but benign return value in
res/res_pjsip/pjsip_options.c:options_on_rx_request().
Richard Mudgett [Tue, 17 Mar 2015 21:56:23 +0000 (21:56 +0000)]
Audit ast_sockaddr_resolve() usage for memory leaks.
Valgrind found some memory leaks associated with ast_sockaddr_resolve().
Most of the leaks had already been fixed by earlier memory leak hunt
patches. This patch performs an audit of ast_sockaddr_resolve() and found
one more.
* Fix ast_sockaddr_resolve() memory leak in
apps/app_externalivr.c:app_exec().
* Made main/netsock2.c:ast_sockaddr_resolve() always set the addrs
parameter for safety so the pointer will never be uninitialized on return.
The same goes for res/res_pjsip_acl.c:extract_contact_addr().
* Made functions that call ast_sockaddr_resolve() with RAII_VAR()
controlling the addrs variable use ast_free instead of ast_free_ptr to
provide better MALLOC_DEBUG information.
Kevin Harwell [Tue, 17 Mar 2015 18:44:48 +0000 (18:44 +0000)]
res_pjsip: Allow configuration of endpoint identifier query order
Updated some documentation stating that endpoint identifiers registered without
a name are place at the front of the lookup list. Also renamed register method
'ast_sip_register_endpoint_identifier_by_name' to
'ast_sip_register_endpoint_identifier_with_name'
ASTERISK-24840
Reported by: Mark Michelson
........
Merged revisions 433031 from http://svn.asterisk.org/svn/asterisk/branches/13
Kevin Harwell [Tue, 17 Mar 2015 18:43:03 +0000 (18:43 +0000)]
res_pjsip: Allow configuration of endpoint identifier query order
This patch fixes previously reverted code that caused binary incompatibility
problems with some modules. And like the original patch it makes sure that
no matter what order the endpoint identifier modules were loaded, priority is
given based on the ones specified in the new global 'endpoint_identifier_order'
option.
ASTERISK-24840
Reported by: Mark Michelson
Review: https://reviewboard.asterisk.org/r/4489/
........
Merged revisions 433028 from http://svn.asterisk.org/svn/asterisk/branches/13