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: 
pull/866/head
George Joseph 9 months ago committed by asterisk-org-access-app[bot]
parent 2acd9982de
commit e203c227bd

@ -2434,6 +2434,7 @@ static void endpoint_destructor(void* obj)
ast_free(endpoint->contact_user); ast_free(endpoint->contact_user);
ast_free_acl_list(endpoint->contact_acl); ast_free_acl_list(endpoint->contact_acl);
ast_free_acl_list(endpoint->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) static int init_subscription_configuration(struct ast_sip_endpoint_subscription_configuration *subscription)

@ -30,7 +30,7 @@
#include "asterisk/res_pjsip.h" #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; struct ast_sip_security_mechanism *mech;
@ -47,7 +47,7 @@ static struct ast_sip_security_mechanism *ast_sip_security_mechanisms_alloc(size
return mech; 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) const struct ast_sip_security_mechanism *src)
{ {
struct ast_sip_security_mechanism *dst = NULL; 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); 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) { if (dst == NULL) {
return NULL; return NULL;
} }
@ -71,13 +71,9 @@ static struct ast_sip_security_mechanism *ast_sip_security_mechanisms_copy(
return dst; 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; AST_VECTOR_RESET(&mech->mechanism_parameters, ast_free);
for (i = 0; i < AST_VECTOR_SIZE(&mech->mechanism_parameters); i++) {
ast_free(AST_VECTOR_GET(&mech->mechanism_parameters, i));
}
AST_VECTOR_FREE(&mech->mechanism_parameters); AST_VECTOR_FREE(&mech->mechanism_parameters);
ast_free(mech); 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); ast_sip_security_mechanisms_vector_destroy(dst);
for (i = 0; i < AST_VECTOR_SIZE(src); i++) { for (i = 0; i < AST_VECTOR_SIZE(src); i++) {
mech = AST_VECTOR_GET(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) 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) { if (!security_mechanisms) {
return; return;
} }
for (i = 0; i < AST_VECTOR_SIZE(security_mechanisms); i++) { AST_VECTOR_RESET(security_mechanisms, security_mechanism_destroy);
mech = AST_VECTOR_GET(security_mechanisms, i);
ast_sip_security_mechanisms_destroy(mech);
}
AST_VECTOR_FREE(security_mechanisms); AST_VECTOR_FREE(security_mechanisms);
} }
static int ast_sip_str_to_security_mechanism_type(const char *security_mechanism) { static char *mechanism_str[] = {
int result = -1; [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) { for (i = 0; i < ARRAY_LEN(mechanism_str); i++) {
if (mech_type == AST_SIP_SECURITY_MECH_MSRP_TLS) { if (!strcasecmp(security_mechanism, mechanism_str[i])) {
return "msrp-tls"; return i;
} 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;
} }
}
return -1;
} }
static int security_mechanism_to_str(const struct ast_sip_security_mechanism *security_mechanism, int add_qvalue, char **buf) static int security_mechanism_to_str(const struct ast_sip_security_mechanism *security_mechanism, int add_qvalue, char **buf)
{ {
size_t size; size_t size;
size_t buf_size = 128;
int i; 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; return ENOMEM;
} }
if (security_mechanism == NULL) { if (security_mechanism == NULL) {
ast_free(ret);
return EINVAL; 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) { 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); size = AST_VECTOR_SIZE(&security_mechanism->mechanism_parameters);
for (i = 0; i < size; ++i) { 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; return 0;
} }
@ -171,27 +164,38 @@ int ast_sip_security_mechanisms_to_str(const struct ast_sip_security_mechanism_v
size_t vec_size; size_t vec_size;
struct ast_sip_security_mechanism *mech; struct ast_sip_security_mechanism *mech;
char *tmp_buf; char *tmp_buf;
char ret[512]; RAII_VAR(struct ast_str *, str, ast_str_create(MAX_OBJECT_FIELD), ast_free);
size_t i; size_t i;
int rc = 0;
if (str == NULL) {
return ENOMEM;
}
if (!security_mechanisms) { if (!security_mechanisms) {
return -1; return -1;
} }
vec_size = AST_VECTOR_SIZE(security_mechanisms); vec_size = AST_VECTOR_SIZE(security_mechanisms);
ret[0] = '\0'; if (vec_size == 0) {
return -1;
}
for (i = 0; i < vec_size; ++i) { for (i = 0; i < vec_size; ++i) {
mech = AST_VECTOR_GET(security_mechanisms, i); mech = AST_VECTOR_GET(security_mechanisms, i);
if (security_mechanism_to_str(mech, add_qvalue, &tmp_buf)) { rc = security_mechanism_to_str(mech, add_qvalue, &tmp_buf);
continue; if (rc) {
return rc;
} }
snprintf(ret + strlen(ret), sizeof(ret) - 1, "%s%s", rc = ast_str_append(&str, 0, "%s, ", tmp_buf);
tmp_buf, i == vec_size - 1 ? "" : ", ");
ast_free(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; return 0;
} }
@ -243,14 +247,14 @@ int ast_sip_str_to_security_mechanism(struct ast_sip_security_mechanism **securi
int err = 0; int err = 0;
int type = -1; int type = -1;
mech = ast_sip_security_mechanisms_alloc(1); mech = security_mechanisms_alloc(1);
if (!mech) { if (!mech) {
err = ENOMEM; err = ENOMEM;
goto out; goto out;
} }
tmp = ast_strsep(&mechanism, ';', AST_STRSEP_ALL); 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) { if (type == -1) {
err = EINVAL; err = EINVAL;
goto out; goto out;
@ -278,7 +282,7 @@ int ast_sip_str_to_security_mechanism(struct ast_sip_security_mechanism **securi
out: out:
if (err && (mech != NULL)) { if (err && (mech != NULL)) {
ast_sip_security_mechanisms_destroy(mech); security_mechanism_destroy(mech);
} }
return err; return err;
} }

@ -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); struct ast_sip_contact_status *contact_status = ast_sip_get_contact_status(contact);
if (!contact_status) { if (!contact_status) {
return -1; return 0;
} }
if (!AST_VECTOR_SIZE(&contact_status->security_mechanisms)) { if (!AST_VECTOR_SIZE(&contact_status->security_mechanisms)) {
ao2_cleanup(contact_status); ao2_cleanup(contact_status);
return -1; return 0;
} }
*ret = contact_status; *ret = contact_status;
return 0; return CMP_MATCH | CMP_STOP;
} }
static int contact_add_security_headers_to_status(void *obj, void *arg, int flags) 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); struct ast_sip_contact_status *contact_status = ast_sip_get_contact_status(contact);
if (!contact_status) { if (!contact_status) {
return -1; return 0;
} }
if (AST_VECTOR_SIZE(&contact_status->security_mechanisms)) { if (AST_VECTOR_SIZE(&contact_status->security_mechanisms)) {
goto out; 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 /* Retrieve all contacts associated with aors from this endpoint
* and find the first one that has security mechanisms. * 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) { if (contact_status) {
ao2_lock(contact_status); ao2_lock(contact_status);
sec_mechs = &contact_status->security_mechanisms; sec_mechs = &contact_status->security_mechanisms;

Loading…
Cancel
Save