George Joseph [Thu, 24 Oct 2019 17:41:23 +0000 (11:41 -0600)]
manager.c: Prevent the Originate action from running the Originate app
If an AMI user without the "system" authorization calls the
Originate AMI command with the Originate application,
the second Originate could run the "System" command.
Ben Ford [Mon, 21 Oct 2019 19:55:06 +0000 (14:55 -0500)]
chan_sip.c: Prevent address change on unauthenticated SIP request.
If the name of a peer is known and a SIP request is sent using that
peer's name, the address of the peer will change even if the request
fails the authentication challenge. This means that an endpoint can
be altered and even rendered unusuable, even if it was in a working
state previously. This can only occur when the nat option is set to the
default, or auto_force_rport.
This change checks the result of authentication first to ensure it is
successful before setting the address and the nat option.
Ben Ford [Fri, 8 Nov 2019 19:21:15 +0000 (13:21 -0600)]
res_pjsip_session.c: Check for port of zero on incoming SDP.
If a re-invite comes in initiating T.38, but there is no c line in the
SDP and the port is also 0, a crash can occur. A check is now done on
the port to see if the steam is already declined, preventing the crash.
The logic was moved to res_pjsip_session.c because it is handled in a
similar manner in later versions of Asterisk.
George Joseph [Wed, 9 Oct 2019 14:32:45 +0000 (08:32 -0600)]
pjproject_bundled: Replace earlier reverts with official fixes.
Issues in pjproject 2.9 caused us to revert some of their changes
as a work around. This introduced another issue where pjproject
wouldn't build with older gcc versions such as that found on
CentOS 6. This commit replaces the reverts with the official
fixes for the original issues and allows pjproject to be built
on CentOS 6 again.
Kevin Harwell [Thu, 10 Oct 2019 20:30:06 +0000 (15:30 -0500)]
res_pjsip_mwi: potential double unref, and potential unwanted double link
When creating an unsolicited MWI aggregate subscription it was possible for
the subscription object to be double unref'ed. This patch removes the explicit
unref as it is not needed since the RAII_VAR will handle it at function end.
Less concerning there was also a bug that could potentially allow the aggregate
subscription object to be added to the unsolicited container twice. This patch
ensures it is added only once.
We've found a connection re-use regression in pjproject 2.9
introduced by commit
"Close #1019: Support for multiple listeners."
https://trac.pjsip.org/repos/changeset/6002
https://trac.pjsip.org/repos/ticket/1019
Normally, multiple SSL requests should reuse the same connection
if one already exists to the remote server. When a transport
error occurs, the next request should establish a new connection
and any following requests should use that same one. With this
patch, when a transport error occurs, every new request creates
a new connection so you can wind up with thousands of open tcp
sockets, possibly exhausting file handles, and increasing memory
usage.
Reverting pjproject commit 6002 (and related 6021) restores the
expected behavior.
We also found a memory leak in SSL processing that was introduced by
commit
"Fixed #2204: Add OpenSSL remote certificate chain info"
https://trac.pjsip.org/repos/changeset/6014
https://trac.pjsip.org/repos/ticket/2204
Apparently the remote certificate chain is continually recreated
causing the leak.
Reverting pjproject commit 6014 (and related 6022) restores the
expected behavior.
Both of these issues have been acknowledged by Teluu.
Kevin Harwell [Mon, 23 Sep 2019 16:01:36 +0000 (11:01 -0500)]
res_sorcery_memory_cache: stale item update leak
When a stale item was being updated the object was being retrieved, but its
reference was not being decremented after the update. This patch makes it so
the object is now appropriately de-referenced.
chan_pjsip: Relock correct channel during "fax" redirect.
When fax detection occurs on an outbound PJSIP channel the
redirect operation will result in a masquerade occurring and
the underlying channel on the session changing. The code
incorrectly relocked the new channel instead of the old
channel when returning. This resulted in the new channel
being locked indefinitely. The code now always acts on the
expected channel.
On FreeBSD using the clang/llvm compiler build fails to build due
to the switch statement argument being a non integer type expression.
Switch to an if/else if/else construct to sidestep the issue.
sungtae kim [Tue, 27 Aug 2019 22:44:33 +0000 (00:44 +0200)]
res_musiconhold: Added unregister realtime moh class
This fix allows a realtime moh class to be unregistered from the command
line. This is useful when the contents of a directory referenced by a
realtime moh class have changed.
The realtime moh class is then reloaded on the next request and uses the
new directory contents.
ChanIsAvail() generates a CDR when unanswered=yes in cdr.conf.
ChanIsAvail() creates a temporary channel with ast_request() to test
resource availability. It should not generate a CDR when it hangs up
this temporary channel.
This patch disables CDR generation for the temporary channel with
ast_cdr_set_property().
chan_dahdi: set CHANNEL(hangupsource) when a PRI channel hangs up
When the remote ISDN party ends an ISDN call on a PRI link
(DISCONNECT), CHANNEL(hangupsource) information is not available.
chan_dahdi already contains an ast_set_hangupsource() in
__dahdi_exception() function but it seems that ISDN message processing
does not use this part of code.
Two other channel modules associate ast_queue_hangup() and
ast_set_hangupsource() functions calls:
- chan_pjsip in chan_pjsip_session_end() function,
- chan_sip in sip_queue_hangup_cause() function.
chan_iax2 separates them, in iax2_queue_hangup()/iax2_destroy() and
set_hangup_source_and_cause().
Thus, I propose to add ast_set_hangupsource() beside
ast_queue_hangup() in sig_pri_queue_hangup(), like chan_pjsip and
chan_sip already do.
Joshua Colp [Mon, 26 Aug 2019 12:53:27 +0000 (09:53 -0300)]
AST-2019-005 - translate: Don't assume all frames will have a src.
This change removes the assumption that a frame will always have
a src set on it. This assumption is incorrect.
Given a scenario where an RTP packet is received with no payload
the resulting audio frame will have no samples. If this frame goes
through a signed linear translation path an interpolated frame can
be created (if generic packet loss concealment is enabled) that has
minimal data on it, including no src. If this frame is given to a
translation path a crash will occur due to the lack of src.
chan_unistim: Fix clang warning: variable sized type not at end of a struct
On reading information about initial client packet unistim use dirty
implementation of destination ip address retrieval. This fix uses
CMSG_*(..) to get ip address and make clang compile without warning.
ASTERISK-25592 #close Reported-by: Alexander Traud
Change-Id: Ic1fd34c2c2bcc951da65bf62e3f7a8adff8351b1
Kevin Harwell [Fri, 23 Aug 2019 21:24:50 +0000 (16:24 -0500)]
res_pjsip_mwi: add better handling of solicited vs unsolicited subscriptions
res_pjsip_mwi allows both solicited and unsolicited MWI subscription types.
While both can be set in the configuration for a given endpoint/aor, only
one is allowed. Precedence is given to unsolicited. Meaning if an endpoint/aor
is configured to allow both types then the solicited subscription is rejected
when it comes in. However, there is a configuration option to override that
behavior:
mwi_subscribe_replaces_unsolicited
When set to "yes" then when a solicited subscription comes in instead of
rejecting it Asterisk is suppose to replace the unsolicited one if it exists.
Prior to this patch there was a bug in Asterisk that allowed the solicted one
to be added, but did not remove the unsolicited. As a matter of fact a new
unsolicited subscription got added everytime a SIP register was received.
Over time this eventually could "flood" a phone with SIP notifies.
This patch fixes that behavior to now make it work as expected. If configured
to do so a solicited subscription now properly replaces the unsolicited one.
As well when an unsubscribe is received the unsolicited subscription is
restored. Logic was also put in to handle reloads, and any configuration changes
that might result from that. For instance, if a solicited subscription had
previously replaced an unsolicited one, but after reload it was configured to
not allow that then the solicited one needs to be shutdown, and the unsolicited
one added.
chan_unistim: Fix code, causing all incoming DTMF sent back to asterisk
Current implementation of ast_channel_tech send_digit_begin hook uses
same function for tone playback as key press handler. This cause every
incoming dtmf send back to asterisk. In case of two unistim phones
connected to each other, it'll cause indefinite DTMF loop. Fix add
separate function for dtmf tone phone play.
chan_unistim: Fix RTP port byte order for big-endian arch
This patch fixes one-way oudio that users expirienced on
big-endian architechtires. RTP port number bytes was stored
in improper order and phone sent RTP to wrong RTP port.
Dan Cropp [Wed, 21 Aug 2019 15:58:00 +0000 (10:58 -0500)]
pjproject: Configurable setting for cnonce to include hyphens or not
NEC SIP Station interface with authenticated registration only supports cnonce
up to 32 characters. In Linux, PJSIP would generate 36 character cnonce
which included hyphens. Teluu developed this patch adding a compile time
setting to default to not include the hyphens. They felt it best to still
generate the UUID and strip the hyphens.
They have indicated it will be part of PJSIP 2.10.
ASTERISK-28509 Reported-by: Dan Cropp
Change-Id: Ibdfcf845d4f8c0a14df09fd983b11f2d72c5f470
In chan_sip, there was variable SIPFROMDOMAIN that allows to set
From header URI domain per channel. This patch introduces res_pjsip
variable SIPFROMDOMAIN for backward compatibility with chan_sip.
George Joseph [Tue, 20 Aug 2019 18:04:56 +0000 (12:04 -0600)]
res_ari.c: Prefer exact handler match over wildcard
Given the following request path and 2 handler paths...
Request: /channels/externalMedia
Handler: /channels/{channelId} "wildcard"
Handler: /channels/externalmedia "non-wildcard"
...if /channels/externalMedia was registered as a handler after
/channels/{channelId} as shown above, the request would automatically
match the wildcard handler and attempt to parse "externalMedia" into
the channelId variable which isn't what was intended. It'd work
if the non-wildard entry was defined in rest-api/api-docs/channels.json
before the wildcard entry but that makes the json files
order-dependent which isn't a good thing.
To combat this issue, the search loop saves any wildcard match but
continues looking for exact matches at the same level. If it finds
one, it's used. If it hasn't found an exact match at the end of
the current level, the wildcard is used. Regardless, after
searching the current level, the wildcard is cleared so it won't
accidentally match for a different object or a higher level.
BTW, it's currently not possible for more than 1 wildcard entry
to be defined for a level. For instance, there couldn't be:
Handler: /channels/{channelId}
Handler: /channels/{channelName}
We wouldn't know which one to match.
Sean Bright [Fri, 9 Aug 2019 20:53:03 +0000 (16:53 -0400)]
audiohook.c: Substitute silence for unavailable audio frames
There are 4 scenarios to consider when capturing audio from a channel
with an audiohook:
1. There is no rx and no tx audio, so return nothing.
2. There is rx but no tx audio, so return rx.
3. There is tx but no rx audio, so return tx.
4. There is rx and tx audio, so mix them and return.
The file passed as the primary argument to MixMonitor will be written to
in scenarios 2, 3, and 4. However, if you pass the r() and t() options
to MixMonitor, a frame will only be written to the r() file if there was
rx audio and a frame will only be written to the t() file if there was
tx audio.
If you subsequently take the r() and t() files and try to mix them, the
sides of the conversation will 'drift' and be non-representative of the
user experience.
This patch adds a new 'S' option to MixMonitor that injects a frame of
silence on either the r() side or the t() side of the channel so that
when later mixed, there is no such drift.
Alexei Gradinari [Wed, 14 Aug 2019 19:52:01 +0000 (15:52 -0400)]
app_voicemail/IMAP: check mailstream not NULL in leave_voicemail
The function leave_voicemail checks if expungeonhangup is set,
but does not check if IMAP stream is closed,
so it could call imap function with NULL stream.
This leads to segfault.
Kevin Harwell [Wed, 7 Aug 2019 22:54:34 +0000 (17:54 -0500)]
srtp: Fix possible race condition, and add NULL checks
Somehow it's possible for the srtp session object to be NULL even though the
Asterisk srtp object itself is valid. When this happened it would cause a
crash down in the srtp code when attempting to protect or unprotect data.
After looking at the code there is at least one spot that makes this situation
possible. If Asterisk fails to unprotect the data, and after several retries
it still can't then the srtp->session gets freed, and set to NULL while still
leaving the Asterisk srtp object around. However, according to the original
issue reporter this does not appear to be their situation since they found
no errors logged stating the above happened (which Asterisk does for that
situation).
An issue was found however, where a possible race condition could occur between
the pjsip incoming negotiation, and the receiving of RTP packets. Both places
could attempt to create/setup srtp for the same rtp instance at the same time.
This potentially could be the cause of the problem as well.
Given the above this patch adds locking around srtp setup for a given rtp, or
rtcp instance. NULL checks for the session have also been added within the
protect and unprotect functions as a precaution. These checks should at least
stop Asterisk from crashing if it gets in this situation again.
This patch also fixes one other issue noticed during investigation. When doing
a replace the old object was freed before creating the replacement. If the new
replacement object failed to create then the rtp/rtcp instance would now point
to freed srtp data which could potentially cause a crash as well when the next
attempt to reference it was made. This is now fixed so the old srtp object is
kept upon replacement failure.
Lastly, more logging has been added to help diagnose future issues.
George Joseph [Thu, 8 Aug 2019 12:12:18 +0000 (06:12 -0600)]
CI: Add "throttle" label and "skip_gate" capability
To make throttling by label fully active, the "throttle" option
has to be specified with a specific label.
You can now specify "skip_gate" in the Gerrit comments when you
do a +2 code review to tell Jenkins not to actually run the
gate. You'd do this if you plan to manually merge the change.
Also updated the "printenv" debug output to better sort multi-line
comments.
Joshua Colp [Mon, 5 Aug 2019 12:23:53 +0000 (09:23 -0300)]
cdr / cel: Use event time at event creation instead of processing.
When updating times on CDR or CEL records using the time at which
it is done can result in times being incorrect if the system is
heavily loaded and stasis message processing is delayed.
This change instead makes it so CDR and CEL use the time at which
the stasis messages that drive the systems are created. This allows
them to be backed up while still producing correct records.
George Joseph [Tue, 6 Aug 2019 15:40:54 +0000 (09:40 -0600)]
CI: Make node labels job-specific
Originally, the eligible nodes for a job were labelled only by
"swdev-docker". So basically any node could run any job. We had
found that allowing a node to run more than 1 gate at a time was
problematic so we limited the nodes to processing 1 job at a time.
With the creation of the Asterisk 17 branches however, we now have
so many active branches that getting checks and gates through in
a timely manner is problematic when a node can run only 1 job
at a time.
Now the nodes are also labelled by the job type they can run.
For instance: "asterisk-check", "asterisk-gate", etc. With the
"Throttle Concurrent Builds" plugin, we can now allow a node to
run more than 1 job BUT throttle by job type. For instance:
Allow 2 jobs but only 1 asterisk-gate at a time.
Now a node can run 2 checks or 1 check and 1 gate or 1 gate but
not 2 gates at a time.
Kevin Harwell [Thu, 1 Aug 2019 21:22:01 +0000 (16:22 -0500)]
various modules: json integer overflow
There were still a few places in the code that could overflow when "packing"
a json object with a value outside the base type integer's range. For instance:
unsigned int value = INT_MAX + 1
ast_json_pack("{s: i}", value);
would result in a negative number being "packed". In those situations this patch
alters those values to a ast_json_int_t, which widens the value up to a long or
long long.
Joshua Colp [Thu, 1 Aug 2019 10:07:45 +0000 (10:07 +0000)]
res_pjsip: Fix multiple of the same contact in "pjsip show contacts".
The code for gathering contacts could result in the same contact
being retrieved and added to the list multiple times. The container
which stores the contacts to display will now only allow a contact
to be added to it once instead of multiple times.
Sean Bright [Wed, 24 Jul 2019 20:12:49 +0000 (16:12 -0400)]
manager: Send fewer packets
The functions that build manager message headers do so in a way that
results in a single messages being split across multiple packets. While
this doesn't matter to the remote end, it makes network captures noisier
and harder to follow, and also means additional system calls.
With this patch, we build up more of the message content into the TLS
buffer before flushing to the network. This change is completely
internal to the manager code and does not affect any of the existing
API's consumers.
George Joseph [Wed, 24 Jul 2019 20:15:27 +0000 (14:15 -0600)]
CI: Don't enable non-core modules in Certified branches
We don't support non-core modules for Certified releases but we
were enabling them for CI builds which was causing lots of test
failures. Now we don't.
George Joseph [Fri, 19 Jul 2019 16:20:38 +0000 (10:20 -0600)]
CI: Add cleanWs to cleanup steps in jenkinsfiles
We're at the point where there are enough Jenkins jobs for
Asterisk branches than even cleaned checkouts of Asterisk
will add up to more disk space than is available on the
in-memory workspace mount. Since we archive all relevent
artifacts anyway, there's no need to keep the workspace
around after the job finishes, whether it succeeds or fails.
Walter Doekes [Wed, 17 Jul 2019 13:06:12 +0000 (15:06 +0200)]
sched: Don't allow ast_sched_del to deadlock ast_sched_runq from same thread
When fixing ASTERISK~24212, a change was done so a scheduled callback could not
be removed while it was running. The caller of ast_sched_del would have to wait.
However, when the caller of ast_sched_del is the callback itself (however wrong
this might be), this new check would cause a deadlock: it would wait forever
for itself.
This changeset introduces an additional check: if ast_sched_del is called
by the callback itself, it is immediately rejected (along with an ERROR log and
a backtrace). Additionally, the AST_SCHED_DEL_UNREF macro is adjusted so the
after-ast_sched_del-refcall function is only run if ast_sched_del returned
success.
This should fix the following spurious race condition found in chan_sip:
- thread 1: schedule sip_poke_peer_now (using AST_SCHED_REPLACE)
- thread 2: run sip_poke_peer_now
- thread 2: blank out sched-ID (too soon!)
- thread 1: set sched-ID (too late!)
- thread 2: try to delete the currently running sched-ID
After this fix, an ERROR would be logged, but no deadlocks (in do_monitor) nor
excess calls to sip_unref_peer(peer) (causing double frees of rtp_instances and
other madness) should occur.
(Thanks Richard Mudgett for reviewing/improving this "scary" change.)
Note that this change does not fix the observed race condition: unlocked
access to peer->pokeexpire (and potentially other scheduled items in chan_sip),
causing AST_SCHED_DEL_UNREF to look at a changing id. But it will make the
deadlock go away. And in the observed case, it will not have adverse affects
(like memory leaks) because the scheduled item is removed through a different
path.
George Joseph [Tue, 16 Jul 2019 13:15:14 +0000 (07:15 -0600)]
Build: Add separate header install/uninstall targets
Two new Makefile targets have been added... "install-headers" and
"uninstall-headers" to separately control header installation.
The existing behavior has not changed so "make install" and
"make uninstall" will continue to also install/uninstall the headers.
The new targets were added for forward compatibility with Asterisk 17
in which the headers are no longer installed/uninstalled with the
"install" and "uninstall" targets.
Also corrects an issue where /usr/include/asterisk.h was never
being removed at all.
chan_sip: Handle invalid SDP answer to T.38 re-invite
The chan_sip module performs a T.38 re-invite using a single media
stream of udptl, and expects the SDP answer to be the same.
If an SDP answer is received instead that contains an additional
media stream with no joint codec a crash will occur as the code
assumes that at least one joint codec will exist in this
scenario.
Details:
- The memcpy() call copied part of "dahdi_conf" and not "dahdi_conf.mfcr2"
- As a result, the memcmp() in dahdi_r2_get_link() always fails
- This cause dahdi_r2_get_link() to create new link for every channel
(instead of a new link for every ~30 channels)
- With the fix, far less links are generated -- so we use far less threads
Fixes a crash in chan_dahdi occurring on 32-bit systems. A previous
patch introduced a variable of type unassigned long long which is 64-bits.
Casting it as 'ast_json_int_t' along with JSON type 'I' makes it work
with 32-bit systems.
George Joseph [Thu, 27 Jun 2019 17:46:44 +0000 (11:46 -0600)]
pjproject_bundled: Add peer information to most SSL/TLS errors
Most SSL/TLS error messages coming from pjproject now have either
the peer address:port or peer hostname, depending on what was
available at the time and code location where the error was
generated.