Commit Graph

315 Commits

Author SHA1 Message Date
sungtae kim c8c94b6cf1 res_rtp_asterisk.c: Fixed memory leak
Added freeifaddrs() for memory releasing.

ASTERISK-28904

Change-Id: I109403866e85a30659351946903a679de9727a8f
2020-05-18 16:31:58 +00:00
Guido Falsi e4366308e1 res_rtp_asterisk: Protect access to nochecksums with #ifdef
Recently code accessing nochecksums variable has been added without including #ifdef SO_NO_CHECK protection, while the variable is created only when such constant is defined.

ASTERISK-28852 #close

Change-Id: I381718893b80599ab8635f2b594a10c1000d595e
2020-04-28 13:57:20 -05:00
Pirmin Walthert d50fd0acc0 res_rtp_asterisk: Resolve loop when receive buffer is flushed
When the receive buffer was flushed by a received packet while it
already contained a packet with the same sequence number, Asterisk
never left the while loop which tried to order the packets.

This change makes it so if the packet is in the receive buffer it
is retrieved and freed allowing the buffer to empty.

ASTERISK-28827

Change-Id: Idaa376101bc1ac880047c49feb6faee773e718b3
2020-04-17 06:11:19 -05:00
Pirmin Walthert ca032d1e2e res_rtp_asterisk: Free payload when error on insertion to data buffer
When the ast_data_buffer_put rejects to add a packet, for example because
the buffer already contains a packet with the same sequence number, the
payload will never be freed, resulting in a memory leak.

The data buffer will now return an error if this situation occurs
allowing the caller to free the payload. The res_rtp_asterisk module
has also been updated to do this.

ASTERISK-28826

Change-Id: Ie6c49495d1c921d5f997651c7d0f79646f095cf1
2020-04-15 13:56:40 -05:00
bernard merindol 7db03e12a7 res_rtp_asterisk.c: Check for first DTMF having timestamp set to 0
When the first DTMF receive in RF2833 codec have TimeStamp at 0
is not processed.

ASTERISK-28812

Change-Id: I3196803a062dd2daee4938c9a778c3810cb7e504
2020-04-14 10:28:51 -05:00
Jaco Kroon 2b80e5f5da res_rtp_asterisk: iterate all local addresses looking to populate ICE.
By using pjproject to give us a list of candidates, and then filtering,
if the host has >32 addresses configured, then it is possible that we
end up filtering out all 32 of those, and ending up with no candidates
at all.  Instead, get getifaddrs (which pjsip is using underlying
anyway) to retrieve all local addresses, and iterate those, adding the
first 32 addresses not excluded by the ICE ACL.

In our setup at any point in time We've got between 6 and 328 addresses
on any given system.  The lower limit is the lower limit but the upper
limit is growing on a near daily basis currently.

Change-Id: I109eaffc3e2b432f00bf958e3caa0f38cacb4edb
Signed-off-by: Jaco Kroon <jaco@uls.co.za>
2020-04-13 19:43:54 -05:00
Alexander Traud ee1c7f465b
res_rtp_asterisk: Build without PJProject.
Change-Id: Ifc5059cd867e77b9c92ed9f4b895a9a91200d3ec
2020-04-13 18:27:28 +02:00
traud 1ef1b1b0c2 res_rtp_asterisk: Avoid absolute value on unsigned subtraction.
ASTERISK-28809

Change-Id: I269731715347c8e5ef7db1b6ffd3f8d15fc04be4
2020-04-08 10:01:42 -05:00
Joshua C. Colp 96e8d411e1 res_rtp_asterisk: Ensure sufficient space for worst case NACK.
ASTERISK-28790

Change-Id: I10df52f98b19ed62575f25dab36e82d136dccd99
2020-03-26 08:37:22 -05:00
Jaco Kroon 82c3939c38 res_rtp_asterisk: implement ACL mechanism for ICE and STUN addresses.
A pure blacklist is not good enough, we need a whitelist mechanism as
well, and the simplest way to do that is to re-use existing ACL
infrastructure.

This makes it simpler to blacklist say an entire block (/24) except a
smaller block (eg, a /29 or even a /32).  Normally you'd need to
recursively split the block, so if you want to blacklist a /24 except
for a /29 you'd end up with a blacklit for a /25, /26, /27 and /28.  I
feel that having an ACL instead of a blacklist only is clearer.

Change-Id: Id57a8df51fcfd3bd85ea67c489c85c6c3ecd7b30
Signed-off-by: Jaco Kroon <jaco@uls.co.za>
2020-03-20 08:41:02 -05:00
Torrey Searle a1dba820cf res_rtp_asterisk: Send correct sender SSRC when p2p bridge in use
bridge_p2p_rtp_write will forward rtp to the bridged rtp instance
without modifying the ssrc.  However, it is not updating the SSRC
in the bridged rtp.  Thus, when SSRC packets are generated, they
have the correct SSRC for the sender.

ASTERISK-28773 #close

Change-Id: I39f923bde28ebb4f0fddc926b92494aed294a478
2020-03-12 10:33:04 -05:00
Joshua Colp e8468eee13 Merge "res_rtp_asterisk: Add 'rtp show settings' cli command" 2020-03-09 08:57:09 -05:00
Rodrigo Ramírez Norambuena e089779908 res_rtp_asterisk: Add 'rtp show settings' cli command
This change introduce a CLI command for the RTP to display the general
configuration.

In the first step add the follow fields of the configurations:
  - rtpstart
  - rtpend
  - dtmftimeout
  - rtpchecksum
  - strictrtp
  - learning_min_sequential
  - icesupport

Change-Id: Ibe5450898e2c3e1ed68c10993aa1ac6bf09b821f
2020-03-05 15:48:27 +00:00
Joshua C. Colp 87fda066ea res_rtp_asterisk: Improve video performance in certain networks.
The receive buffer will now grow if we end up flushing the
receive queue after not receiving the expected packet in time.
This is done in hopes that if this is encountered again the
extra buffer size will allow more time to pass and any missing
packets to be received.

The send buffer will now grow if we are asked for packets and
can't find them. This is done in hopes that the packets are
from the past and have simply been expired. If so then in
the future with the extra buffer space the packets should be
available.

Sequence number cycling has been handled so that the
correct sequence number is calculated and used in
various places, including for sorting packets and
for determining if a packet is old or not.

NACK sending is now more aggressive. If a substantial number
of missing sequence numbers are added a NACK will be sent
immediately. Afterwards once the receive buffer reaches 25%
a single NACK is sent. If the buffer continues to grow and
reaches 50% or greater a NACK will be sent for each received
future packet to aggressively ask the remote endpoint to
retransmit.

ASTERISK-28764

Change-Id: I97633dfa8a09a7889cef815b2be369f3f0314b41
2020-03-03 04:53:25 -06:00
Ben Ford 168637cc0c RTP/ICE: Send on first valid pair.
When handling ICE negotiations, it's possible that there can be a delay
between STUN binding requests which in turn will cause a delay in ICE
completion, preventing media from flowing. It should be possible to send
media when there is at least one valid pair, preventing this scenario
from occurring.

A change was added to PJPROJECT that adds an optional callback
(on_valid_pair) that will be called when the first valid pair is found
during ICE negotiation. Asterisk uses this to start the DTLS handshake,
allowing media to flow. It will only be called once, either on the first
valid pair, or when ICE negotiation is complete.

ASTERISK-28716

Change-Id: Ia7b68c34f06d2a1d91c5ed51627b66fd0363d867
2020-02-18 09:55:12 -06:00
Kevin Harwell 3865b3fd6a res_rtp_asterisk: bad audio (static) due to incomplete dtls/srtp setup
There was a race condition between client initiated DTLS setup, and handling
of server side ice completion that caused the underlying SSL object to get
cleared during DTLS initialization. If this happened Asterisk would be left
in a partial DTLS setup state. RTP packets were sent and received, but were
not being encrypted and decrypted. This resulted in no audio, or static.

Specifically, this occurred when '__rtp_recvfrom' was processing the handshake
sequence from the client to the server, and then 'ast_rtp_on_ice_complete'
gets called from another thread and clears the SSL object when calling the
'dtls_perform_setup' function. The timing had to be just right in the sense
that from the external SSL library perspective SSL initialization completed
(rtp recv), Asterisk clears/resets the SSL object (ice done), and then checks
to see if SSL is intialized (rtp recv). Since it was cleared, Asterisk thinks
it is not finished, thus not completing 'dtls_srtp_setup'.

This patch removes calls to 'dtls_perform_setup', which clears the SSL object,
in 'ast_rtp_on_ice_complete'. When ice completes, there is no reason to clear
the underlying SSL object. If an ice candidate changes a full protocol level
renegotiation occurs. Also, in the case of bundled ICE candidates are reused
when a stream is added. So no real reason to have to clear, and reset in this
instance.

Also, this patch adds a bit of extra logging to aid in diagnosis of any future
problems.

ASTERISK-28742 #close

Change-Id: I34c9e6bad5a39b087164646e2836e3e48fe6892f
2020-02-14 10:52:16 -06:00
Joshua C. Colp 1b53d329ac res_rtp_asterisk: Don't produce transport-cc if no packets.
The code assumed that when the transport-cc feedback
function was called at least one packet will have been
received. In practice this isn't always true, so now
we just reschedule the sending and do nothing.

Change-Id: Iabe7b358704da446fc3b0596b847bff8b8a0da6a
2020-02-04 08:19:55 -06:00
George Joseph 39c920ac78 res_rtp_asterisk: Add frame list cleanups to ast_rtp_read
In Asterisk 16+, there are a few places in ast_rtp_read where we've
allocated a frame list but return a null frame instead of the list.
In these cases, any frames left in the list won't be freed.  In the
vast majority of the cases, the list is empty when we return so
there's nothing to free but there have been leaks reported in the
wild that can be traced back to frames left in the list before
returning.

The escape paths now all have logic to free frames left in the
list.

ASTERISK-28609
Reported by: Ted G

Change-Id: Ia1d7075857ebd26b47183c44b1aebb0d8f985f7a
2019-12-10 12:48:32 -06:00
George Joseph b3de3ce042 Merge "res_rtp_asterisk: Always return provided DTLS packet length." 2019-11-18 13:04:05 -06:00
Joshua Colp 02129ad4d0 res_rtp_asterisk: Always return provided DTLS packet length.
OpenSSL can not tolerate if the packet sent out does not
match the length that it provided to the sender. This change
lies and says that each time the full packet was sent. If
a problem does occur then a retransmission will occur as
appropriate.

ASTERISK-28576

Change-Id: Id42455b15c9dc4eb987c8c023ece6fbf3c22a449
2019-11-18 08:34:26 -06:00
Kevin Harwell bdd785d31c various files - fix some alerts raised by lgtm code analysis
This patch fixes several issues reported by the lgtm code analysis tool:

https://lgtm.com/projects/g/asterisk/asterisk

Not all reported issues were addressed in this patch. This patch mostly fixes
confirmed reported errors, potential problematic code points, and a few other
"low hanging" warnings or recommendations found in core supported modules.
These include, but are not limited to the following:

* innapropriate stack allocation in loops
* buffer overflows
* variable declaration "hiding" another variable declaration
* comparisons results that are always the same
* ambiguously signed bit-field members
* missing header guards

Change-Id: Id4a881686605d26c94ab5409bc70fcc21efacc25
2019-11-18 08:30:45 -06:00
Joshua Colp 6e907ae5d4 res_rtp_asterisk: Remove a log message that slipped in.
This was only supposed to be for testing, so now it can be
removed.

Change-Id: I3dfc2e776e70b3196aeed5688372ea80c0214b59
2019-10-17 05:50:49 -05:00
Joshua Colp e79a3b428a Merge "func_jitterbuffer: Add audio/video sync support." 2019-09-19 08:23:15 -05:00
Joshua Colp 7298a785ad func_jitterbuffer: Add audio/video sync support.
This change adds support to the JITTERBUFFER dialplan function
for audio and video synchronization. When enabled the RTCP SR
report is used to produce an NTP timestamp for both the audio and
video streams. Using this information the video frames are queued
until their NTP timestamp is equal to or behind the NTP timestamp
of the audio. The audio jitterbuffer acts as the leader deciding
when to shrink/grow the jitterbuffer when adaptive is in use. For
both adaptive and fixed the video buffer follows the size of the
audio jitterbuffer.

ASTERISK-28533

Change-Id: I3fd75160426465e6d46bb2e198c07b9d314a4492
2019-09-18 20:22:50 +00:00
Ben Ford 723b695ce5 res_rtp_asterisk.c: Send RTCP as compound packets.
According to RFC3550, ALL RTCP packets must be sent in a compond packet
of at least two individual packets, including SR/RR and SDES. REMB,
FIR, and NACK were not following this format, and as a result, would
fail the packet check in ast_rtcp_interpret. This was found from writing
unit tests for RTCP. The browser would accept the way we were
constructing these RTCP packets, but when sending directly from one
Asterisk instance to another, the above mentioned problem would occur.

Change-Id: Ieb140e9c22568a251a564cd953dd22cd33244605
2019-09-13 09:48:31 -05:00
Ben Ford 0e56643d9f res_rtp: Add unit tests for RTCP stats.
Added unit tests for RTCP video stats. These tests include NACK, REMB,
FIR/FUR/PLI, SR/RR/SDES, and packet loss statistics. The REMB and FIR
tests are currently disabled due to a bug. We expect to receive a
compound packet, but the code sends this out as a single packet, which
the browser accepts, but makes Asterisk upset.

While writing these tests, I noticed an issue with NACK as well. Where
it is handling a received NACK request, it was reading in only the first
8 bits of following packets that were also lost. This has been changed
to the correct value of 16 bits.

Also made a minor fix to the data buffer unit test.

Change-Id: I56107c7411003a247589bbb6086d25c54719901b
2019-09-10 13:11:07 -05:00
Joshua Colp 2feac1d361 res_rtp_asterisk: Move where DTLS MTU variable is defined.
The DTLS MTU variable is not dependent on pjproject and should
not exist in its block.

Change-Id: I7e97d64dc192f2ac81bfe2b72b8229d321c7d026
2019-07-14 12:27:13 -06:00
Joshua Colp a8e5cf557d res_rtp_asterisk: Add support for DTLS packet fragmentation.
This change adds support for larger TLS certificates by allowing
OpenSSL to fragment the DTLS packets according to the configured
MTU. By default this is set to 1200.

This is accomplished by implementing our own BIO method that
supports MTU querying. The configured MTU is returned to OpenSSL
which fragments the packet accordingly. When a packet is to be
sent it is done directly out the RTP instance.

ASTERISK-28018

Change-Id: If2d5032019a28ffd48f43e9e93ed71dbdbf39c06
2019-06-13 07:51:57 -06:00
Friendly Automation 9b0a21c402 Merge "res_rtp_asterisk: timestamp should be unsigned instead of signed int" 2019-05-23 09:03:49 -05:00
Morten Tryfoss 3224ac07c9 res_rtp_asterisk: timestamp should be unsigned instead of signed int
Using timestamp with signed int will cause timestamps exceeding max value
to be negative.
This causes the jitterbuffer to do passthrough of the packet.

ASTERISK-28421

Change-Id: I9dabd0718180f2978856c50f43aac4e52dc3cde9
2019-05-21 18:32:57 +02:00
George Joseph be83591f99 res_rtp_asterisk: Add ability to propose local address in ICE
You can now add the "include_local_address" flag to an entry in
rtp.conf "[ice_host_candidates]" to include both the advertized
address and the local address in ICE negotiation:

[ice_host_candidates]
192.168.1.1 = 1.2.3.4,include_local_address

This causes both 192.168.1.1 and 1.2.3.4 to be advertized.

Change-Id: Ide492cd45ce84546175ca7d557de80d9770513db
2019-05-17 17:50:06 -06:00
Joshua Colp 7a6fd83aca res_rtp_asterisk: Fix sequence number cycling and packet loss count.
This change fixes two bugs which both resulted in the packet loss
count exceeding 65,000.

The first issue is that the sequence number check to determine if
cycling had occurred was using the wrong variable resulting in the
check never seeing that cycling has occurred, throwing off the
packet loss calculation. It now uses the correct variable.

The second issue is that the packet loss calculation assumed that
the received number of packets in an interval could never exceed
the expected number. In practice this isn't true due to delayed
or retransmitted packets. The expected will now be updated to
the received number if the received exceeds it.

ASTERISK-28379

Change-Id: If888ebc194ab69ac3194113a808c414b014ce0f6
2019-05-08 09:44:02 -06:00
Joshua Colp 6bb70c93f1 rtp: Add support for transport-cc in receiver direction.
The transport-cc draft is a mechanism by which additional information
about packet reception can be provided to the sender of packets so
they can do sender side bandwidth estimation. This is accomplished
by having a transport specific sequence number and an RTCP feedback
message. This change implements this in the receiver direction.

For each received RTP packet where transport-cc is negotiated we store
the time at which the RTP packet was received and its sequence number.
At a 1 second interval we go through all packets in that period of time
and use the stored time of each in comparison to its preceding packet to
calculate its delta. This delta information is placed in the RTCP
feedback message, along with indicators for any packets which were not
received.

The browser then uses this information to better estimate available
bandwidth and adjust accordingly. This may result in it lowering the
available send bandwidth or adjusting how "bursty" it can be.

ASTERISK-28400

Change-Id: I654a2cff5bd5554ab94457a14f70adb71f574afc
2019-05-01 05:13:14 -06:00
Matthew Fredrickson f78306470b res/res_rtp_asterisk: Enable rxjitter calculation for video
It looks like we're not properly calculating jitter values on received
video streams.  This patch enables the code that does jitter calculations
for those streams.

Change-Id: Iaac985808829c8f034db8c57318789c4c8c11392
2019-03-27 19:30:45 +00:00
sungtae kim 8641fd9700 res/res_rtp_asterisk.c: Fixing possible divide by zero
Currently, when the Asterisk calculates rtp statistics, it uses
sample_count as a unsigned integer parameter. This would be fine
for most of cases, but in case of large enough number of sample_count,
this might be causing the divide by zero error.

ASTERISK-28321

Change-Id: If7e0629abaceddd2166eb012456c53033ea26249
2019-03-11 09:09:09 -03:00
Torrey Searle 360f543677 res/res_rtp_asterisk: smoother can cause wrong timestamps if dtmf happen
Delivery timeval in the smoother object will fall behind while a DTMF is
being generated.  This can eventually lead to invalid rtp timestamps.
To prevent this from happening the smoother needs to be reset after every
DTMF to keep the timing up to date.

ASTERISK-28303 #close

Change-Id: Iaba3f7b428ebd72a4caa90e13b829ab4f088310f
2019-02-26 08:13:38 -06:00
Torrey Searle 8ea9608efb res/res_rtp_asterisk: clear smoother when local bridging
p2p_write updates txformat but doesn't require a smoother.  If a smoother
was created by another bridge type the smoother could fall out of date causing
one way audio issues.  To prevent this the smoother is now destroyed on the
start of native bridge.

ASTERISK-28284 #close

Change-Id: I84e67f144963787fff9b4d79ac500514fb40cdc6
2019-02-19 01:37:57 -06:00
Alexei Gradinari f662a26ea0 RTP: reset DTMF last seqno/timestamp on RTP renegotiation
The remote side may start a new stream when renegotiating RTP.
Need to reset the DTMF last sequence number and the timestamp
of the last END packet on RTP renegotiation.

If the new time stamp is lower then the timestamp of the last DTMF END packet
the asterisk drops all DTMF frames as out of order.

This bug was caught using Cisco ip-phone SPA5XX and codec g722.
On SIP session update the SPA50X resets stream and a new timestamp is twice
smaller then the previous.

ASTERISK-28162 #close

Change-Id: Ic72b4497e74d801b27a635559c1cf29c16c95254
2019-01-04 10:58:39 -05:00
Sean Bright 357219dfb3 res_rtp_asterisk: Remove some unused structure fields.
All of the fields that were removed were no longer referenced except for
'lastrxts' and 'rxseqno' which were only ever written to.

Change-Id: I5a5d31eb33e97663843698f58d0d97f22a76627c
2018-12-14 12:57:06 -05:00
Joshua C. Colp a28f0382e8 Merge "Use non-blocking socket() and pipe() wrappers" 2018-12-12 11:31:00 -06:00
George Joseph 5d4d723844 Merge "Revert "RTP: reset DTMF last seqno/timestamp on voice packet with marker bit"" 2018-12-11 14:18:25 -06:00
Sean Bright 42ff856216 Use non-blocking socket() and pipe() wrappers
Change-Id: I050ceffe5a133d5add2dab46687209813d58f597
2018-12-11 12:29:09 -05:00
George Joseph d1598dbc7d Revert "RTP: reset DTMF last seqno/timestamp on voice packet with marker bit"
This reverts commit 3f53041267.

Pending resolution of ASTERISK_28200

Change-Id: Iad4f3614cac95b00fdbb2b799aab8ae6285ec988
2018-12-11 09:28:48 -05:00
Joshua Colp b80c9071e3 Merge "RTP: need to reset DTMF last seqno/timestamp on voice packet with marker bit" 2018-11-26 13:47:32 -06:00
Alexei Gradinari 3f53041267 RTP: need to reset DTMF last seqno/timestamp on voice packet with marker bit
The marker bit set on the voice packet indicates the start
of a new stream and a new time stamp.
Need to reset the DTMF last sequence number and the timestamp
of the last END packet.

If the new time stamp is lower then the timestamp of the last DTMF END packet
the asterisk drops all DTMF frames as out of order.

This bug was caught using Cisco ip-phone SPA50X and codec g722.
On SIP session update the SPA50X resets stream indicating it with market bit
and a new timestamp is twice smaller then the previous.

ASTERISK-28162 #close

Change-Id: If9c5742158fa836ad549713a9814d46a5d2b1620
2018-11-23 10:41:52 -05:00
Corey Farrell 021ce938ca
astobj2: Remove legacy ao2_container_alloc routine.
Replace usage of ao2_container_alloc with ao2_container_alloc_hash or
ao2_container_alloc_list.  Remove ao2_container_alloc macro.

Change-Id: I0907d78bc66efc775672df37c8faad00f2f6c088
2018-11-21 09:56:16 -05:00
Richard Mudgett 7ab4befc2b res_rtp_asterisk.c: Add conditional module dependency to res_pjproject
* The dependency ensures that res_pjproject cannot be manually unloaded
before res_rtp_asterisk.
* The dependency allows startup loading errors to report that
res_rtp_asterisk depends upon res_pjproject.

Change-Id: Icf5e7581f4ddd6189929f6174c74dd951f887377
2018-10-17 16:13:51 -05:00
George Joseph 9914e3998e Merge "res_rtp_asterisk.c: Add "seqno" strictrtp option" 2018-09-28 07:27:24 -05:00
Ben Ford b11a6643cf res_rtp_asterisk.c: Add "seqno" strictrtp option
When networks experience disruptions, there can be large gaps of time
between receiving packets. When strictrtp is enabled, this created
issues where a flood of packets could come in and be seen as an attack.
Another option - seqno - has been added to the strictrtp option that
ignores the time interval and goes strictly by sequence number for
validity.

Change-Id: I8a42b8d193673899c8fc22fe7f98ea87df89be71
2018-09-26 13:27:03 -05:00
Joshua Colp 8bb264841a res_rtp_asterisk: Raise event when RTP port is allocated
This change raises a testsuite event to provide what port
Asterisk has actually allocated for RTP. This ensures that
testsuite tests can remove any assumption of ports and instead
use the actual port in use.

ASTERISK-28070

Change-Id: I91bd45782e84284e01c89acf4b2da352e14ae044
2018-09-25 05:35:26 -05:00