Sean Bright [Fri, 21 Apr 2017 17:04:44 +0000 (13:04 -0400)]
cleanup: Fix fread() and fwrite() error handling
Cleaned up some of the incorrect uses of fread() and fwrite(), mostly in
the format modules. Neither of these functions will ever return a value
less than 0, which we were checking for in some cases.
I've introduced a fair amount of duplication in the format modules, but
I plan to change how format modules work internally in a subsequent
patch set, so this is simply a stop-gap.
This change adds a T.38 format which can be used in a stream
topology to specify that a UDPTL stream needs to be created.
The SDP API has been changed to understand T.38 and create
the UDPTL session, add the attributes, and parse the attributes.
This change does not change the boundary of the T.38 state
machine. It is still up to the channel driver to implement and
act on it (such as queueing control frames or reacting to them).
Mark Michelson [Tue, 21 Mar 2017 20:44:44 +0000 (15:44 -0500)]
SDP: Ensure SDPs "merge" properly.
The gist of this work ensures that when a remote SDP is received, it is
merged properly with the local capabilities. The remote SDP is converted
into a stream topology. That topology is then merged with the current
local topology on the SDP state. That new merged topology is then used
to create an SDP. Finally, adjustments are made to RTP instances based
on knowledge gained from the remote SDP.
There are also a battery of tests in this commit that ensure that some
basic SDP merges work as expected.
While this may not sound like a big change, it has the property that it
caused lots of ancillary changes.
* The remote SDP is no longer stored on the SDP state. Biggest reason:
there's no need for it. The remote SDP is used at the time it is being
set and nowhere else.
* Some new SDP APIs were added in order to find attributes and convert
generic SDP attributes into rtpmap structures.
* Writing tests made me realize that retrieving a value from an SDP
options structure, the SDP options needs to be made const.
* The SDP state machine was essentially gutted by a previous commit.
Initially, I attempted to reinstate it, but I found that as it had
been defined, it was not all that useful. What was more useful was
knowing the role we play in SDP negotiation, so the SDP state machine
has been transformed into an indicator of role.
* Rather than storing separate local and joint stream state
capabilities, it makes more sense to keep track of current stream
state and update it as things change.
Sean Bright [Tue, 18 Apr 2017 00:06:10 +0000 (20:06 -0400)]
core: Use eventfd for alert pipes on Linux when possible
The primary win of switching to eventfd when possible is that it only
uses a single file descriptor while pipe() will use two. This means for
each bridge channel we're reducing the number of required file
descriptors by 1, and - if you're using timerfd - we also now have 1
less file descriptor per Asterisk channel.
The API is not ideal (passing int arrays), but this is the cleanest
approach I could come up with to maintain API/ABI.
I've also removed what I believe to be an erroneous code block that
checked the non-blocking flag on the pipe ends for each read. If the
file descriptor is 'losing' its non-blocking mode, it is because of a
bug somewhere else in our code.
In my testing I haven't seen any measurable difference in performance.
Richard Mudgett [Fri, 21 Apr 2017 17:33:34 +0000 (12:33 -0500)]
res_pjsip_session.c: Send 100 Trying out earlier to prevent retransmissions.
If ICE is enabled and a STUN server does not respond then we will block
until we give up on the STUN response. This will take nine seconds. In
the mean time the peer that sent the INVITE will send retransmissions.
* Restructure res_pjsip_session.c:new_invite() to send a 100 Trying out
earlier to prevent these retransmissions.
* Restructure ast_sip_session_alloc() to need less cleanup on off nominal
error paths.
* Made ast_sip_session_alloc() and ast_sip_session_create_outgoing() avoid
unnecessary ref manipulation to return a session. This is faster than
calling a function. That function may do logging of the ref changes with
REF_DEBUG enabled.
Sean Bright [Wed, 19 Apr 2017 20:08:39 +0000 (16:08 -0400)]
pbx: Use same thread if AST_OUTGOING_WAIT_COMPLETE specified
Both ast_pbx_outgoing_app() and ast_pbx_outgoing_exten() cause the core
to spawn a new thread to perform the dial. When AST_OUTGOING_WAIT_COMPLETE
is passed to these functions, the calling thread will be blocked until
the newly created channel has been hung up.
After this patch, we run the dial on the current thread rather than
spawning a new one. The only in-tree code that passes
AST_OUTGOING_WAIT_COMPLETE is pbx_spool, so you should see reduced
thread usage if you are using .call files.
Richard Mudgett [Wed, 19 Apr 2017 18:23:55 +0000 (13:23 -0500)]
res_rtp_asterisk.c: Fix crash in RTCP DTLS operation.
Occasionally a crash happens when processing the RTCP DTLS timeout
handler. The RTCP DTLS timeout timer could be left running if we have not
completed the DTLS handshake before we place the call on hold or we
attempt direct media.
* Made ast_rtp_prop_set() stop the RTCP DTLS timer when disabling RTCP.
* Made some sanity tweaks to ast_rtp_prop_set() when switching from
standard RTCP mode to RTCP multiplexed mode.
The struct ast_rtp_instance has historically been indirectly protected
from reentrancy issues by the channel lock because early channel drivers
held the lock for really long times. Holding the channel lock for such a
long time has caused many deadlock problems in the past. Along comes
chan_pjsip/res_pjsip which doesn't necessarily hold the channel lock
because sometimes there may not be an associated channel created yet or
the channel pointer isn't available.
In the case of ASTERISK-26835 a pjsip serializer thread was processing a
message's SDP body while another thread was reading a RTP packet from the
socket. Both threads wound up changing the rtp->rtcp->local_addr_str
string and interfering with each other. The classic reentrancy problem
resulted in a crash.
In the case of ASTERISK-26853 a pjsip serializer thread was processing a
message's SDP body while another thread was reading a RTP packet from the
socket. Both threads wound up processing ICE candidates in PJPROJECT and
interfering with each other. The classic reentrancy problem resulted in a
crash.
* rtp_engine.c: Make the ast_rtp_instance_xxx() calls lock the RTP
instance struct.
* rtp_engine.c: Make ICE and DTLS wrapper functions to lock the RTP
instance struct for the API call.
* res_rtp_asterisk.c: Lock the RTP instance to prevent a reentrancy
problem with rtp->rtcp->local_addr_str in the scheduler thread running
ast_rtcp_write().
* res_rtp_asterisk.c: Avoid deadlock when local RTP bridging in
bridge_p2p_rtp_write() because there are two RTP instance structs
involved.
* res_rtp_asterisk.c: Avoid deadlock when trying to stop scheduler
callbacks. We cannot hold the instance lock when trying to stop a
scheduler callback.
* res_rtp_asterisk.c: Remove the lock in struct dtls_details and use the
struct ast_rtp_instance ao2 object lock instead. The lock was used to
synchronize two threads to prevent a race condition between starting and
stopping a timeout timer. The race condition is no longer present between
dtls_perform_handshake() and __rtp_recvfrom() because the instance lock
prevents these functions from overlapping each other with regards to the
timeout timer.
* res_rtp_asterisk.c: Remove the lock in struct ast_rtp and use the struct
ast_rtp_instance ao2 object lock instead. The lock was used to
synchronize two threads using a condition signal to know when TURN
negotiations complete.
* res_rtp_asterisk.c: Avoid deadlock when trying to stop the TURN
ioqueue_worker_thread(). We cannot hold the instance lock when trying to
create or shut down the worker thread without a risk of deadlock.
This patch exposed a race condition between a PJSIP serializer thread
setting up an ICE session in ice_create() and another thread reading RTP
packets.
* res_rtp_asterisk.c:ice_create(): Set the new rtp->ice pointer after we
have re-locked the RTP instance to prevent the other thread from trying to
process ICE packets on an incomplete ICE session setup.
A similar race condition is between a PJSIP serializer thread resetting up
an ICE session in ice_create() and the timer_worker_thread() processing
the completion of the previous ICE session.
* res_rtp_asterisk.c:ast_rtp_on_ice_complete(): Protect against an
uninitialized/null remote_address after calling
update_address_with_ice_candidate().
* res_rtp_asterisk.c: Eliminate the chance of ice_reset_session()
destroying and setting the rtp->ice pointer to NULL while other threads
are using it by adding an ao2 wrapper around the PJPROJECT ice pointer.
Now when we have to unlock the RTP instance object to call a PJPROJECT ICE
function we will hold a ref to the wrapper. Also added some rtp->ice NULL
checks after we relock the RTP instance and have to do something with the
ICE structure.
George Joseph [Mon, 17 Apr 2017 00:59:54 +0000 (18:59 -0600)]
make ari-stubs so doc periodic jobs can run
The periodic doc job does a make ari-stubs and checks that
there are no changes before generating the docs. Since I changed
the mustache template (and the generated code directly) recently
and forgot to regenerate the stubs, the doc job thinks they're out
of date.
Sean Bright [Fri, 14 Apr 2017 21:50:56 +0000 (17:50 -0400)]
res_stun_monitor: Don't fail to load if DNS resolution fails
res_stun_monitor will fail to load if DNS resolution of the STUN server
fails. Instead, we continue without the STUN server being resolved and
we will re-attempt the resolution on the STUN refresh interval.
Sean Bright [Fri, 14 Apr 2017 19:36:57 +0000 (15:36 -0400)]
format_pcm: Track actual header size of .au files
Sun's Au file format has a minimum data offset 24 bytes, but this
offset is encoded in each .au file. Instead of assuming the minimum,
read the actual value and store it for later use.
ASTERISK-20984 #close
Reported by: Roman S.
Patches:
asterisk-1.8.20.0-au-clicks-2.diff (license #6474) patch
uploaded by Roman S.
Alexander Traud [Mon, 10 Apr 2017 10:13:39 +0000 (12:13 +0200)]
res_pjsip_sdp_rtp: No rtpmap for static RTP payload IDs in SDP.
This saves around 100 bytes when G.711, G.722, G.729, and GSM are advertised in
SDP. This reduces the chance to hit the MTU bearer of 1300 bytes for SIP over
UDP, if many codecs are allowed in Asterisk. This new feature is enabled
together with the optional feature compact_headers=yes via the file pjsip.conf.
George Joseph [Tue, 11 Apr 2017 16:07:39 +0000 (10:07 -0600)]
modules: change module LOAD_FAILUREs to LOAD_DECLINES
In all non-pbx modules, AST_MODULE_LOAD_FAILURE has been changed
to AST_MODULE_LOAD_DECLINE. This prevents asterisk from exiting
if a module can't be loaded. If the user wishes to retain the
FAILURE behavior for a specific module, they can use the "require"
or "preload-require" keyword in modules.conf.
A new API was added to logger: ast_is_logger_initialized(). This
allows asterisk.c/check_init() to print to the error log once the
logger subsystem is ready instead of just to stdout. If something
does fail before the logger is initialized, we now print to stderr
instead of stdout.
strings.h: Avoid overflows in the string hash functions
On 2's compliment machines abs(INT_MIN) behavior is undefined and
results in a negative value still being returnd. This results in
negative hash codes that can result in crashes.
Richard Mudgett [Fri, 7 Apr 2017 21:14:16 +0000 (16:14 -0500)]
res_rtp_asterisk.c: Add stun_blacklist option
Added the stun_blacklist option to rtp.conf. Some multihomed servers have
IP interfaces that cannot reach the STUN server specified by stunaddr.
Blacklist those interface subnets from trying to send a STUN packet to
find the external IP address. Attempting to send the STUN packet
needlessly delays processing incoming and outgoing SIP INVITEs because we
will wait for a response that can never come until we give up on the
response. Multiple subnets may be listed.
Richard Mudgett [Thu, 6 Apr 2017 22:31:14 +0000 (17:31 -0500)]
stun.c: Fix ast_stun_request() erratic timeout.
If ast_stun_request() receives packets other than a STUN response then we
could conceivably never exit if we continue to receive packets with less
than three seconds between them.
* Fix poll timeout to keep track of the time when we sent the STUN
request. We will now send a STUN request every three seconds regardless
of how many other packets we receive while waiting for a response until we
have completed three STUN request transmission cycles.
Richard Mudgett [Thu, 6 Apr 2017 23:18:16 +0000 (18:18 -0500)]
res_pjsip_sdp_rtp.c: Don't use deprecated transport struct member.
* create_rtp(): Eliminate use of deprecated transport struct member. That
member and several others in the transport structure were deprecated
because of an infinite loop created when using realtime configuration.
See 2451d4e4550336197ee2e482750cc53f30afa352
Richard Mudgett [Mon, 10 Apr 2017 22:45:35 +0000 (17:45 -0500)]
tcptls.c: Cleanup TCP/TLS listener thread on abnormal exit.
Temporarily running out of file descriptors should not terminate the
listener thread. Otherwise, when there becomes more file descriptors
available, nothing is listening.
* Added EMFILE exception to abnormal thread exit.
* Added an abnormal TCP/TLS listener exit error message.
* Closed the TCP/TLS listener socket on abnormal exit so Asterisk does not
appear dead if something tries to connect to the socket.
This change adds database tables for the PUBLISH support so it
can be configured using realtime. A minor fix to the
res_pjsip_publish_asterisk module was done so that it read the
sorcery configuration from the correct section. Finally the
sample configuration files have been updated.
Alexander Traud [Fri, 7 Apr 2017 13:06:11 +0000 (15:06 +0200)]
pjproject_bundled: Crash on pj_ssl_get_info() while ioqueue_on_read_complete().
When the Asterisk channel driver res_pjsip offers SIP-over-TLS, sometimes, not
reproducible, Asterisk crashed in pj_ssl_sock_get_info() because a NULL pointer
was read. This change avoids this crash.
We needed the reason for our reporting when agents pause/unpause all of
their queues at once. This is a small, simple patch that adds a reason
for PAUSEALL and UNPAUSEALL. I have been using it in production for years.
Corey Farrell [Mon, 27 Mar 2017 14:03:49 +0000 (10:03 -0400)]
CDR: Protect from data overflow in ast_cdr_setuserfield.
ast_cdr_setuserfield wrote to a fixed length field using strcpy. This could
result in a buffer overrun when called from chan_sip or func_cdr. This patch
adds a maximum bytes written to the field by using ast_copy_string instead.
ASTERISK-26897 #close
patches:
0001-CDR-Protect-from-data-overflow-in-ast_cdr_setuserfie.patch submitted
by Corey Farrell (license #5909)
Corey Farrell [Fri, 31 Mar 2017 17:09:38 +0000 (13:09 -0400)]
core: Improve/simplify handling of required headers.
* Report failures if configure finds a required header is missing.
* Deduplicate includes between asterisk.h, astmm.h and compat.h.
* Unconditionally include headers in compat.h if required elsewhere.
Alexander Traud [Mon, 3 Apr 2017 07:30:43 +0000 (09:30 +0200)]
chan_sip: Session Timers required but refused wrongly.
SIP user-agents indicate which protocol extensions are allowed in headers
like Supported and Required. Such protocol extensions are Session Timers
(RFC 4028) for example. Session Timers are supported since Mantis-10665.
Since ASTERISK-21721, not only the first but multiple Supported/Required
headers in a message are parsed. In that change, an existing variable was
re-used within a newly added do-loop. Currently, at the end of that loop,
that variable is an empty string always. Previously, that variable was used
within log output. However, the log output was not changed.
Joshua Colp [Fri, 31 Mar 2017 21:31:24 +0000 (21:31 +0000)]
res_pjsip_session: Allow BYE to be sent on disconnected session.
It is perfectly acceptable for a BYE to be sent on a disconnected
session. This occurs when we respond to a challenge to the BYE
for authentication credentials.
Corey Farrell [Thu, 30 Mar 2017 23:28:18 +0000 (19:28 -0400)]
Forward declare 'struct ast_json' in asterisk.h
The ast_json structure is used in many Asterisk headers and is often the
only part of json.h used. This adds a forward declaration to asterisk.h
and removes the include of json.h from many headers. The declaration
has been left in endpoints.h and stasis.h to avoid problems with source
files that use ast_json functions without directly including json.h.
ari.h continues to include json.h as it uses enum
ast_json_encoding_format.
Walter Doekes [Tue, 28 Mar 2017 18:01:16 +0000 (20:01 +0200)]
build: Fix deb build issues with fakeroot
If DESTDIR is set, don't call ldconfig. Assume that DESTDIR is used to
create a binary archive. The ldconfig call should be delegated to the
archive postinst script. This fixes the case where fakeroot wraps 'make
install' causing $EUID to be 0 even though it doesn't have permission to
call ldconfig.
The previous logic in configure.ac to detect and correct libdir
has been removed as it was not completely accurate. CentOS 64-bit
users should again specifiy --libdir=/usr/lib64 when configuring
to prevent install to /usr/lib.
Updated Makefile:check-old-libdir to check for orphans in
lib64 when installing to lib as well as orphans in lib when installing
to lib64.
Updated Makefile and main/Makefile uninstall targets to remove the
orphans using the new logic.
Joshua Colp [Mon, 27 Mar 2017 20:32:45 +0000 (20:32 +0000)]
sdp: Add support for setting connection address and clean up state.
This change cleans up state management for media streams by moving
RTP instances into their own session structure and adding additional
details that are not relevant to the core (such as connection address).
These can live either in the local capabilities or joint capabilities.
The ability to set explicit connection address information for
the purposes of direct media and NAT has also been added at the
global and stream specific level.
Sean Bright [Wed, 29 Mar 2017 15:11:51 +0000 (11:11 -0400)]
astobj2: Prevent potential deadlocks with ao2_global_obj_release
The ao2_global_obj_release() function holds an exclusive lock on the
global object while it is being dereferenced. Any destructors that
run during this time that call ao2_global_obj_ref() will deadlock
because a read lock is required.
Instead, we make the global object inaccessible inside of the write
lock and only dereference it once we have released the lock. This
allows the affected destructors to fail gracefully.
While this doesn't completely solve the referenced issue (the error
message about not being able to create an IQ continues to be shown)
it does solve the backtrace spew that accompanied it.