security_agreements.c: Refactor the to_str functions and fix a few other bugs

* A static array of security mechanism type names was created.

* ast_sip_str_to_security_mechanism_type() was refactored to do
  a lookup in the new array instead of using fixed "if/else if"
  statments.

* security_mechanism_to_str() and ast_sip_security_mechanisms_to_str()
  were refactored to use ast_str instead of a fixed length buffer
  to store the result.

* ast_sip_security_mechanism_type_to_str was removed in favor of
  just referencing the new type name array.  Despite starting with
  "ast_sip_", it was a static function so removing it doesn't affect
  ABI.

* Speaking of "ast_sip_", several other static functions that
  started with "ast_sip_" were renamed to avoid confusion about
  their public availability.

* A few VECTOR free loops were replaced with AST_VECTOR_RESET().

* Fixed a meomry leak in pjsip_configuration.c endpoint_destructor
  caused by not calling ast_sip_security_mechanisms_vector_destroy().

* Fixed a memory leak in res_pjsip_outbound_registration.c
  add_security_headers() caused by not specifying OBJ_NODATA in
  an ao2_callback.

* Fixed a few ao2_callback return code misuses.

Resolves: #845
(cherry picked from commit ca60f7db8fe636a4c4973bb8aaadb05b2ec3a427)

diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index c83ee33179d8cba5a310f389aab320545b82b658..7a76a88c44ec475dfff9bc9de8265ccb8e0767d5 100644 (file)
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -2434,6 +2434,7 @@ static void endpoint_destructor(void* obj)
        ast_free(endpoint->contact_user);
        ast_free_acl_list(endpoint->contact_acl);
        ast_free_acl_list(endpoint->acl);
+       ast_sip_security_mechanisms_vector_destroy(&endpoint->security_mechanisms);
 }
 
 static int init_subscription_configuration(struct ast_sip_endpoint_subscription_configuration *subscription)

diff --git a/res/res_pjsip/security_agreements.c b/res/res_pjsip/security_agreements.c
index 7c322fcbb3ec86bbbcb61ac2eaf6d8da9c7392c8..9f65c35bb7f268d25d9b71579dfd4b18a44a064b 100644 (file)
--- a/res/res_pjsip/security_agreements.c
+++ b/res/res_pjsip/security_agreements.c
@@ -30,7 +30,7 @@
 
 #include "asterisk/res_pjsip.h"
 
-static struct ast_sip_security_mechanism *ast_sip_security_mechanisms_alloc(size_t n_params)
+static struct ast_sip_security_mechanism *security_mechanisms_alloc(size_t n_params)
 {
        struct ast_sip_security_mechanism *mech;
 
@@ -47,7 +47,7 @@ static struct ast_sip_security_mechanism *ast_sip_security_mechanisms_alloc(size
        return mech;
 }
 
-static struct ast_sip_security_mechanism *ast_sip_security_mechanisms_copy(
+static struct ast_sip_security_mechanism *security_mechanisms_copy(
        const struct ast_sip_security_mechanism *src)
 {
        struct ast_sip_security_mechanism *dst = NULL;
@@ -56,7 +56,7 @@ static struct ast_sip_security_mechanism *ast_sip_security_mechanisms_copy(
 
        n_params = AST_VECTOR_SIZE(&src->mechanism_parameters);
 
-       dst = ast_sip_security_mechanisms_alloc(n_params);
+       dst = security_mechanisms_alloc(n_params);
        if (dst == NULL) {
                return NULL;
        }
@@ -71,13 +71,9 @@ static struct ast_sip_security_mechanism *ast_sip_security_mechanisms_copy(
        return dst;
 }
 
-static void ast_sip_security_mechanisms_destroy(struct ast_sip_security_mechanism *mech)
+static void security_mechanism_destroy(struct ast_sip_security_mechanism *mech)
 {
-       int i;
-
-       for (i = 0; i < AST_VECTOR_SIZE(&mech->mechanism_parameters); i++) {
-               ast_free(AST_VECTOR_GET(&mech->mechanism_parameters, i));
-       }
+       AST_VECTOR_RESET(&mech->mechanism_parameters, ast_free);
        AST_VECTOR_FREE(&mech->mechanism_parameters);
        ast_free(mech);
 }
@@ -91,78 +87,75 @@ void ast_sip_security_mechanisms_vector_copy(struct ast_sip_security_mechanism_v
        ast_sip_security_mechanisms_vector_destroy(dst);
        for (i = 0; i < AST_VECTOR_SIZE(src); i++) {
                mech = AST_VECTOR_GET(src, i);
-               AST_VECTOR_APPEND(dst, ast_sip_security_mechanisms_copy(mech));
+               AST_VECTOR_APPEND(dst, security_mechanisms_copy(mech));
        }
 };
 
 void ast_sip_security_mechanisms_vector_destroy(struct ast_sip_security_mechanism_vector *security_mechanisms)
 {
-       struct ast_sip_security_mechanism *mech;
-       int i;
-
        if (!security_mechanisms) {
                return;
        }
 
-       for (i = 0; i < AST_VECTOR_SIZE(security_mechanisms); i++) {
-               mech = AST_VECTOR_GET(security_mechanisms, i);
-               ast_sip_security_mechanisms_destroy(mech);
-       }
+       AST_VECTOR_RESET(security_mechanisms, security_mechanism_destroy);
        AST_VECTOR_FREE(security_mechanisms);
 }
 
-static int ast_sip_str_to_security_mechanism_type(const char *security_mechanism) {
-       int result = -1;
+static char *mechanism_str[] = {
+       [AST_SIP_SECURITY_MECH_NONE] = "none",
+       [AST_SIP_SECURITY_MECH_MSRP_TLS] = "msrp-tls",
+       [AST_SIP_SECURITY_MECH_SDES_SRTP] = "sdes-srtp",
+       [AST_SIP_SECURITY_MECH_DTLS_SRTP] = "dtls-srtp",
+};
 
-       if (!strcasecmp(security_mechanism, "msrp-tls")) {
-               result = AST_SIP_SECURITY_MECH_MSRP_TLS;
-       } else if (!strcasecmp(security_mechanism, "sdes-srtp")) {
-               result = AST_SIP_SECURITY_MECH_SDES_SRTP;
-       } else if (!strcasecmp(security_mechanism, "dtls-srtp")) {
-               result = AST_SIP_SECURITY_MECH_DTLS_SRTP;
-       }
 
-       return result;
-}
+static int str_to_security_mechanism_type(const char *security_mechanism) {
+       int i = 0;
 
-static char *ast_sip_security_mechanism_type_to_str(enum ast_sip_security_mechanism_type mech_type) {
-       if (mech_type == AST_SIP_SECURITY_MECH_MSRP_TLS) {
-               return "msrp-tls";
-       } else if (mech_type == AST_SIP_SECURITY_MECH_SDES_SRTP) {
-               return "sdes-srtp";
-       } else if (mech_type == AST_SIP_SECURITY_MECH_DTLS_SRTP) {
-               return "dtls-srtp";
-       } else {
-               return NULL;
+       for (i = 0; i < ARRAY_LEN(mechanism_str); i++) {
+               if (!strcasecmp(security_mechanism, mechanism_str[i])) {
+                       return i;
+               }
        }
+
+       return -1;
 }
 
 static int security_mechanism_to_str(const struct ast_sip_security_mechanism *security_mechanism, int add_qvalue, char **buf)
 {
        size_t size;
-       size_t buf_size = 128;
        int i;
-       char *ret = ast_calloc(buf_size, sizeof(char));
+       int rc = 0;
+       RAII_VAR(struct ast_str *, str, ast_str_create(MAX_OBJECT_FIELD), ast_free);
 
-       if (ret == NULL) {
+       if (str == NULL) {
                return ENOMEM;
        }
+
        if (security_mechanism == NULL) {
-               ast_free(ret);
                return EINVAL;
        }
 
-    snprintf(ret, buf_size - 1, "%s", ast_sip_security_mechanism_type_to_str(security_mechanism->type));
+       rc = ast_str_set(&str, 0, "%s", mechanism_str[security_mechanism->type]);
+       if (rc <= 0) {
+               return ENOMEM;
+       }
        if (add_qvalue) {
-               snprintf(ret + strlen(ret), buf_size - 1, ";q=%f.4", security_mechanism->qvalue);
+               rc = ast_str_append(&str, 0, ";q=%f.4", security_mechanism->qvalue);
+               if (rc <= 0) {
+                       return ENOMEM;
+               }
        }
 
        size = AST_VECTOR_SIZE(&security_mechanism->mechanism_parameters);
        for (i = 0; i < size; ++i) {
-               snprintf(ret + strlen(ret), buf_size - 1, ";%s", AST_VECTOR_GET(&security_mechanism->mechanism_parameters, i));
+               rc = ast_str_append(&str, 0, ";%s", AST_VECTOR_GET(&security_mechanism->mechanism_parameters, i));
+               if (rc <= 0) {
+                       return ENOMEM;
+               }
        }
 
-       *buf = ret;
+       *buf = ast_strdup(ast_str_buffer(str));
        return 0;
 }
 
@@ -171,27 +164,38 @@ int ast_sip_security_mechanisms_to_str(const struct ast_sip_security_mechanism_v
        size_t vec_size;
        struct ast_sip_security_mechanism *mech;
        char *tmp_buf;
-       char ret[512];
+       RAII_VAR(struct ast_str *, str, ast_str_create(MAX_OBJECT_FIELD), ast_free);
        size_t i;
+       int rc = 0;
+
+       if (str == NULL) {
+               return ENOMEM;
+       }
 
        if (!security_mechanisms) {
                return -1;
        }
 
        vec_size = AST_VECTOR_SIZE(security_mechanisms);
-       ret[0] = '\0';
+       if (vec_size == 0) {
+               return -1;
+       }
 
        for (i = 0; i < vec_size; ++i) {
                mech = AST_VECTOR_GET(security_mechanisms, i);
-               if (security_mechanism_to_str(mech, add_qvalue, &tmp_buf)) {
-                       continue;
+               rc = security_mechanism_to_str(mech, add_qvalue, &tmp_buf);
+               if (rc) {
+                       return rc;
                }
-               snprintf(ret + strlen(ret), sizeof(ret) - 1, "%s%s",
-                  tmp_buf, i == vec_size - 1 ? "" : ", ");
+               rc = ast_str_append(&str, 0, "%s, ", tmp_buf);
                ast_free(tmp_buf);
+               if (rc <= 0) {
+                       return ENOMEM;
+               }
        }
 
-       *buf = ast_strdup(ret);
+       /* ast_str_truncate removes the trailing ", " on the last mechanism */
+       *buf = ast_strdup(ast_str_truncate(str, -2));
 
        return 0;
 }
@@ -243,14 +247,14 @@ int ast_sip_str_to_security_mechanism(struct ast_sip_security_mechanism **securi
        int err = 0;
        int type = -1;
 
-       mech = ast_sip_security_mechanisms_alloc(1);
+       mech = security_mechanisms_alloc(1);
        if (!mech) {
                err = ENOMEM;
                goto out;
        }
 
        tmp = ast_strsep(&mechanism, ';', AST_STRSEP_ALL);
-       type = ast_sip_str_to_security_mechanism_type(tmp);
+       type = str_to_security_mechanism_type(tmp);
        if (type == -1) {
                err = EINVAL;
                goto out;
@@ -278,7 +282,7 @@ int ast_sip_str_to_security_mechanism(struct ast_sip_security_mechanism **securi
 
 out:
        if (err && (mech != NULL)) {
-               ast_sip_security_mechanisms_destroy(mech);
+               security_mechanism_destroy(mech);
        }
        return err;
 }

diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 4ea61615dec8e35bc3b3c666f946bc4ebfa29715..59a9fde5a5f6d9b4bcf191405cb3cb72e7dbed2c 100644 (file)
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -615,14 +615,14 @@ static int contact_has_security_mechanisms(void *obj, void *arg, int flags)
        struct ast_sip_contact_status *contact_status = ast_sip_get_contact_status(contact);
 
        if (!contact_status) {
-               return -1;
+               return 0;
        }
        if (!AST_VECTOR_SIZE(&contact_status->security_mechanisms)) {
                ao2_cleanup(contact_status);
-               return -1;
+               return 0;
        }
        *ret = contact_status;
-       return 0;
+       return CMP_MATCH | CMP_STOP;
 }
 
 static int contact_add_security_headers_to_status(void *obj, void *arg, int flags)
@@ -632,7 +632,7 @@ static int contact_add_security_headers_to_status(void *obj, void *arg, int flag
        struct ast_sip_contact_status *contact_status = ast_sip_get_contact_status(contact);
 
        if (!contact_status) {
-               return -1;
+               return 0;
        }
        if (AST_VECTOR_SIZE(&contact_status->security_mechanisms)) {
                goto out;
@@ -671,7 +671,7 @@ static void add_security_headers(struct sip_outbound_registration_client_state *
                /* Retrieve all contacts associated with aors from this endpoint
                 * and find the first one that has security mechanisms.
                 */
-               ao2_callback(contact_container, 0, contact_has_security_mechanisms, &contact_status);
+               ao2_callback(contact_container, OBJ_NODATA, contact_has_security_mechanisms, &contact_status);
                if (contact_status) {
                        ao2_lock(contact_status);
                        sec_mechs = &contact_status->security_mechanisms;