summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--README-SERIOUSLY.bestpractices.txt7
-rw-r--r--apps/app_minivm.c36
-rw-r--r--apps/app_mixmonitor.c15
-rw-r--r--apps/app_system.c10
-rw-r--r--configs/samples/minivm.conf.sample2
-rw-r--r--funcs/func_shell.c5
-rw-r--r--include/asterisk/app.h31
-rw-r--r--main/asterisk.c91
-rw-r--r--res/res_monitor.c13
-rw-r--r--res/res_pjsip/pjsip_message_ip_updater.c56
-rw-r--r--res/res_rtp_asterisk.c129
11 files changed, 293 insertions, 102 deletions
diff --git a/README-SERIOUSLY.bestpractices.txt b/README-SERIOUSLY.bestpractices.txt
index b6b418d9f..0d3e670cf 100644
--- a/README-SERIOUSLY.bestpractices.txt
+++ b/README-SERIOUSLY.bestpractices.txt
@@ -94,6 +94,13 @@ your ITSP in a place where you didn't expect to allow it. There are a couple of
ways in which you can mitigate this impact: stricter pattern matching, or using
the FILTER() dialplan function.
+The CALLERID(num) and CALLERID(name) values are other commonly used values that
+are sources of data potentially supplied by outside sources. If you use these
+values as parameters to the System(), MixMonitor(), or Monitor() applications
+or the SHELL() dialplan function, you can allow injection of arbitrary operating
+system command execution. The FILTER() dialplan function is available to remove
+dangerous characters from untrusted strings to block the command injection.
+
Strict Pattern Matching
-----------------------
diff --git a/apps/app_minivm.c b/apps/app_minivm.c
index 15449ad4e..0d7a5f407 100644
--- a/apps/app_minivm.c
+++ b/apps/app_minivm.c
@@ -1774,21 +1774,35 @@ static int play_record_review(struct ast_channel *chan, char *playfile, char *re
/*! \brief Run external notification for voicemail message */
static void run_externnotify(struct ast_channel *chan, struct minivm_account *vmu)
{
- char arguments[BUFSIZ];
+ char fquser[AST_MAX_CONTEXT * 2];
+ char *argv[5] = { NULL };
+ struct ast_party_caller *caller;
+ char *cid;
+ int idx;
- if (ast_strlen_zero(vmu->externnotify) && ast_strlen_zero(global_externnotify))
+ if (ast_strlen_zero(vmu->externnotify) && ast_strlen_zero(global_externnotify)) {
return;
+ }
+
+ snprintf(fquser, sizeof(fquser), "%s@%s", vmu->username, vmu->domain);
- snprintf(arguments, sizeof(arguments), "%s %s@%s %s %s&",
- ast_strlen_zero(vmu->externnotify) ? global_externnotify : vmu->externnotify,
- vmu->username, vmu->domain,
- (ast_channel_caller(chan)->id.name.valid && ast_channel_caller(chan)->id.name.str)
- ? ast_channel_caller(chan)->id.name.str : "",
- (ast_channel_caller(chan)->id.number.valid && ast_channel_caller(chan)->id.number.str)
- ? ast_channel_caller(chan)->id.number.str : "");
+ caller = ast_channel_caller(chan);
+ idx = 0;
+ argv[idx++] = ast_strlen_zero(vmu->externnotify) ? global_externnotify : vmu->externnotify;
+ argv[idx++] = fquser;
+ cid = S_COR(caller->id.name.valid, caller->id.name.str, NULL);
+ if (cid) {
+ argv[idx++] = cid;
+ }
+ cid = S_COR(caller->id.number.valid, caller->id.number.str, NULL);
+ if (cid) {
+ argv[idx++] = cid;
+ }
+ argv[idx] = NULL;
- ast_debug(1, "Executing: %s\n", arguments);
- ast_safe_system(arguments);
+ ast_debug(1, "Executing: %s %s %s %s\n",
+ argv[0], argv[1], argv[2] ?: "", argv[3] ?: "");
+ ast_safe_execvp(1, argv[0], argv);
}
/*!\internal
diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c
index 3258b301f..ac4564205 100644
--- a/apps/app_mixmonitor.c
+++ b/apps/app_mixmonitor.c
@@ -136,6 +136,11 @@
<para>Will be executed when the recording is over.</para>
<para>Any strings matching <literal>^{X}</literal> will be unescaped to <variable>X</variable>.</para>
<para>All variables will be evaluated at the time MixMonitor is called.</para>
+ <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable>
+ or <variable>CALLERID(name)</variable> as part of the command parameters. You
+ risk a command injection attack executing arbitrary commands if the untrusted
+ strings aren't filtered to remove dangerous characters. See function
+ <variable>FILTER()</variable>.</para></warning>
</parameter>
</syntax>
<description>
@@ -148,6 +153,11 @@
<para>Will contain the filename used to record.</para>
</variable>
</variablelist>
+ <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable>
+ or <variable>CALLERID(name)</variable> as part of ANY of the application's
+ parameters. You risk a command injection attack executing arbitrary commands
+ if the untrusted strings aren't filtered to remove dangerous characters. See
+ function <variable>FILTER()</variable>.</para></warning>
</description>
<see-also>
<ref type="application">Monitor</ref>
@@ -222,6 +232,11 @@
<para>Will be executed when the recording is over.
Any strings matching <literal>^{X}</literal> will be unescaped to <variable>X</variable>.
All variables will be evaluated at the time MixMonitor is called.</para>
+ <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable>
+ or <variable>CALLERID(name)</variable> as part of the command parameters. You
+ risk a command injection attack executing arbitrary commands if the untrusted
+ strings aren't filtered to remove dangerous characters. See function
+ <variable>FILTER()</variable>.</para></warning>
</parameter>
</syntax>
<description>
diff --git a/apps/app_system.c b/apps/app_system.c
index 09179f7f7..64d529798 100644
--- a/apps/app_system.c
+++ b/apps/app_system.c
@@ -46,6 +46,11 @@
<syntax>
<parameter name="command" required="true">
<para>Command to execute</para>
+ <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable>
+ or <variable>CALLERID(name)</variable> as part of the command parameters. You
+ risk a command injection attack executing arbitrary commands if the untrusted
+ strings aren't filtered to remove dangerous characters. See function
+ <variable>FILTER()</variable>.</para></warning>
</parameter>
</syntax>
<description>
@@ -71,6 +76,11 @@
<syntax>
<parameter name="command" required="true">
<para>Command to execute</para>
+ <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable>
+ or <variable>CALLERID(name)</variable> as part of the command parameters. You
+ risk a command injection attack executing arbitrary commands if the untrusted
+ strings aren't filtered to remove dangerous characters. See function
+ <variable>FILTER()</variable>.</para></warning>
</parameter>
</syntax>
<description>
diff --git a/configs/samples/minivm.conf.sample b/configs/samples/minivm.conf.sample
index 2df3449d1..79fdbb0e2 100644
--- a/configs/samples/minivm.conf.sample
+++ b/configs/samples/minivm.conf.sample
@@ -51,7 +51,7 @@ silencethreshold=128
; If you need to have an external program, i.e. /usr/bin/myapp called when a
; voicemail is received by the server. The arguments are
;
-; <app> <username@domain> <callerid-number> <callerid-name>
+; <app> <username@domain> <callerid-name> <callerid-number>
;
;externnotify=/usr/bin/myapp
; The character set for voicemail messages can be specified here
diff --git a/funcs/func_shell.c b/funcs/func_shell.c
index 0398cd839..fe1debe88 100644
--- a/funcs/func_shell.c
+++ b/funcs/func_shell.c
@@ -82,6 +82,11 @@ static int shell_helper(struct ast_channel *chan, const char *cmd, char *data,
<syntax>
<parameter name="command" required="true">
<para>The command that the shell should execute.</para>
+ <warning><para>Do not use untrusted strings such as <variable>CALLERID(num)</variable>
+ or <variable>CALLERID(name)</variable> as part of the command parameters. You
+ risk a command injection attack executing arbitrary commands if the untrusted
+ strings aren't filtered to remove dangerous characters. See function
+ <variable>FILTER()</variable>.</para></warning>
</parameter>
</syntax>
<description>
diff --git a/include/asterisk/app.h b/include/asterisk/app.h
index d86b63338..0505a6b98 100644
--- a/include/asterisk/app.h
+++ b/include/asterisk/app.h
@@ -871,9 +871,34 @@ int ast_vm_test_destroy_user(const char *context, const char *mailbox);
int ast_vm_test_create_user(const char *context, const char *mailbox);
#endif
-/*! \brief Safely spawn an external program while closing file descriptors
- \note This replaces the \b system call in all Asterisk modules
-*/
+/*!
+ * \brief Safely spawn an external program while closing file descriptors
+ *
+ * \note This replaces the \b execvp call in all Asterisk modules
+ *
+ * \param dualfork Non-zero to simulate running the program in the
+ * background by forking twice. The option provides similar
+ * functionality to the '&' in the OS shell command "cmd &". The
+ * option allows Asterisk to run a reaper loop to watch the first fork
+ * which immediately exits after spaning the second fork. The actual
+ * program is run in the second fork.
+ * \param file execvp(file, argv) file parameter
+ * \param argv execvp(file, argv) argv parameter
+ */
+int ast_safe_execvp(int dualfork, const char *file, char *const argv[]);
+
+/*!
+ * \brief Safely spawn an OS shell command while closing file descriptors
+ *
+ * \note This replaces the \b system call in all Asterisk modules
+ *
+ * \param s - OS shell command string to execute.
+ *
+ * \warning Command injection can happen using this call if the passed
+ * in string is created using untrusted data from an external source.
+ * It is best not to use untrusted data. However, the caller could
+ * filter out dangerous characters to avoid command injection.
+ */
int ast_safe_system(const char *s);
/*!
diff --git a/main/asterisk.c b/main/asterisk.c
index 0a131fda9..d55594983 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -1170,11 +1170,10 @@ void ast_unreplace_sigchld(void)
ast_mutex_unlock(&safe_system_lock);
}
-int ast_safe_system(const char *s)
+/*! \brief fork and perform other preparations for spawning applications */
+static pid_t safe_exec_prep(int dualfork)
{
pid_t pid;
- int res;
- int status;
#if defined(HAVE_WORKING_FORK) || defined(HAVE_WORKING_VFORK)
ast_replace_sigchld();
@@ -1196,35 +1195,101 @@ int ast_safe_system(const char *s)
cap_free(cap);
#endif
#ifdef HAVE_WORKING_FORK
- if (ast_opt_high_priority)
+ if (ast_opt_high_priority) {
ast_set_priority(0);
+ }
/* Close file descriptors and launch system command */
ast_close_fds_above_n(STDERR_FILENO);
#endif
- execl("/bin/sh", "/bin/sh", "-c", s, (char *) NULL);
- _exit(1);
- } else if (pid > 0) {
+ if (dualfork) {
+#ifdef HAVE_WORKING_FORK
+ pid = fork();
+#else
+ pid = vfork();
+#endif
+ if (pid < 0) {
+ /* Second fork failed. */
+ /* No logger available. */
+ _exit(1);
+ }
+
+ if (pid > 0) {
+ /* This is the first fork, exit so the reaper finishes right away. */
+ _exit(0);
+ }
+
+ /* This is the second fork. The first fork will exit immediately so
+ * Asterisk doesn't have to wait for completion.
+ * ast_safe_system("cmd &") would run in the background, but the '&'
+ * cannot be added with ast_safe_execvp, so we have to double fork.
+ */
+ }
+ }
+
+ if (pid < 0) {
+ ast_log(LOG_WARNING, "Fork failed: %s\n", strerror(errno));
+ }
+#else
+ ast_log(LOG_WARNING, "Fork failed: %s\n", strerror(ENOTSUP));
+ pid = -1;
+#endif
+
+ return pid;
+}
+
+/*! \brief wait for spawned application to complete and unreplace sigchld */
+static int safe_exec_wait(pid_t pid)
+{
+ int res = -1;
+
+#if defined(HAVE_WORKING_FORK) || defined(HAVE_WORKING_VFORK)
+ if (pid > 0) {
for (;;) {
+ int status;
+
res = waitpid(pid, &status, 0);
if (res > -1) {
res = WIFEXITED(status) ? WEXITSTATUS(status) : -1;
break;
- } else if (errno != EINTR)
+ }
+ if (errno != EINTR) {
break;
+ }
}
- } else {
- ast_log(LOG_WARNING, "Fork failed: %s\n", strerror(errno));
- res = -1;
}
ast_unreplace_sigchld();
-#else /* !defined(HAVE_WORKING_FORK) && !defined(HAVE_WORKING_VFORK) */
- res = -1;
#endif
return res;
}
+int ast_safe_execvp(int dualfork, const char *file, char *const argv[])
+{
+ pid_t pid = safe_exec_prep(dualfork);
+
+ if (pid == 0) {
+ execvp(file, argv);
+ _exit(1);
+ /* noreturn from _exit */
+ }
+
+ return safe_exec_wait(pid);
+}
+
+int ast_safe_system(const char *s)
+{
+ pid_t pid = safe_exec_prep(0);
+
+ if (pid == 0) {
+ execl("/bin/sh", "/bin/sh", "-c", s, (char *) NULL);
+ _exit(1);
+ /* noreturn from _exit */
+ }
+
+ return safe_exec_wait(pid);
+}
+
/*!
* \brief enable or disable a logging level to a specified console
*/
diff --git a/res/res_monitor.c b/res/res_monitor.c
index fd3ff7a1c..3e3611b36 100644
--- a/res/res_monitor.c
+++ b/res/res_monitor.c
@@ -59,17 +59,17 @@
<syntax>
<parameter name="file_format" argsep=":">
<argument name="file_format" required="true">
- <para>optional, if not set, defaults to <literal>wav</literal></para>
+ <para>Optional. If not set, defaults to <literal>wav</literal></para>
</argument>
<argument name="urlbase" />
</parameter>
<parameter name="fname_base">
- <para>if set, changes the filename used to the one specified.</para>
+ <para>If set, changes the filename used to the one specified.</para>
</parameter>
<parameter name="options">
<optionlist>
<option name="m">
- <para>when the recording ends mix the two leg files into one and
+ <para>When the recording ends mix the two leg files into one and
delete the two leg files. If the variable <variable>MONITOR_EXEC</variable>
is set, the application referenced in it will be executed instead of
soxmix/sox and the raw leg files will NOT be deleted automatically.
@@ -80,6 +80,13 @@
will be passed on as additional arguments to <variable>MONITOR_EXEC</variable>.
Both <variable>MONITOR_EXEC</variable> and the Mix flag can be set from the
administrator interface.</para>
+ <warning><para>Do not use untrusted strings such as
+ <variable>CALLERID(num)</variable> or <variable>CALLERID(name)</variable>
+ as part of <variable>MONITOR_EXEC</variable> or
+ <variable>MONITOR_EXEC_ARGS</variable>. You risk a command injection
+ attack executing arbitrary commands if the untrusted strings aren't
+ filtered to remove dangerous characters. See function
+ <variable>FILTER()</variable>.</para></warning>
</option>
<option name="b">
<para>Don't begin recording unless a call is bridged to another channel.</para>
diff --git a/res/res_pjsip/pjsip_message_ip_updater.c b/res/res_pjsip/pjsip_message_ip_updater.c
index 2d074640a..099ecaa66 100644
--- a/res/res_pjsip/pjsip_message_ip_updater.c
+++ b/res/res_pjsip/pjsip_message_ip_updater.c
@@ -153,7 +153,16 @@ static int multihomed_rewrite_sdp(struct pjmedia_sdp_session *sdp)
return 0;
}
-static void sanitize_tdata(pjsip_tx_data *tdata)
+#define is_sip_uri(uri) \
+ (PJSIP_URI_SCHEME_IS_SIP(uri) || PJSIP_URI_SCHEME_IS_SIPS(uri))
+
+#ifdef AST_DEVMODE
+#define FUNC_ATTRS __attribute__ ((noinline))
+#else
+#define FUNC_ATTRS
+#endif
+
+static void FUNC_ATTRS sanitize_tdata(pjsip_tx_data *tdata)
{
static const pj_str_t x_name = { AST_SIP_X_AST_TXP, AST_SIP_X_AST_TXP_LEN };
pjsip_param *x_transport;
@@ -161,29 +170,50 @@ static void sanitize_tdata(pjsip_tx_data *tdata)
pjsip_fromto_hdr *fromto;
pjsip_contact_hdr *contact;
pjsip_hdr *hdr;
+#ifdef AST_DEVMODE
+ char hdrbuf[512];
+ int hdrbuf_len;
+#endif
if (tdata->msg->type == PJSIP_REQUEST_MSG) {
- uri = pjsip_uri_get_uri(tdata->msg->line.req.uri);
- x_transport = pjsip_param_find(&uri->other_param, &x_name);
- if (x_transport) {
- pj_list_erase(x_transport);
+ if (is_sip_uri(tdata->msg->line.req.uri)) {
+ uri = pjsip_uri_get_uri(tdata->msg->line.req.uri);
+#ifdef AST_DEVMODE
+ hdrbuf_len = pjsip_uri_print(PJSIP_URI_IN_REQ_URI, uri, hdrbuf, 512);
+ ast_debug(2, "Sanitizing Request: %s\n", hdrbuf);
+#endif
+ while ((x_transport = pjsip_param_find(&uri->other_param, &x_name))) {
+ pj_list_erase(x_transport);
+ }
}
}
for (hdr = tdata->msg->hdr.next; hdr != &tdata->msg->hdr; hdr = hdr->next) {
if (hdr->type == PJSIP_H_TO || hdr->type == PJSIP_H_FROM) {
fromto = (pjsip_fromto_hdr *) hdr;
- uri = pjsip_uri_get_uri(fromto->uri);
- x_transport = pjsip_param_find(&uri->other_param, &x_name);
- if (x_transport) {
- pj_list_erase(x_transport);
+ if (is_sip_uri(fromto->uri)) {
+ uri = pjsip_uri_get_uri(fromto->uri);
+#ifdef AST_DEVMODE
+ hdrbuf_len = pjsip_uri_print(PJSIP_URI_IN_FROMTO_HDR, uri, hdrbuf, 512);
+ hdrbuf[hdrbuf_len] = '\0';
+ ast_debug(2, "Sanitizing From/To: %s\n", hdrbuf);
+#endif
+ while ((x_transport = pjsip_param_find(&uri->other_param, &x_name))) {
+ pj_list_erase(x_transport);
+ }
}
} else if (hdr->type == PJSIP_H_CONTACT) {
contact = (pjsip_contact_hdr *) hdr;
- uri = pjsip_uri_get_uri(contact->uri);
- x_transport = pjsip_param_find(&uri->other_param, &x_name);
- if (x_transport) {
- pj_list_erase(x_transport);
+ if (is_sip_uri(contact->uri)) {
+ uri = pjsip_uri_get_uri(contact->uri);
+#ifdef AST_DEVMODE
+ hdrbuf_len = pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, uri, hdrbuf, 512);
+ hdrbuf[hdrbuf_len] = '\0';
+ ast_debug(2, "Sanitizing Contact: %s\n", hdrbuf);
+#endif
+ while ((x_transport = pjsip_param_find(&uri->other_param, &x_name))) {
+ pj_list_erase(x_transport);
+ }
}
}
}
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index ab8038422..091533e19 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -219,8 +219,9 @@ static AST_RWLIST_HEAD_STATIC(host_candidates, ast_ice_host_candidate);
/*! \brief RTP learning mode tracking information */
struct rtp_learning_info {
- int max_seq; /*!< The highest sequence number received */
- int packets; /*!< The number of remaining packets before the source is accepted */
+ int max_seq; /*!< The highest sequence number received */
+ int packets; /*!< The number of remaining packets before the source is accepted */
+ struct timeval received; /*!< The time of the last received packet */
};
#ifdef HAVE_OPENSSL_SRTP
@@ -328,7 +329,6 @@ struct ast_rtp {
* but these are in place to keep learning mode sequence values sealed from their normal counterparts.
*/
struct rtp_learning_info rtp_source_learn; /* Learning mode track for the expected RTP source */
- struct rtp_learning_info alt_source_learn; /* Learning mode tracking for a new RTP source after one has been chosen */
struct rtp_red *red;
@@ -2823,6 +2823,7 @@ static void rtp_learning_seq_init(struct rtp_learning_info *info, uint16_t seq)
{
info->max_seq = seq - 1;
info->packets = learning_min_sequential;
+ memset(&info->received, 0, sizeof(info->received));
}
/*!
@@ -2837,6 +2838,13 @@ static void rtp_learning_seq_init(struct rtp_learning_info *info, uint16_t seq)
*/
static int rtp_learning_rtp_seq_update(struct rtp_learning_info *info, uint16_t seq)
{
+ if (!ast_tvzero(info->received) && ast_tvdiff_ms(ast_tvnow(), info->received) < 5) {
+ /* During the probation period the minimum amount of media we'll accept is
+ * 10ms so give a reasonable 5ms buffer just in case we get it sporadically.
+ */
+ return 1;
+ }
+
if (seq == info->max_seq + 1) {
/* packet is in sequence */
info->packets--;
@@ -2845,6 +2853,7 @@ static int rtp_learning_rtp_seq_update(struct rtp_learning_info *info, uint16_t
info->packets = learning_min_sequential - 1;
}
info->max_seq = seq;
+ info->received = ast_tvnow();
return (info->packets == 0);
}
@@ -3110,7 +3119,6 @@ static int rtp_allocate_transport(struct ast_rtp_instance *instance, struct ast_
rtp->strict_rtp_state = (strictrtp ? STRICT_RTP_LEARN : STRICT_RTP_OPEN);
if (strictrtp) {
rtp_learning_seq_init(&rtp->rtp_source_learn, (uint16_t)rtp->seqno);
- rtp_learning_seq_init(&rtp->alt_source_learn, (uint16_t)rtp->seqno);
}
/* Create a new socket for us to listen on and use */
@@ -4775,17 +4783,6 @@ static struct ast_frame *ast_rtcp_interpret(struct ast_rtp_instance *instance, c
packetwords = size / 4;
- if (ast_rtp_instance_get_prop(transport, AST_RTP_PROPERTY_NAT)) {
- /* Send to whoever sent to us */
- if (ast_sockaddr_cmp(&transport_rtp->rtcp->them, addr)) {
- ast_sockaddr_copy(&transport_rtp->rtcp->them, addr);
- if (rtpdebug) {
- ast_debug(0, "RTCP NAT: Got RTCP from other end. Now sending to address %s\n",
- ast_sockaddr_stringify(&transport_rtp->rtcp->them));
- }
- }
- }
-
ast_debug(1, "Got RTCP report of %zu bytes\n", size);
while (position < packetwords) {
@@ -4841,6 +4838,25 @@ static struct ast_frame *ast_rtcp_interpret(struct ast_rtp_instance *instance, c
rtp = transport_rtp;
}
+ if ((rtp->strict_rtp_state != STRICT_RTP_OPEN) && (rtcp_report->ssrc != rtp->themssrc)) {
+ /* Skip over this RTCP record as it does not contain the correct SSRC */
+ position += (length + 1);
+ ast_debug(1, "%p -- Received RTCP report from %s, dropping due to strict RTP protection. Received SSRC '%u' but expected '%u'\n",
+ rtp, ast_sockaddr_stringify(addr), rtcp_report->ssrc, rtp->themssrc);
+ continue;
+ }
+
+ if (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_NAT)) {
+ /* Send to whoever sent to us */
+ if (ast_sockaddr_cmp(&rtp->rtcp->them, addr)) {
+ ast_sockaddr_copy(&rtp->rtcp->them, addr);
+ if (rtpdebug) {
+ ast_debug(0, "RTCP NAT: Got RTCP from other end. Now sending to address %s\n",
+ ast_sockaddr_stringify(&rtp->rtcp->them));
+ }
+ }
+ }
+
i += 2; /* Advance past header and ssrc */
switch (pt) {
case RTCP_PT_SR:
@@ -5297,39 +5313,54 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc
return &ast_null_frame;
}
- /* If strict RTP protection is enabled see if we need to learn the remote address or if we need to drop the packet */
- if (rtp->strict_rtp_state == STRICT_RTP_LEARN) {
- ast_debug(1, "%p -- Probation learning mode pass with source address %s\n", rtp, ast_sockaddr_stringify(&addr));
- /* For now, we always copy the address. */
- ast_sockaddr_copy(&rtp->strict_rtp_address, &addr);
-
- /* Send the rtp and the seqno from header to rtp_learning_rtp_seq_update to see whether we can exit or not*/
- if (rtp_learning_rtp_seq_update(&rtp->rtp_source_learn, seqno)) {
- ast_debug(1, "%p -- Probation at seq %d with %d to go; discarding frame\n",
- rtp, rtp->rtp_source_learn.max_seq, rtp->rtp_source_learn.packets);
- return &ast_null_frame;
- }
+ /* If the version is not what we expected by this point then just drop the packet */
+ if (version != 2) {
+ return &ast_null_frame;
+ }
- ast_verb(4, "%p -- Probation passed - setting RTP source address to %s\n", rtp, ast_sockaddr_stringify(&addr));
- rtp->strict_rtp_state = STRICT_RTP_CLOSED;
+ /* We use the SSRC to determine what RTP instance this packet is actually for */
+ ssrc = ntohl(rtpheader[2]);
+
+ /* Determine the appropriate instance for this */
+ child = rtp_find_instance_by_ssrc(instance, rtp, ssrc);
+ if (child != instance) {
+ /* It is safe to hold the child lock while holding the parent lock, we guarantee that the locking order
+ * is always parent->child or that the child lock is not held when acquiring the parent lock.
+ */
+ ao2_lock(child);
+ instance = child;
+ rtp = ast_rtp_instance_get_data(instance);
+ } else {
+ /* The child is the parent! We don't need to unlock it. */
+ child = NULL;
}
- if (rtp->strict_rtp_state == STRICT_RTP_CLOSED) {
+
+ /* If strict RTP protection is enabled see if we need to learn the remote address or if we need to drop the packet */
+ if (rtp->strict_rtp_state == STRICT_RTP_LEARN) {
if (!ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) {
- /* Always reset the alternate learning source */
- rtp_learning_seq_init(&rtp->alt_source_learn, seqno);
+ /* We are learning a new address but have received traffic from the existing address,
+ * accept it but reset the current learning for the new source so it only takes over
+ * once sufficient traffic has been received. */
+ rtp_learning_seq_init(&rtp->rtp_source_learn, seqno);
} else {
/* Start trying to learn from the new address. If we pass a probationary period with
* it, that means we've stopped getting RTP from the original source and we should
* switch to it.
*/
- if (rtp_learning_rtp_seq_update(&rtp->alt_source_learn, seqno)) {
+ if (rtp_learning_rtp_seq_update(&rtp->rtp_source_learn, seqno)) {
ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection. Will switch to it in %d packets\n",
- rtp, ast_sockaddr_stringify(&addr), rtp->alt_source_learn.packets);
+ rtp, ast_sockaddr_stringify(&addr), rtp->rtp_source_learn.packets);
return &ast_null_frame;
}
- ast_verb(4, "%p -- Switching RTP source address to %s\n", rtp, ast_sockaddr_stringify(&addr));
ast_sockaddr_copy(&rtp->strict_rtp_address, &addr);
+
+ ast_verb(4, "%p -- Probation passed - setting RTP source address to %s\n", rtp, ast_sockaddr_stringify(&addr));
+ rtp->strict_rtp_state = STRICT_RTP_CLOSED;
}
+ } else if (rtp->strict_rtp_state == STRICT_RTP_CLOSED && ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) {
+ ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection.\n",
+ rtp, ast_sockaddr_stringify(&addr));
+ return &ast_null_frame;
}
/* If symmetric RTP is enabled see if the remote side is not what we expected and change where we are sending audio */
@@ -5350,28 +5381,6 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc
}
}
- /* If the version is not what we expected by this point then just drop the packet */
- if (version != 2) {
- return &ast_null_frame;
- }
-
- /* We use the SSRC to determine what RTP instance this packet is actually for */
- ssrc = ntohl(rtpheader[2]);
-
- /* Determine the appropriate instance for this */
- child = rtp_find_instance_by_ssrc(instance, rtp, ssrc);
- if (child != instance) {
- /* It is safe to hold the child lock while holding the parent lock, we guarantee that the locking order
- * is always parent->child or that the child lock is not held when acquiring the parent lock.
- */
- ao2_lock(child);
- instance = child;
- rtp = ast_rtp_instance_get_data(instance);
- } else {
- /* The child is the parent! We don't need to unlock it. */
- child = NULL;
- }
-
/* If we are currently sending DTMF to the remote party send a continuation packet */
if (rtp->sending_digit) {
ast_rtp_dtmf_continuation(instance);
@@ -5883,7 +5892,11 @@ static void ast_rtp_remote_address_set(struct ast_rtp_instance *instance, struct
rtp->rxseqno = 0;
- if (strictrtp && rtp->strict_rtp_state != STRICT_RTP_OPEN) {
+ if (strictrtp && rtp->strict_rtp_state != STRICT_RTP_OPEN && !ast_sockaddr_isnull(addr) &&
+ ast_sockaddr_cmp(addr, &rtp->strict_rtp_address)) {
+ /* We only need to learn a new strict source address if we've been told the source is
+ * changing to something different.
+ */
rtp->strict_rtp_state = STRICT_RTP_LEARN;
rtp_learning_seq_init(&rtp->rtp_source_learn, rtp->seqno);
}
generated by cgit v1.2.3 (git 2.39.1) at 2024-07-22 00:29:48 +0000