res_pjsip_registrar.c: Update remove_existing AOR contact handling.
authorRichard Mudgett <rmudgett@digium.com>
Wed, 20 Sep 2017 23:36:15 +0000 (18:36 -0500)
committerRichard Mudgett <rmudgett@digium.com>
Mon, 9 Oct 2017 17:52:30 +0000 (12:52 -0500)
When "rewrite_contact" is enabled, the "max_contacts" count option can
block re-registrations because the source port from the endpoint can be
random.  When the re-registration is blocked, the endpoint may give up
re-registering and require manual intervention.

* The "remove_existing" option now allows a registration to succeed by
displacing any existing contacts that now exceed the "max_contacts" count.
Any removed contacts are the next to expire.  The behaviour change is
beneficial when "rewrite_contact" is enabled and "max_contacts" is greater
than one.  The removed contact is likely the old contact created by
"rewrite_contact" that the device is refreshing.

ASTERISK-27192

Change-Id: I64c107a10b70db1697d17136051ae6bf22b5314b

CHANGES
configs/samples/pjsip.conf.sample
include/asterisk/vector.h
res/res_pjsip.c
res/res_pjsip_registrar.c

diff --git a/CHANGES b/CHANGES
index dde24e2..4d1395f 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -21,6 +21,19 @@ chan_sip
    dialplan in the hash TRANSFER_DATA.
 
 ------------------------------------------------------------------------------
+--- Functionality changes from Asterisk 15.0.0 to Asterisk 15.1.0 ------------
+------------------------------------------------------------------------------
+
+res_pjsip
+------------------
+ * The "remove_existing" option now allows a registration to succeed by
+   displacing any existing contacts that now exceed the "max_contacts" count.
+   Any removed contacts are the next to expire.  The behaviour change is
+   beneficial when "rewrite_contact" is enabled and "max_contacts" is greater
+   than one.  The removed contact is likely the old contact created by
+   "rewrite_contact" that the device is refreshing.
+
+------------------------------------------------------------------------------
 --- Functionality changes from Asterisk 14 to Asterisk 15 --------------------
 ------------------------------------------------------------------------------
 
index 9a2592b..c535aaf 100644 (file)
 ;max_contacts=0 ; Maximum number of contacts that can bind to an AoR (default:
                 ; "0")
 ;minimum_expiration=60  ; Minimum keep alive time for an AoR (default: "60")
-;remove_existing=no     ; Determines whether new contacts replace existing ones
+;remove_existing=no     ; Allow a registration to succeed by displacing any existing
+                        ; contacts that now exceed the max_contacts count.  Any
+                        ; removed contacts are the next to expire.  The behaviour is
+                        ; beneficial when rewrite_contact is enabled and max_contacts
+                        ; is greater than one.  The removed contact is likely the old
+                        ; contact created by rewrite_contact that the device is
+                        ; refreshing.
                         ; (default: "no")
 ;type=  ; Must be of type aor (default: "")
 ;qualify_frequency=0    ; Interval at which to qualify an AoR (default: "0")
 
 ;outbound_auth=            ; Authentication object(s) to be used for outbound
                            ; publishes.
-                           ; This is a comma-delimited list of auth    sections
+                           ; This is a comma-delimited list of auth sections
                            ; defined in pjsip.conf used to respond to outbound
                            ; authentication challenges.
                            ; Using the same auth section for inbound and
index 1e6fe03..68ce130 100644 (file)
@@ -548,6 +548,14 @@ AST_VECTOR(ast_vector_int, int);
 #define AST_VECTOR_SIZE(vec) (vec)->current
 
 /*!
+ * \brief Get the maximum number of elements the vector can currently hold.
+ *
+ * \param vec Vector to query.
+ * \return Maximum number of elements the vector can currently hold.
+ */
+#define AST_VECTOR_MAX_SIZE(vec) (vec)->max
+
+/*!
  * \brief Reset vector.
  *
  * \param vec Vector to reset.
index 6c1f776..cbeb5ea 100644 (file)
                                                It only limits contacts added through external interaction, such as
                                                registration.
                                                </para>
+                                               <note><para>The <replaceable>rewrite_contact</replaceable> option
+                                               registers the source address as the contact address to help with
+                                               NAT and reusing connection oriented transports such as TCP and
+                                               TLS.  Unfortunately, refreshing a registration may register a
+                                               different contact address and exceed
+                                               <replaceable>max_contacts</replaceable>.  The
+                                               <replaceable>remove_existing</replaceable> option can help by
+                                               removing the soonest to expire contact(s) over
+                                               <replaceable>max_contacts</replaceable> which is likely the
+                                               old <replaceable>rewrite_contact</replaceable> contact source
+                                               address being refreshed.
+                                               </para></note>
                                                <note><para>This should be set to <literal>1</literal> and
                                                <replaceable>remove_existing</replaceable> set to <literal>yes</literal> if you
                                                wish to stick with the older <literal>chan_sip</literal> behaviour.
                                <configOption name="minimum_expiration" default="60">
                                        <synopsis>Minimum keep alive time for an AoR</synopsis>
                                        <description><para>
-                                               Minimum time to keep a peer with an explict expiration. Time in seconds.
+                                               Minimum time to keep a peer with an explicit expiration. Time in seconds.
                                        </para></description>
                                </configOption>
                                <configOption name="remove_existing" default="no">
                                        <synopsis>Determines whether new contacts replace existing ones.</synopsis>
                                        <description><para>
-                                               On receiving a new registration to the AoR should it remove
-                                               the existing contact that was registered against it?
+                                               On receiving a new registration to the AoR should it remove enough
+                                               existing contacts not added or updated by the registration to
+                                               satisfy <replaceable>max_contacts</replaceable>?  Any removed
+                                               contacts will expire the soonest.
                                                </para>
+                                               <note><para>The <replaceable>rewrite_contact</replaceable> option
+                                               registers the source address as the contact address to help with
+                                               NAT and reusing connection oriented transports such as TCP and
+                                               TLS.  Unfortunately, refreshing a registration may register a
+                                               different contact address and exceed
+                                               <replaceable>max_contacts</replaceable>.  The
+                                               <replaceable>remove_existing</replaceable> option can help by
+                                               removing the soonest to expire contact(s) over
+                                               <replaceable>max_contacts</replaceable> which is likely the
+                                               old <replaceable>rewrite_contact</replaceable> contact source
+                                               address being refreshed.
+                                               </para></note>
                                                <note><para>This should be set to <literal>yes</literal> and
                                                <replaceable>max_contacts</replaceable> set to <literal>1</literal> if you
                                                wish to stick with the older <literal>chan_sip</literal> behaviour.
index ba1c074..3290601 100644 (file)
@@ -129,7 +129,8 @@ static int registrar_find_contact(void *obj, void *arg, int flags)
 /*! \brief Internal function which validates provided Contact headers to confirm that they are acceptable, and returns number of contacts */
 static int registrar_validate_contacts(const pjsip_rx_data *rdata, struct ao2_container *contacts, struct ast_sip_aor *aor, int *added, int *updated, int *deleted)
 {
-       pjsip_contact_hdr *previous = NULL, *contact = (pjsip_contact_hdr *)&rdata->msg_info.msg->hdr;
+       pjsip_contact_hdr *previous = NULL;
+       pjsip_contact_hdr *contact = (pjsip_contact_hdr *)&rdata->msg_info.msg->hdr;
        struct registrar_contact_details details = {
                .pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(), "Contact Comparison", 256, 256),
        };
@@ -140,15 +141,18 @@ static int registrar_validate_contacts(const pjsip_rx_data *rdata, struct ao2_co
 
        while ((contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, contact->next))) {
                int expiration = registrar_get_expiration(aor, contact, rdata);
-               RAII_VAR(struct ast_sip_contact *, existing, NULL, ao2_cleanup);
+               struct ast_sip_contact *existing;
                char contact_uri[pjsip_max_url_size];
 
                if (contact->star) {
                        /* The expiration MUST be 0 when a '*' contact is used and there must be no other contact */
-                       if ((expiration != 0) || previous) {
+                       if (expiration != 0 || previous) {
                                pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
                                return -1;
                        }
+                       /* Count all contacts to delete */
+                       *deleted = ao2_container_count(contacts);
+                       previous = contact;
                        continue;
                } else if (previous && previous->star) {
                        /* If there is a previous contact and it is a '*' this is a deal breaker */
@@ -177,14 +181,16 @@ static int registrar_validate_contacts(const pjsip_rx_data *rdata, struct ao2_co
                }
 
                /* Determine if this is an add, update, or delete for policy enforcement purposes */
-               if (!(existing = ao2_callback(contacts, 0, registrar_find_contact, &details))) {
+               existing = ao2_callback(contacts, 0, registrar_find_contact, &details);
+               ao2_cleanup(existing);
+               if (!existing) {
                        if (expiration) {
-                               (*added)++;
+                               ++*added;
                        }
                } else if (expiration) {
-                       (*updated)++;
+                       ++*updated;
                } else {
-                       (*deleted)++;
+                       ++*deleted;
                }
        }
 
@@ -219,7 +225,7 @@ static int registrar_delete_contact(void *obj, void *arg, int flags)
                                contact->user_agent);
        }
 
-       return 0;
+       return CMP_MATCH;
 }
 
 /*! \brief Internal function which adds a contact to a response */
@@ -351,6 +357,96 @@ static void register_contact_transport_shutdown_cb(void *data)
        ao2_ref(aor, -1);
 }
 
+AST_VECTOR(excess_contact_vector, struct ast_sip_contact *);
+
+static int vec_contact_cmp(struct ast_sip_contact *left, struct ast_sip_contact *right)
+{
+       struct ast_sip_contact *left_contact = left;
+       struct ast_sip_contact *right_contact = right;
+
+       /* Sort from soonest to expire to last to expire */
+       return ast_tvcmp(left_contact->expiration_time, right_contact->expiration_time);
+}
+
+static int vec_contact_add(void *obj, void *arg, int flags)
+{
+       struct ast_sip_contact *contact = obj;
+       struct excess_contact_vector *contact_vec = arg;
+
+       /*
+        * Performance wise, an insertion sort is fine because we
+        * shouldn't need to remove more than a handful of contacts.
+        * I expect we'll typically be removing only one contact.
+        */
+       AST_VECTOR_ADD_SORTED(contact_vec, contact, vec_contact_cmp);
+       if (AST_VECTOR_SIZE(contact_vec) == AST_VECTOR_MAX_SIZE(contact_vec)) {
+               /*
+                * We added a contact over the number we need to remove.
+                * Remove the longest to expire contact from the vector
+                * which is the last element in the vector.  It may be
+                * the one we just added or the one we just added pushed
+                * out an earlier contact from removal consideration.
+                */
+               --AST_VECTOR_SIZE(contact_vec);
+       }
+       return 0;
+}
+
+/*!
+ * \internal
+ * \brief Remove excess existing contacts that expire the soonest.
+ * \since 13.18.0
+ *
+ * \param contacts Container of unmodified contacts that could remove.
+ * \param to_remove Maximum number of contacts to remove.
+ *
+ * \return Nothing
+ */
+static void remove_excess_contacts(struct ao2_container *contacts, unsigned int to_remove)
+{
+       struct excess_contact_vector contact_vec;
+
+       /*
+        * Create a sorted vector to hold the to_remove soonest to
+        * expire contacts.  The vector has an extra space to
+        * temporarily hold the longest to expire contact that we
+        * won't remove.
+        */
+       if (AST_VECTOR_INIT(&contact_vec, to_remove + 1)) {
+               return;
+       }
+       ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, vec_contact_add, &contact_vec);
+
+       /*
+        * The vector should always be populated with the number
+        * of contacts we need to remove.  Just in case, we will
+        * remove all contacts in the vector even if the contacts
+        * container had fewer contacts than there should be.
+        */
+       ast_assert(AST_VECTOR_SIZE(&contact_vec) == to_remove);
+       to_remove = AST_VECTOR_SIZE(&contact_vec);
+
+       /* Remove the excess contacts that expire the soonest */
+       while (to_remove--) {
+               struct ast_sip_contact *contact;
+
+               contact = AST_VECTOR_GET(&contact_vec, to_remove);
+
+               ast_sip_location_delete_contact(contact);
+               ast_verb(3, "Removed contact '%s' from AOR '%s' due to remove_existing\n",
+                       contact->uri, contact->aor);
+               ast_test_suite_event_notify("AOR_CONTACT_REMOVED",
+                       "Contact: %s\r\n"
+                       "AOR: %s\r\n"
+                       "UserAgent: %s",
+                       contact->uri,
+                       contact->aor,
+                       contact->user_agent);
+       }
+
+       AST_VECTOR_FREE(&contact_vec);
+}
+
 static int register_aor_core(pjsip_rx_data *rdata,
        struct ast_sip_endpoint *endpoint,
        struct ast_sip_aor *aor,
@@ -359,7 +455,10 @@ static int register_aor_core(pjsip_rx_data *rdata,
 {
        static const pj_str_t USER_AGENT = { "User-Agent", 10 };
 
-       int added = 0, updated = 0, deleted = 0;
+       int added = 0;
+       int updated = 0;
+       int deleted = 0;
+       int contact_count;
        pjsip_contact_hdr *contact_hdr = NULL;
        struct registrar_contact_details details = { 0, };
        pjsip_tx_data *tdata;
@@ -396,7 +495,14 @@ static int register_aor_core(pjsip_rx_data *rdata,
                return PJ_TRUE;
        }
 
-       if ((MAX(added - deleted, 0) + (!aor->remove_existing ? ao2_container_count(contacts) : 0)) > aor->max_contacts) {
+       if (aor->remove_existing) {
+               /* Cumulative number of contacts affected by this registration */
+               contact_count = MAX(updated + added - deleted,  0);
+       } else {
+               /* Total contacts after this registration */
+               contact_count = ao2_container_count(contacts) + added - deleted;
+       }
+       if (contact_count > aor->max_contacts) {
                /* Enforce the maximum number of contacts */
                pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 403, NULL, NULL, NULL);
                ast_sip_report_failed_acl(endpoint, rdata, "registrar_attempt_exceeds_maximum_configured_contacts");
@@ -405,7 +511,9 @@ static int register_aor_core(pjsip_rx_data *rdata,
                return PJ_TRUE;
        }
 
-       if (!(details.pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(), "Contact Comparison", 256, 256))) {
+       details.pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(),
+               "Contact Comparison", 256, 256);
+       if (!details.pool) {
                pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);
                return PJ_TRUE;
        }
@@ -446,7 +554,8 @@ static int register_aor_core(pjsip_rx_data *rdata,
 
                if (contact_hdr->star) {
                        /* A star means to unregister everything, so do so for the possible contacts */
-                       ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, registrar_delete_contact, (void *)aor_name);
+                       ao2_callback(contacts, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE,
+                               registrar_delete_contact, (void *)aor_name);
                        break;
                }
 
@@ -459,7 +568,8 @@ static int register_aor_core(pjsip_rx_data *rdata,
                details.uri = pjsip_uri_get_uri(contact_hdr->uri);
                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))) {
+               contact = ao2_callback(contacts, OBJ_UNLINK, registrar_find_contact, &details);
+               if (!contact) {
                        int prune_on_boot = 0;
                        pj_str_t host_name;
 
@@ -600,15 +710,29 @@ static int register_aor_core(pjsip_rx_data *rdata,
 
        pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
 
-       /* If the AOR is configured to remove any existing contacts that have not been updated/added as a result of this REGISTER
-        * do so
+       /*
+        * If the AOR is configured to remove any contacts over max_contacts
+        * that have not been updated/added/deleted as a result of this
+        * REGISTER do so.
+        *
+        * The contacts container currently holds the existing contacts that
+        * were not affected by this REGISTER.
         */
        if (aor->remove_existing) {
-               ao2_callback(contacts, OBJ_NODATA | OBJ_MULTIPLE, registrar_delete_contact, NULL);
+               /* Total contacts after this registration */
+               contact_count = ao2_container_count(contacts) + updated + added;
+               if (contact_count > aor->max_contacts) {
+                       /* Remove excess existing contacts that expire the soonest */
+                       remove_excess_contacts(contacts, contact_count - aor->max_contacts);
+               }
        }
 
        /* Re-retrieve contacts.  Caller will clean up the original container. */
        contacts = ast_sip_location_retrieve_aor_contacts_nolock(aor);
+       if (!contacts) {
+               pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 500, NULL, NULL, NULL);
+               return PJ_TRUE;
+       }
        response_contact = ao2_callback(contacts, 0, NULL, NULL);
 
        /* Send a response containing all of the contacts (including static) that are present on this AOR */