res_pjsip: return all codecs on a re-INVITE without SDP

Currently chan_pjsip on receiving a re-INVITE without SDP will only
return the codecs that are previously negotiated and not offering
all enabled codecs.

This causes interoperability issues with different equipment (e.g.
from Cisco) for some of our customers and probably also in other
scenarios involving 3PCC infrastructure.

According to RFC 3261, section 14.2 we SHOULD return all codecs
on a re-INVITE without SDP

The PR proposes a new parameter to configure this behaviour:
all_codecs_on_empty_reinvite. It includes the code, documentation,
alembic migrations, CHANGES file and example configuration additions.

ASTERISK-30193 #close

Change-Id: I69763708d5039d512f391e296ee8a4d43a1e2148

diff --git a/configs/samples/pjsip.conf.sample b/configs/samples/pjsip.conf.sample
index 6fbdb8b934b8d000e63fcf5f204eb6e4cd8ffba1..b6a9cd0e6f1c96bd59e9404dda05f1d06ed97a48 100644 (file)
--- a/configs/samples/pjsip.conf.sample
+++ b/configs/samples/pjsip.conf.sample
@@ -1336,6 +1336,13 @@
                     ; creating an implicit subscription (see RFC 4488).
                     ; (default: "yes")
 
+;all_codecs_on_empty_reinvite=yes
+                    ; On reception of a re-INVITE without SDP Asterisk will send an SDP
+                    ; offer in the 200 OK response containing all configured codecs on the
+                    ; endpoint, instead of simply those that have already been negotiated.
+                    ; RFC 3261 specifies this as a SHOULD requirement.
+                    ; (default: "no")
+
 ;allow_sending_180_after_183=yes       ; Allow Asterisk to send 180 Ringing to an endpoint
                                        ; after 183 Session Progress has been send.
                                        ; If disabled Asterisk will instead send only a

diff --git a/contrib/ast-db-manage/config/versions/ccf795ee535f_all_codecs_on_empty_reinvite.py b/contrib/ast-db-manage/config/versions/ccf795ee535f_all_codecs_on_empty_reinvite.py
new file mode 100644 (file)
index 0000000..125684f
--- /dev/null
+++ b/contrib/ast-db-manage/config/versions/ccf795ee535f_all_codecs_on_empty_reinvite.py
@@ -0,0 +1,37 @@
+"""all_codecs_on_empty_reinvite
+
+Revision ID: ccf795ee535f
+Revises: 539f68bede2c
+Create Date: 2022-09-28 09:14:36.709781
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'ccf795ee535f'
+down_revision = '417c0247fd7e'
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects.postgresql import ENUM
+
+AST_BOOL_NAME = 'ast_bool_values'
+# We'll just ignore the n/y and f/t abbreviations as Asterisk does not write
+# those aliases.
+AST_BOOL_VALUES = [ '0', '1',
+                    'off', 'on',
+                    'false', 'true',
+                    'no', 'yes' ]
+
+def upgrade():
+    ############################# Enums ##############################
+
+    # ast_bool_values has already been created, so use postgres enum object
+    # type to get around "already created" issue - works okay with mysql
+    ast_bool_values = ENUM(*AST_BOOL_VALUES, name=AST_BOOL_NAME, create_type=False)
+
+    op.add_column('ps_globals', sa.Column('all_codecs_on_empty_reinvite', ast_bool_values))
+
+def downgrade():
+    if op.get_context().bind.dialect.name == 'mssql':
+        op.drop_constraint('ck_ps_globals_all_codecs_on_empty_reinvite_ast_bool_values', 'ps_globals')
+    op.drop_column('ps_globals', 'all_codecs_on_empty_reinvite')

diff --git a/doc/CHANGES-staging/res_pjsip_all_codecs_on_empty_reinvite_option.txt b/doc/CHANGES-staging/res_pjsip_all_codecs_on_empty_reinvite_option.txt
new file mode 100644 (file)
index 0000000..99eccbb
--- /dev/null
+++ b/doc/CHANGES-staging/res_pjsip_all_codecs_on_empty_reinvite_option.txt
@@ -0,0 +1,8 @@
+Subject: res_pjsip
+
+A new option named "all_codecs_on_empty_reinvite" has been added to the
+global section. When this option is enabled, on reception of a re-INVITE
+without SDP, Asterisk will send an SDP offer in the 200 OK response containing
+all configured codecs on the endpoint, instead of simply those that have
+already been negotiated. RFC 3261 specifies this as a SHOULD requirement.
+The default value is "off".
\ No newline at end of file

diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 743b19fe1b9d75bbcdebe02058fb15df6807a910..d207297e9fe72427c9eec4071aa1a16521492809 100644 (file)
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -3902,4 +3902,11 @@ const pj_str_t *ast_sip_pjsip_uri_get_hostname(pjsip_uri *uri);
  */
 struct pjsip_param *ast_sip_pjsip_uri_get_other_param(pjsip_uri *uri, const pj_str_t *param_str);
 
+/*!
+ * \brief Retrieve the system setting 'all_codecs_on_empty_reinvite'.
+ *
+ * \retval non zero if we should return all codecs on empty re-INVITE
+ */
+unsigned int ast_sip_get_all_codecs_on_empty_reinvite(void);
+
 #endif /* _RES_PJSIP_H */

diff --git a/res/res_pjsip/config_global.c b/res/res_pjsip/config_global.c
index 4620c3f544932132ced2ebd382bb71d57ad927b0..5a71b485b4161f1873ab970c8650d31e3d6d9fb9 100644 (file)
--- a/res/res_pjsip/config_global.c
+++ b/res/res_pjsip/config_global.c
@@ -54,6 +54,7 @@
 #define DEFAULT_SEND_CONTACT_STATUS_ON_UPDATE_REGISTRATION 0
 #define DEFAULT_TASKPROCESSOR_OVERLOAD_TRIGGER TASKPROCESSOR_OVERLOAD_TRIGGER_GLOBAL
 #define DEFAULT_NOREFERSUB 1
+#define DEFAULT_ALL_CODECS_ON_EMPTY_REINVITE 0
 
 /*!
  * \brief Cached global config object
@@ -119,6 +120,8 @@ struct global_config {
        enum ast_sip_taskprocessor_overload_trigger overload_trigger;
        /*! Nonzero if norefersub is to be sent in Supported header */
        unsigned int norefersub;
+       /*! Nonzero if we should return all codecs on empty re-INVITE */
+       unsigned int all_codecs_on_empty_reinvite;
 };
 
 static void global_destructor(void *obj)
@@ -537,6 +540,21 @@ unsigned int ast_sip_get_norefersub(void)
        return norefersub;
 }
 
+unsigned int ast_sip_get_all_codecs_on_empty_reinvite(void)
+{
+       unsigned int all_codecs_on_empty_reinvite;
+       struct global_config *cfg;
+
+       cfg = get_global_cfg();
+       if (!cfg) {
+               return DEFAULT_ALL_CODECS_ON_EMPTY_REINVITE;
+       }
+
+       all_codecs_on_empty_reinvite = cfg->all_codecs_on_empty_reinvite;
+       ao2_ref(cfg, -1);
+       return all_codecs_on_empty_reinvite;
+}
+
 static int overload_trigger_handler(const struct aco_option *opt,
        struct ast_variable *var, void *obj)
 {
@@ -744,6 +762,9 @@ int ast_sip_initialize_sorcery_global(void)
        ast_sorcery_object_field_register(sorcery, "global", "norefersub",
                DEFAULT_NOREFERSUB ? "yes" : "no",
                OPT_YESNO_T, 1, FLDSET(struct global_config, norefersub));
+       ast_sorcery_object_field_register(sorcery, "global", "all_codecs_on_empty_reinvite",
+               DEFAULT_ALL_CODECS_ON_EMPTY_REINVITE ? "yes" : "no",
+               OPT_BOOL_T, 1, FLDSET(struct global_config, all_codecs_on_empty_reinvite));
 
        if (ast_sorcery_instance_observer_add(sorcery, &observer_callbacks_global)) {
                return -1;

diff --git a/res/res_pjsip/pjsip_config.xml b/res/res_pjsip/pjsip_config.xml
index 9e3b17d24be30675a50fafdf1d9bba5c50fdbade..7ef3c30ed4736afe6b71b54324a17555fd373ca6 100644 (file)
--- a/res/res_pjsip/pjsip_config.xml
+++ b/res/res_pjsip/pjsip_config.xml
@@ -2435,6 +2435,15 @@
                                                </para>
                                        </description>
                                </configOption>
+                               <configOption name="all_codecs_on_empty_reinvite" default="no">
+                                       <synopsis>If we should return all codecs on re-INVITE without SDP</synopsis>
+                                       <description><para>
+                                               On reception of a re-INVITE without SDP Asterisk will send an SDP
+                                               offer in the 200 OK response containing all configured codecs on the
+                                               endpoint, instead of simply those that have already been negotiated.
+                                               RFC 3261 specifies this as a SHOULD requirement.
+                                       </para></description>
+                               </configOption>
                        </configObject>
                </configFile>
        </configInfo>

diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index fa3d813135d506533b684119e3481f846ec21bad..c5890e0390c57607c7113cb6c6b3aa9d2252273e 100644 (file)
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -102,7 +102,7 @@ struct sdp_handler_list {
        char stream_type[1];
 };
 
-static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer);
+static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer, const unsigned int ignore_active_stream_topology);
 
 static int sdp_handler_list_hash(const void *obj, int flags)
 {
@@ -1631,7 +1631,7 @@ static pjmedia_sdp_session *generate_session_refresh_sdp(struct ast_sip_session
                        pjmedia_sdp_neg_get_active_local(inv_session->neg, &previous_sdp);
                }
        }
-       SCOPE_EXIT_RTN_VALUE(create_local_sdp(inv_session, session, previous_sdp));
+       SCOPE_EXIT_RTN_VALUE(create_local_sdp(inv_session, session, previous_sdp, 0));
 }
 
 static void set_from_header(struct ast_sip_session *session)
@@ -2556,7 +2556,7 @@ int ast_sip_session_regenerate_answer(struct ast_sip_session *session,
                pjmedia_sdp_neg_set_remote_offer(inv_session->pool, inv_session->neg, previous_offer);
        }
 
-       new_answer = create_local_sdp(inv_session, session, previous_offer);
+       new_answer = create_local_sdp(inv_session, session, previous_offer, 0);
        if (!new_answer) {
                ast_log(LOG_WARNING, "Could not create a new local SDP answer for channel '%s'\n",
                        ast_channel_name(session->channel));
@@ -2850,7 +2850,7 @@ int ast_sip_session_create_invite(struct ast_sip_session *session, pjsip_tx_data
        pjmedia_sdp_session *offer;
        SCOPE_ENTER(1, "%s\n", ast_sip_session_get_name(session));
 
-       if (!(offer = create_local_sdp(session->inv_session, session, NULL))) {
+       if (!(offer = create_local_sdp(session->inv_session, session, NULL, 0))) {
                pjsip_inv_terminate(session->inv_session, 500, PJ_FALSE);
                SCOPE_EXIT_RTN_VALUE(-1, "Couldn't create offer\n");
        }
@@ -4028,10 +4028,10 @@ static int new_invite(struct new_invite *invite)
                        goto end;
                }
                /* We are creating a local SDP which is an answer to their offer */
-               local = create_local_sdp(invite->session->inv_session, invite->session, sdp_info->sdp);
+               local = create_local_sdp(invite->session->inv_session, invite->session, sdp_info->sdp, 0);
        } else {
                /* We are creating a local SDP which is an offer */
-               local = create_local_sdp(invite->session->inv_session, invite->session, NULL);
+               local = create_local_sdp(invite->session->inv_session, invite->session, NULL, 0);
        }
 
        /* If we were unable to create a local SDP terminate the session early, it won't go anywhere */
@@ -5103,7 +5103,7 @@ static int add_bundle_groups(struct ast_sip_session *session, pj_pool_t *pool, p
        return 0;
 }
 
-static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer)
+static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, struct ast_sip_session *session, const pjmedia_sdp_session *offer, const unsigned int ignore_active_stream_topology)
 {
        static const pj_str_t STR_IN = { "IN", 2 };
        static const pj_str_t STR_IP4 = { "IP4", 3 };
@@ -5136,11 +5136,19 @@ static struct pjmedia_sdp_session *create_local_sdp(pjsip_inv_session *inv, stru
                /* We've encountered a situation where we have been told to create a local SDP but noone has given us any indication
                 * of what kind of stream topology they would like. We try to not alter the current state of the SDP negotiation
                 * by using what is currently negotiated. If this is unavailable we fall back to what is configured on the endpoint.
+                * We will also do this if wanted by the ignore_active_stream_topology flag.
                 */
+               ast_trace(-1, "no information about stream topology received\n");
                ast_stream_topology_free(session->pending_media_state->topology);
-               if (session->active_media_state->topology) {
+               if (session->active_media_state->topology && !ignore_active_stream_topology) {
+                       ast_trace(-1, "using existing topology\n");
                        session->pending_media_state->topology = ast_stream_topology_clone(session->active_media_state->topology);
                } else {
+                       if (ignore_active_stream_topology) {
+                               ast_trace(-1, "fall back to endpoint configuration - ignore active stream topolog\n");
+                       } else {
+                               ast_trace(-1, "fall back to endpoint configuration\n");
+                       }
                        session->pending_media_state->topology = ast_stream_topology_clone(session->endpoint->media.topology);
                }
                if (!session->pending_media_state->topology) {
@@ -5274,7 +5282,7 @@ static void session_inv_on_rx_offer(pjsip_inv_session *inv, const pjmedia_sdp_se
                SCOPE_EXIT_RTN("%s: handle_incoming_sdp failed\n", ast_sip_session_get_name(session));
        }
 
-       if ((answer = create_local_sdp(inv, session, offer))) {
+       if ((answer = create_local_sdp(inv, session, offer, 0))) {
                pjsip_inv_set_sdp_answer(inv, answer);
                SCOPE_EXIT_RTN("%s: Set SDP answer\n", ast_sip_session_get_name(session));
        }
@@ -5287,6 +5295,7 @@ static void session_inv_on_create_offer(pjsip_inv_session *inv, pjmedia_sdp_sess
        const pjmedia_sdp_session *previous_sdp = NULL;
        pjmedia_sdp_session *offer;
        int i;
+       unsigned int ignore_active_stream_topology = 0;
 
        /* We allow PJSIP to produce an SDP if no channel is present. This may result
         * in an incorrect SDP occurring, but if no channel is present then we are in
@@ -5294,8 +5303,24 @@ static void session_inv_on_create_offer(pjsip_inv_session *inv, pjmedia_sdp_sess
         * produce an SDP doesn't need to worry about a channel being present or not,
         * just in case.
         */
+       SCOPE_ENTER(3, "%s\n", ast_sip_session_get_name(session));
        if (!session->channel) {
-               return;
+               SCOPE_EXIT_RTN("%s: No channel\n", ast_sip_session_get_name(session));
+       }
+
+       /* Some devices send a re-INVITE offer with empty SDP. Asterisk by default return
+        * an answer with the current used codecs, which is not strictly compliant to RFC
+        * 3261 (SHOULD requirement). So we detect this condition and include all
+        * configured codecs in the answer if the workaround is activated.
+        */
+       if (inv->invite_tsx && inv->state == PJSIP_INV_STATE_CONFIRMED
+                       && inv->invite_tsx->method.id == PJSIP_INVITE_METHOD) {
+               ast_trace(-1, "re-INVITE\n");
+               if (inv->invite_tsx->role == PJSIP_ROLE_UAS && !pjmedia_sdp_neg_was_answer_remote(inv->neg)
+                               && ast_sip_get_all_codecs_on_empty_reinvite()) {
+                       ast_trace(-1, "no codecs in re-INIVTE, include all codecs in the answer\n");
+                       ignore_active_stream_topology = 1;
+               }
        }
 
        if (inv->neg) {
@@ -5306,9 +5331,13 @@ static void session_inv_on_create_offer(pjsip_inv_session *inv, pjmedia_sdp_sess
                }
        }
 
-       offer = create_local_sdp(inv, session, previous_sdp);
+       if (ignore_active_stream_topology) {
+               offer = create_local_sdp(inv, session, NULL, 1);
+       } else {
+               offer = create_local_sdp(inv, session, previous_sdp, 0);
+       }
        if (!offer) {
-               return;
+               SCOPE_EXIT_RTN("%s: create offer failed\n", ast_sip_session_get_name(session));
        }
 
        ast_queue_unhold(session->channel);
@@ -5354,6 +5383,7 @@ static void session_inv_on_create_offer(pjsip_inv_session *inv, pjmedia_sdp_sess
        }
 
        *p_offer = offer;
+       SCOPE_EXIT_RTN("%s: offer created\n", ast_sip_session_get_name(session));
 }
 
 static void session_inv_on_media_update(pjsip_inv_session *inv, pj_status_t status)