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