<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6197">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved; Verified
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip: Remove ephemeral registered contacts on transport shutdown.<br><br>The fix for the issue is broken up into three parts.<br><br>This is part two which handles the server side of REGISTER requests when<br>rewrite_contact is enabled. Any registered reliable transport contact<br>becomes invalid when the transport connection becomes disconnected.<br><br>* Monitor the rewrite_contact's reliable transport REGISTER contact for<br>shutdown. If it is shutdown then the contact must be removed because it<br>is no longer valid. Otherwise, when the client attempts to re-REGISTER it<br>may be blocked because the invalid contact is there. Also if we try to<br>send a call to the endpoint using the invalid contact then the endpoint is<br>not likely to see the request. The endpoint either won't be listening on<br>that port for new connections or a NAT/firewall will block it.<br><br>* Prune any rewrite_contact's registered reliable transport contacts on<br>boot. The reliable transport no longer exists so the contact is invalid.<br><br>* Websockets always rewrite the REGISTER contact address and the transport<br>needs to be monitored for shutdown.<br><br>* Made the websocket transport set a unique name since that is what we use<br>as the ao2 container key. Otherwise, we would not know which transport we<br>find when one of them shuts down. The names are also used for PJPROJECT<br>debug logging.<br><br>* Made the websocket transport post the PJSIP_TP_STATE_CONNECTED state<br>event. Now the global keep_alive_interval option, initially idle shutdown<br>timer, and the server REGISTER contact monitor can work on wetsocket<br>transports.<br><br>* Made the websocket transport set the PJSIP_TP_DIR_INCOMING direction.<br>Now initially idle websockets will automatically shutdown.<br><br>ASTERISK-27147<br><br>Change-Id: I397a5e7d18476830f7ffe1726adf9ee6c15964f4<br>---<br>A contrib/ast-db-manage/config/versions/f3d1c5d38b56_add_prune_on_boot.py<br>M include/asterisk/res_pjsip.h<br>M res/res_pjsip.c<br>M res/res_pjsip/location.c<br>M res/res_pjsip/pjsip_configuration.c<br>M res/res_pjsip_registrar.c<br>M res/res_pjsip_transport_websocket.c<br>7 files changed, 250 insertions(+), 17 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/contrib/ast-db-manage/config/versions/f3d1c5d38b56_add_prune_on_boot.py b/contrib/ast-db-manage/config/versions/f3d1c5d38b56_add_prune_on_boot.py<br>new file mode 100644<br>index 0000000..eccd441<br>--- /dev/null<br>+++ b/contrib/ast-db-manage/config/versions/f3d1c5d38b56_add_prune_on_boot.py<br>@@ -0,0 +1,28 @@<br>+"""add_prune_on_boot<br>+<br>+Revision ID: f3d1c5d38b56<br>+Revises: 164abbd708c<br>+Create Date: 2017-08-04 17:31:23.124767<br>+<br>+"""<br>+<br>+# revision identifiers, used by Alembic.<br>+revision = 'f3d1c5d38b56'<br>+down_revision = '164abbd708c'<br>+<br>+from alembic import op<br>+import sqlalchemy as sa<br>+<br>+<br>+def upgrade():<br>+ ############################# Enums ##############################<br>+<br>+ # yesno_values have already been created, so use postgres enum object<br>+ # type to get around "already created" issue - works okay with mysql<br>+ yesno_values = ENUM(*YESNO_VALUES, name=YESNO_NAME, create_type=False)<br>+<br>+ op.add_column('ps_contacts', sa.Column('prune_on_boot', yesno_values))<br>+<br>+<br>+def downgrade():<br>+ op.drop_column('ps_contacts', 'prune_on_boot')<br>diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h<br>index c630cd5..efc0cd0 100644<br>--- a/include/asterisk/res_pjsip.h<br>+++ b/include/asterisk/res_pjsip.h<br>@@ -270,6 +270,8 @@<br> AST_STRING_FIELD_EXTENDED(call_id);<br> /*! The name of the endpoint that added the contact */<br> AST_STRING_FIELD_EXTENDED(endpoint_name);<br>+ /*! If true delete the contact on Asterisk restart/boot */<br>+ int prune_on_boot;<br> };<br> <br> #define CONTACT_STATUS "contact_status"<br>@@ -1203,6 +1205,9 @@<br> * \param expiration_time Optional expiration time of the contact<br> * \param path_info Path information<br> * \param user_agent User-Agent header from REGISTER request<br>+ * \param via_addr<br>+ * \param via_port<br>+ * \param call_id<br> * \param endpoint The endpoint that resulted in the contact being added<br> *<br> * \retval -1 failure<br>@@ -1226,6 +1231,9 @@<br> * \param expiration_time Optional expiration time of the contact<br> * \param path_info Path information<br> * \param user_agent User-Agent header from REGISTER request<br>+ * \param via_addr<br>+ * \param via_port<br>+ * \param call_id<br> * \param endpoint The endpoint that resulted in the contact being added<br> *<br> * \retval -1 failure<br>@@ -1238,6 +1246,31 @@<br> struct timeval expiration_time, const char *path_info, const char *user_agent,<br> const char *via_addr, int via_port, const char *call_id,<br> struct ast_sip_endpoint *endpoint);<br>+<br>+/*!<br>+ * \brief Create a new contact for an AOR without locking the AOR<br>+ * \since 13.18.0<br>+ *<br>+ * \param aor Pointer to the AOR<br>+ * \param uri Full contact URI<br>+ * \param expiration_time Optional expiration time of the contact<br>+ * \param path_info Path information<br>+ * \param user_agent User-Agent header from REGISTER request<br>+ * \param via_addr<br>+ * \param via_port<br>+ * \param call_id<br>+ * \param prune_on_boot Non-zero if the contact cannot survive a restart/boot.<br>+ * \param endpoint The endpoint that resulted in the contact being added<br>+ *<br>+ * \return The created contact or NULL on failure.<br>+ *<br>+ * \warning<br>+ * This function should only be called if you already hold a named write lock on the aor.<br>+ */<br>+struct ast_sip_contact *ast_sip_location_create_contact(struct ast_sip_aor *aor,<br>+ const char *uri, struct timeval expiration_time, const char *path_info,<br>+ const char *user_agent, const char *via_addr, int via_port, const char *call_id,<br>+ int prune_on_boot, struct ast_sip_endpoint *endpoint);<br> <br> /*!<br> * \brief Update a contact<br>@@ -1260,6 +1293,12 @@<br> int ast_sip_location_delete_contact(struct ast_sip_contact *contact);<br> <br> /*!<br>+ * \brief Prune the prune_on_boot contacts<br>+ * \since 13.18.0<br>+ */<br>+void ast_sip_location_prune_boot_contacts(void);<br>+<br>+/*!<br> * \brief Callback called when an outbound request with authentication credentials is to be sent in dialog<br> *<br> * This callback will have the created request on it. The callback's purpose is to do any extra<br>diff --git a/res/res_pjsip.c b/res/res_pjsip.c<br>index 2d4d433..0d46a15 100644<br>--- a/res/res_pjsip.c<br>+++ b/res/res_pjsip.c<br>@@ -367,9 +367,12 @@<br> <configOption name="rewrite_contact"><br> <synopsis>Allow Contact header to be rewritten with the source IP address-port</synopsis><br> <description><para><br>- On inbound SIP messages from this endpoint, the Contact header or an appropriate Record-Route<br>- header will be changed to have the source IP address and port. This option does not affect<br>- outbound messages sent to this endpoint.<br>+ On inbound SIP messages from this endpoint, the Contact header or an<br>+ appropriate Record-Route header will be changed to have the source IP<br>+ address and port. This option does not affect outbound messages sent to<br>+ this endpoint. This option helps servers communicate with endpoints<br>+ that are behind NATs. This option also helps reuse reliable transport<br>+ connections such as TCP and TLS.<br> </para></description><br> </configOption><br> <configOption name="rtp_ipv6" default="no"><br>@@ -1327,6 +1330,13 @@<br> in incoming SIP REGISTER requests and is not intended to be configured manually.<br> </para></description><br> </configOption><br>+ <configOption name="prune_on_boot"><br>+ <synopsis>A contact that cannot survive a restart/boot.</synopsis><br>+ <description><para><br>+ The option is set if the incoming SIP REGISTER contact is rewritten<br>+ on a reliable transport and is not intended to be configured manually.<br>+ </para></description><br>+ </configOption><br> </configObject><br> <configObject name="aor"><br> <synopsis>The configuration for a location of an endpoint</synopsis><br>diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c<br>index 6213046..557aeb6 100644<br>--- a/res/res_pjsip/location.c<br>+++ b/res/res_pjsip/location.c<br>@@ -356,13 +356,12 @@<br> return ast_sorcery_retrieve_by_id(ast_sip_get_sorcery(), "contact", contact_name);<br> }<br> <br>-int ast_sip_location_add_contact_nolock(struct ast_sip_aor *aor, const char *uri,<br>- struct timeval expiration_time, const char *path_info, const char *user_agent,<br>- const char *via_addr, int via_port, const char *call_id,<br>- struct ast_sip_endpoint *endpoint)<br>+struct ast_sip_contact *ast_sip_location_create_contact(struct ast_sip_aor *aor,<br>+ const char *uri, struct timeval expiration_time, const char *path_info,<br>+ const char *user_agent, const char *via_addr, int via_port, const char *call_id,<br>+ int prune_on_boot, struct ast_sip_endpoint *endpoint)<br> {<br> struct ast_sip_contact *contact;<br>- int res;<br> char name[MAX_OBJECT_FIELD * 2 + 3];<br> char hash[33];<br> <br>@@ -371,7 +370,7 @@<br> <br> contact = ast_sorcery_alloc(ast_sip_get_sorcery(), "contact", name);<br> if (!contact) {<br>- return -1;<br>+ return NULL;<br> }<br> <br> ast_string_field_set(contact, uri, uri);<br>@@ -405,14 +404,30 @@<br> }<br> <br> contact->endpoint = ao2_bump(endpoint);<br>-<br> if (endpoint) {<br> ast_string_field_set(contact, endpoint_name, ast_sorcery_object_get_id(endpoint));<br> }<br> <br>- res = ast_sorcery_create(ast_sip_get_sorcery(), contact);<br>- ao2_ref(contact, -1);<br>- return res;<br>+ contact->prune_on_boot = prune_on_boot;<br>+<br>+ if (ast_sorcery_create(ast_sip_get_sorcery(), contact)) {<br>+ ao2_ref(contact, -1);<br>+ return NULL;<br>+ }<br>+ return contact;<br>+}<br>+<br>+int ast_sip_location_add_contact_nolock(struct ast_sip_aor *aor, const char *uri,<br>+ struct timeval expiration_time, const char *path_info, const char *user_agent,<br>+ const char *via_addr, int via_port, const char *call_id,<br>+ struct ast_sip_endpoint *endpoint)<br>+{<br>+ struct ast_sip_contact *contact;<br>+<br>+ contact = ast_sip_location_create_contact(aor, uri, expiration_time, path_info,<br>+ user_agent, via_addr, via_port, call_id, 0, endpoint);<br>+ ao2_cleanup(contact);<br>+ return contact ? 0 : -1;<br> }<br> <br> int ast_sip_location_add_contact(struct ast_sip_aor *aor, const char *uri,<br>@@ -439,6 +454,29 @@<br> int ast_sip_location_delete_contact(struct ast_sip_contact *contact)<br> {<br> return ast_sorcery_delete(ast_sip_get_sorcery(), contact);<br>+}<br>+<br>+static int prune_boot_contacts_cb(void *obj, void *arg, int flags)<br>+{<br>+ struct ast_sip_contact *contact = obj;<br>+<br>+ if (contact->prune_on_boot) {<br>+ ast_sip_location_delete_contact(contact);<br>+ }<br>+<br>+ return 0;<br>+}<br>+<br>+void ast_sip_location_prune_boot_contacts(void)<br>+{<br>+ struct ao2_container *contacts;<br>+<br>+ contacts = ast_sorcery_retrieve_by_fields(ast_sip_get_sorcery(), "contact",<br>+ AST_RETRIEVE_FLAG_MULTIPLE | AST_RETRIEVE_FLAG_ALL, NULL);<br>+ if (contacts) {<br>+ ao2_callback(contacts, 0, prune_boot_contacts_cb, NULL);<br>+ ao2_ref(contacts, -1);<br>+ }<br> }<br> <br> /*! \brief Custom handler for translating from a string timeval to actual structure */<br>@@ -1221,6 +1259,7 @@<br> ast_sorcery_object_field_register(sorcery, "contact", "via_addr", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_contact, via_addr));<br> ast_sorcery_object_field_register(sorcery, "contact", "via_port", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_contact, via_port));<br> ast_sorcery_object_field_register(sorcery, "contact", "call_id", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_contact, call_id));<br>+ ast_sorcery_object_field_register(sorcery, "contact", "prune_on_boot", "no", OPT_YESNO_T, 1, FLDSET(struct ast_sip_contact, prune_on_boot));<br> <br> ast_sorcery_object_field_register(sorcery, "aor", "type", "", OPT_NOOP_T, 0, 0);<br> ast_sorcery_object_field_register(sorcery, "aor", "minimum_expiration", "60", OPT_UINT_T, 0, FLDSET(struct ast_sip_aor, minimum_expiration));<br>diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c<br>index fb15db0..4d3c616 100644<br>--- a/res/res_pjsip/pjsip_configuration.c<br>+++ b/res/res_pjsip/pjsip_configuration.c<br>@@ -2020,6 +2020,8 @@<br> <br> load_all_endpoints();<br> <br>+ ast_sip_location_prune_boot_contacts();<br>+<br> return 0;<br> }<br> <br>diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c<br>index a4ce547..ba1c074 100644<br>--- a/res/res_pjsip_registrar.c<br>+++ b/res/res_pjsip_registrar.c<br>@@ -310,6 +310,47 @@<br> return -1;<br> }<br> <br>+/*! Transport monitor for incoming REGISTER contacts */<br>+struct contact_transport_monitor {<br>+ /*!<br>+ * \brief Sorcery contact name to remove on transport shutdown<br>+ * \note Stored after aor_name in space reserved when struct allocated.<br>+ */<br>+ char *contact_name;<br>+ /*! AOR name the contact is associated */<br>+ char aor_name[0];<br>+};<br>+<br>+static void register_contact_transport_shutdown_cb(void *data)<br>+{<br>+ struct contact_transport_monitor *monitor = data;<br>+ struct ast_sip_contact *contact;<br>+ struct ast_sip_aor *aor;<br>+<br>+ aor = ast_sip_location_retrieve_aor(monitor->aor_name);<br>+ if (!aor) {<br>+ return;<br>+ }<br>+<br>+ ao2_lock(aor);<br>+ contact = ast_sip_location_retrieve_contact(monitor->contact_name);<br>+ if (contact) {<br>+ ast_sip_location_delete_contact(contact);<br>+ ast_verb(3, "Removed contact '%s' from AOR '%s' due to transport shutdown\n",<br>+ contact->uri, monitor->aor_name);<br>+ ast_test_suite_event_notify("AOR_CONTACT_REMOVED",<br>+ "Contact: %s\r\n"<br>+ "AOR: %s\r\n"<br>+ "UserAgent: %s",<br>+ contact->uri,<br>+ monitor->aor_name,<br>+ contact->user_agent);<br>+ ao2_ref(contact, -1);<br>+ }<br>+ ao2_unlock(aor);<br>+ ao2_ref(aor, -1);<br>+}<br>+<br> static int register_aor_core(pjsip_rx_data *rdata,<br> struct ast_sip_endpoint *endpoint,<br> struct ast_sip_aor *aor,<br>@@ -419,6 +460,9 @@<br> pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, details.uri, contact_uri, sizeof(contact_uri));<br> <br> if (!(contact = ao2_callback(contacts, OBJ_UNLINK, registrar_find_contact, &details))) {<br>+ int prune_on_boot = 0;<br>+ pj_str_t host_name;<br>+<br> /* If they are actually trying to delete a contact that does not exist... be forgiving */<br> if (!expiration) {<br> ast_verb(3, "Attempted to remove non-existent contact '%s' from AOR '%s' by request\n",<br>@@ -426,12 +470,66 @@<br> continue;<br> }<br> <br>- if (ast_sip_location_add_contact_nolock(aor, contact_uri, ast_tvadd(ast_tvnow(),<br>- ast_samp2tv(expiration, 1)), path_str ? ast_str_buffer(path_str) : NULL,<br>- user_agent, via_addr, via_port, call_id, endpoint)) {<br>+ /* Determine if the contact cannot survive a restart/boot. */<br>+ if (details.uri->port == rdata->pkt_info.src_port<br>+ && !pj_strcmp(&details.uri->host,<br>+ pj_cstr(&host_name, rdata->pkt_info.src_name))<br>+ /* We have already checked if the URI scheme is sip: or sips: */<br>+ && PJSIP_TRANSPORT_IS_RELIABLE(rdata->tp_info.transport)) {<br>+ pj_str_t type_name;<br>+<br>+ /* Determine the transport parameter value */<br>+ if (!strcasecmp("WSS", rdata->tp_info.transport->type_name)) {<br>+ /* WSS is special, as it needs to be ws. */<br>+ pj_cstr(&type_name, "ws");<br>+ } else {<br>+ pj_cstr(&type_name, rdata->tp_info.transport->type_name);<br>+ }<br>+<br>+ if (!pj_stricmp(&details.uri->transport_param, &type_name)<br>+ && (endpoint->nat.rewrite_contact<br>+ /* Websockets are always rewritten */<br>+ || !pj_stricmp(&details.uri->transport_param,<br>+ pj_cstr(&type_name, "ws")))) {<br>+ /*<br>+ * The contact was rewritten to the reliable transport's<br>+ * source address. Disconnecting the transport for any<br>+ * reason invalidates the contact.<br>+ */<br>+ prune_on_boot = 1;<br>+ }<br>+ }<br>+<br>+ contact = ast_sip_location_create_contact(aor, contact_uri,<br>+ ast_tvadd(ast_tvnow(), ast_samp2tv(expiration, 1)),<br>+ path_str ? ast_str_buffer(path_str) : NULL,<br>+ user_agent, via_addr, via_port, call_id, prune_on_boot, endpoint);<br>+ if (!contact) {<br> ast_log(LOG_ERROR, "Unable to bind contact '%s' to AOR '%s'\n",<br>- contact_uri, aor_name);<br>+ contact_uri, aor_name);<br> continue;<br>+ }<br>+<br>+ if (prune_on_boot) {<br>+ const char *contact_name;<br>+ struct contact_transport_monitor *monitor;<br>+<br>+ /*<br>+ * Monitor the transport in case it gets disconnected because<br>+ * the contact won't be valid anymore if that happens.<br>+ */<br>+ contact_name = ast_sorcery_object_get_id(contact);<br>+ monitor = ao2_alloc_options(sizeof(*monitor) + 2 + strlen(aor_name)<br>+ + strlen(contact_name), NULL, AO2_ALLOC_OPT_LOCK_NOLOCK);<br>+ if (monitor) {<br>+ strcpy(monitor->aor_name, aor_name);/* Safe */<br>+ monitor->contact_name = monitor->aor_name + strlen(aor_name) + 1;<br>+ strcpy(monitor->contact_name, contact_name);/* Safe */<br>+<br>+ ast_sip_transport_monitor_register(rdata->tp_info.transport,<br>+ register_contact_transport_shutdown_cb, monitor);<br>+ ao2_ref(monitor, -1);<br>+ }<br> }<br> <br> ast_verb(3, "Added contact '%s' to AOR '%s' with expiration of %d seconds\n",<br>@@ -885,6 +983,7 @@<br> ast_manager_unregister(AMI_SHOW_REGISTRATIONS);<br> ast_manager_unregister(AMI_SHOW_REGISTRATION_CONTACT_STATUSES);<br> ast_sip_unregister_service(®istrar_module);<br>+ ast_sip_transport_monitor_unregister_all(register_contact_transport_shutdown_cb);<br> return 0;<br> }<br> <br>diff --git a/res/res_pjsip_transport_websocket.c b/res/res_pjsip_transport_websocket.c<br>index 1429cce..22ec195 100644<br>--- a/res/res_pjsip_transport_websocket.c<br>+++ b/res/res_pjsip_transport_websocket.c<br>@@ -145,6 +145,7 @@<br> {<br> struct transport_create_data *create_data = data;<br> struct ws_transport *newtransport = NULL;<br>+ pjsip_tp_state_callback state_cb;<br> <br> pjsip_endpoint *endpt = ast_sip_get_pjsip_endpoint();<br> struct pjsip_tpmgr *tpmgr = pjsip_endpt_get_tpmgr(endpt);<br>@@ -160,6 +161,10 @@<br> ast_log(LOG_ERROR, "Failed to allocate WebSocket transport.\n");<br> goto on_error;<br> }<br>+<br>+ /* Give websocket transport a unique name for its lifetime */<br>+ snprintf(newtransport->transport.obj_name, PJ_MAX_OBJ_NAME, "ws%p",<br>+ &newtransport->transport);<br> <br> newtransport->transport.endpt = endpt;<br> <br>@@ -219,6 +224,7 @@<br> newtransport->transport.flag = pjsip_transport_get_flag_from_type((pjsip_transport_type_e)newtransport->transport.key.type);<br> newtransport->transport.info = (char *)pj_pool_alloc(newtransport->transport.pool, 64);<br> <br>+ newtransport->transport.dir = PJSIP_TP_DIR_INCOMING;<br> newtransport->transport.tpmgr = tpmgr;<br> newtransport->transport.send_msg = &ws_send_msg;<br> newtransport->transport.destroy = &ws_destroy;<br>@@ -242,6 +248,16 @@<br> }<br> <br> create_data->transport = newtransport;<br>+<br>+ /* Notify application of transport state */<br>+ state_cb = pjsip_tpmgr_get_state_cb(newtransport->transport.tpmgr);<br>+ if (state_cb) {<br>+ pjsip_transport_state_info state_info;<br>+<br>+ memset(&state_info, 0, sizeof(state_info));<br>+ state_cb(&newtransport->transport, PJSIP_TP_STATE_CONNECTED, &state_info);<br>+ }<br>+<br> return 0;<br> <br> on_error:<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6197">change 6197</a>. To unsubscribe, 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/6197"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 14 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I397a5e7d18476830f7ffe1726adf9ee6c15964f4 </div>
<div style="display:none"> Gerrit-Change-Number: 6197 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>