<p>Friendly Automation <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/13592">View Change</a></p><div style="white-space:pre-wrap">Approvals:
George Joseph: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
Friendly Automation: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">chan_sip: Always process updated SDP on media source change<br><br>Fixes no-audio issues when the media source is changed and<br>strictrtp is enabled (default).<br><br>If the peer media source changes, the SDP session version also changes.<br>If it is lower than the one we had stored, chan_sip would ignore it.<br><br>This changeset keeps track of the remote media origin identifier,<br>comparing that as well. If it changes, the session version needn't be<br>higher for us to accept the SDP.<br><br>Common scenario where this would've caused problems: a separate media<br>gateway that informs the caller about premium rates before handing off<br>the call to the final destination.<br><br>(An alternative fix would be to set ignoresdpversion=yes on the peer.)<br><br>ASTERISK-28686<br><br>Change-Id: I88fdbc5aeb777b583e7738c084254c482a7776ee<br>---<br>M channels/chan_sip.c<br>M channels/sip/include/sip.h<br>2 files changed, 47 insertions(+), 18 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/channels/chan_sip.c b/channels/chan_sip.c</span><br><span>index f90d3f9..c200cc10 100644</span><br><span>--- a/channels/chan_sip.c</span><br><span>+++ b/channels/chan_sip.c</span><br><span>@@ -11184,9 +11184,12 @@</span><br><span> </span><br><span> static int process_sdp_o(const char *o, struct sip_pvt *p)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *o_copy_start;</span><br><span> char *o_copy;</span><br><span> char *token;</span><br><span style="color: hsl(0, 100%, 40%);">- int64_t rua_version;</span><br><span style="color: hsl(120, 100%, 40%);">+ int offset;</span><br><span style="color: hsl(120, 100%, 40%);">+ int64_t sess_version;</span><br><span style="color: hsl(120, 100%, 40%);">+ char unique[128];</span><br><span> </span><br><span> /* Store the SDP version number of remote UA. This will allow us to</span><br><span> distinguish between session modifications and session refreshes. If</span><br><span>@@ -11204,35 +11207,49 @@</span><br><span> return FALSE;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- o_copy = ast_strdupa(o);</span><br><span style="color: hsl(0, 100%, 40%);">- token = strsep(&o_copy, " "); /* Skip username */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* o=<username> <sess-id> <sess-version> <nettype> <addrtype></span><br><span style="color: hsl(120, 100%, 40%);">+ <unicast-address> */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ o_copy_start = o_copy = ast_strdupa(o);</span><br><span style="color: hsl(120, 100%, 40%);">+ token = strsep(&o_copy, " "); /* Skip username */</span><br><span> if (!o_copy) {</span><br><span> ast_log(LOG_WARNING, "SDP syntax error in o= line username\n");</span><br><span> return FALSE;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- token = strsep(&o_copy, " "); /* Skip session-id */</span><br><span style="color: hsl(120, 100%, 40%);">+ token = strsep(&o_copy, " "); /* sess-id */</span><br><span> if (!o_copy) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_WARNING, "SDP syntax error in o= line session-id\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_WARNING, "SDP syntax error in o= line sess-id\n");</span><br><span> return FALSE;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- token = strsep(&o_copy, " "); /* Version */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!o_copy) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_WARNING, "SDP syntax error in o= line\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return FALSE;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- if (!sscanf(token, "%30" SCNd64, &rua_version)) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_WARNING, "SDP syntax error in o= line version\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ token = strsep(&o_copy, " "); /* sess-version */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!o_copy || !sscanf(token, "%30" SCNd64, &sess_version)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_log(LOG_WARNING, "SDP syntax error in o= line sess-version\n");</span><br><span> return FALSE;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* we need to check the SDP version number the other end sent us;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Copy all after sess-version on top of sess-version into unique.</span><br><span style="color: hsl(120, 100%, 40%);">+ * <sess-id> is a numeric string such that the tuple of <username>,</span><br><span style="color: hsl(120, 100%, 40%);">+ * <sess-id>, <nettype>, <addrtype>, and <unicast-address> forms a</span><br><span style="color: hsl(120, 100%, 40%);">+ * globally unique identifier for the session.</span><br><span style="color: hsl(120, 100%, 40%);">+ * I.e. all except the <sess-version> */</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_copy_string(unique, o, sizeof(unique)); /* copy all of o= contents */</span><br><span style="color: hsl(120, 100%, 40%);">+ offset = (o_copy - o_copy_start); /* after sess-version */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (offset < sizeof(unique)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* copy all after sess-version on top of sess-version */</span><br><span style="color: hsl(120, 100%, 40%);">+ int sess_version_start = token - o_copy_start;</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_copy_string(unique + sess_version_start, o + offset, sizeof(unique) - sess_version_start);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* We need to check the SDP version number the other end sent us;</span><br><span> * our rules for deciding what to accept are a bit complex.</span><br><span> *</span><br><span> * 1) if 'ignoresdpversion' has been set for this dialog, then</span><br><span> * we will just accept whatever they sent and assume it is</span><br><span> * a modification of the session, even if it is not</span><br><span> * 2) otherwise, if this is the first SDP we've seen from them</span><br><span style="color: hsl(0, 100%, 40%);">- * we accept it</span><br><span style="color: hsl(120, 100%, 40%);">+ * we accept it;</span><br><span style="color: hsl(120, 100%, 40%);">+ * note that _them_ may change, in which case the</span><br><span style="color: hsl(120, 100%, 40%);">+ * sessionunique_remote will be different</span><br><span> * 3) otherwise, if the new SDP version number is higher than the</span><br><span> * old one, we accept it</span><br><span> * 4) otherwise, if this SDP is in response to us requesting a switch</span><br><span>@@ -11242,14 +11259,25 @@</span><br><span> * not request a switch to T.38, then we stop parsing the SDP, as it</span><br><span> * has not changed from the previous version</span><br><span> */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sip_debug_test_pvt(p)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_strlen_zero(p->sessionunique_remote)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_verbose("Got SDP version %" PRId64 " and unique parts [%s]\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ sess_version, unique);</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_verbose("Comparing SDP version %" PRId64 " -> %" PRId64 " and unique parts [%s] -> [%s]\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ p->sessionversion_remote, sess_version, p->sessionunique_remote, unique);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> if (ast_test_flag(&p->flags[1], SIP_PAGE2_IGNORESDPVERSION) ||</span><br><span style="color: hsl(0, 100%, 40%);">- (p->sessionversion_remote < 0) ||</span><br><span style="color: hsl(0, 100%, 40%);">- (p->sessionversion_remote < rua_version)) {</span><br><span style="color: hsl(0, 100%, 40%);">- p->sessionversion_remote = rua_version;</span><br><span style="color: hsl(120, 100%, 40%);">+ sess_version > p->sessionversion_remote ||</span><br><span style="color: hsl(120, 100%, 40%);">+ strcmp(unique, S_OR(p->sessionunique_remote, ""))) {</span><br><span style="color: hsl(120, 100%, 40%);">+ p->sessionversion_remote = sess_version;</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_string_field_set(p, sessionunique_remote, unique);</span><br><span> } else {</span><br><span> if (p->t38.state == T38_LOCAL_REINVITE) {</span><br><span style="color: hsl(0, 100%, 40%);">- p->sessionversion_remote = rua_version;</span><br><span style="color: hsl(120, 100%, 40%);">+ p->sessionversion_remote = sess_version;</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_string_field_set(p, sessionunique_remote, unique);</span><br><span> ast_log(LOG_WARNING, "Call %s responded to our T.38 reinvite without changing SDP version; 'ignoresdpversion' should be set for this peer.\n", p->callid);</span><br><span> } else {</span><br><span> p->session_modify = FALSE;</span><br><span>diff --git a/channels/sip/include/sip.h b/channels/sip/include/sip.h</span><br><span>index 850370c..2243d57 100644</span><br><span>--- a/channels/sip/include/sip.h</span><br><span>+++ b/channels/sip/include/sip.h</span><br><span>@@ -1045,6 +1045,7 @@</span><br><span> AST_STRING_FIELD(last_presence_message); /*!< The last presence message for a subscription */</span><br><span> AST_STRING_FIELD(msg_body); /*!< Text for a MESSAGE body */</span><br><span> AST_STRING_FIELD(tel_phone_context); /*!< The phone-context portion of a TEL URI */</span><br><span style="color: hsl(120, 100%, 40%);">+ AST_STRING_FIELD(sessionunique_remote); /*!< Remote UA's SDP Session unique parts */</span><br><span> );</span><br><span> char via[128]; /*!< Via: header */</span><br><span> int maxforwards; /*!< SIP Loop prevention */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/13592">change 13592</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/13592"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-Change-Id: I88fdbc5aeb777b583e7738c084254c482a7776ee </div>
<div style="display:none"> Gerrit-Change-Number: 13592 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Walter Doekes <walter+asterisk@wjd.nu> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-CC: Jaco Kroon <jaco@uls.co.za> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>