Skip to content
Permalink
Browse files Browse the repository at this point in the history
Use PJ_ASSERT_RETURN() on pjsip_auth_create_digest() and pjsua_init_t…
…pselector() (#3009)

* Use PJ_ASSERT_RETURN on pjsip_auth_create_digest

* Use PJ_ASSERT_RETURN on pjsua_init_tpselector()

* Fix incorrect check.

* Add return value to pjsip_auth_create_digest() and pjsip_auth_create_digestSHA256()

* Modification based on comments.
  • Loading branch information
trengginas committed Mar 8, 2022
1 parent 94886d7 commit d27f79d
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 44 deletions.
48 changes: 39 additions & 9 deletions pjsip/include/pjsip/sip_auth.h
Expand Up @@ -584,16 +584,46 @@ PJ_DECL(pj_status_t) pjsip_auth_srv_challenge( pjsip_auth_srv *auth_srv,
* @param realm Realm.
* @param cred_info Credential info.
* @param method SIP method.
*
* @return PJ_SUCCESS on success.
*/
PJ_DECL(void) pjsip_auth_create_digest(pj_str_t *result,
const pj_str_t *nonce,
const pj_str_t *nc,
const pj_str_t *cnonce,
const pj_str_t *qop,
const pj_str_t *uri,
const pj_str_t *realm,
const pjsip_cred_info *cred_info,
const pj_str_t *method);
PJ_DECL(pj_status_t) pjsip_auth_create_digest(pj_str_t *result,
const pj_str_t *nonce,
const pj_str_t *nc,
const pj_str_t *cnonce,
const pj_str_t *qop,
const pj_str_t *uri,
const pj_str_t *realm,
const pjsip_cred_info *cred_info,
const pj_str_t *method);

/**
* Helper function to create SHA-256 digest out of the specified
* parameters.
*
* @param result String to store the response digest. This string
* must have been preallocated by caller with the
* buffer at least PJSIP_SHA256STRLEN (64 bytes) in size.
* @param nonce Optional nonce.
* @param nc Nonce count.
* @param cnonce Optional cnonce.
* @param qop Optional qop.
* @param uri URI.
* @param realm Realm.
* @param cred_info Credential info.
* @param method SIP method.
*
* @return PJ_SUCCESS on success.
*/
PJ_DEF(pj_status_t) pjsip_auth_create_digestSHA256(pj_str_t* result,
const pj_str_t* nonce,
const pj_str_t* nc,
const pj_str_t* cnonce,
const pj_str_t* qop,
const pj_str_t* uri,
const pj_str_t* realm,
const pjsip_cred_info* cred_info,
const pj_str_t* method);

/**
* @}
Expand Down
21 changes: 11 additions & 10 deletions pjsip/src/pjsip/sip_auth_aka.c
Expand Up @@ -57,18 +57,19 @@ PJ_DEF(pj_status_t) pjsip_auth_create_aka_response(
pj_uint8_t xmac[PJSIP_AKA_MACLEN];
pjsip_cred_info aka_cred;
int i, len;
pj_status_t status;
pj_status_t status = PJ_SUCCESS;

/* Check the algorithm is supported. */
if (chal->algorithm.slen==0 || pj_stricmp2(&chal->algorithm, "md5") == 0) {
/*
* A normal MD5 authentication is requested. Fallbackt to the usual
* A normal MD5 authentication is requested. Fallback to the usual
* MD5 digest creation.
*/
pjsip_auth_create_digest(&auth->response, &auth->nonce, &auth->nc,
&auth->cnonce, &auth->qop, &auth->uri,
&auth->realm, cred, method);
return PJ_SUCCESS;
status = pjsip_auth_create_digest(&auth->response, &auth->nonce,
&auth->nc, &auth->cnonce, &auth->qop,
&auth->uri, &auth->realm, cred, method);

return status;

} else if (pj_stricmp(&chal->algorithm, &pjsip_AKAv1_MD5) == 0) {
/*
Expand Down Expand Up @@ -147,9 +148,9 @@ PJ_DEF(pj_status_t) pjsip_auth_create_aka_response(
aka_cred.data.ptr = (char*)res;
aka_cred.data.slen = PJSIP_AKA_RESLEN;

pjsip_auth_create_digest(&auth->response, &chal->nonce,
status = pjsip_auth_create_digest(&auth->response, &chal->nonce,
&auth->nc, &auth->cnonce, &auth->qop,
&auth->uri, &chal->realm, &aka_cred, method);
&auth->uri, &chal->realm, &aka_cred, method);

} else if (aka_version == 2) {

Expand Down Expand Up @@ -186,7 +187,7 @@ PJ_DEF(pj_status_t) pjsip_auth_create_aka_response(
aka_cred.data.ptr, &len);
aka_cred.data.slen = hmac64_len;

pjsip_auth_create_digest(&auth->response, &chal->nonce,
status = pjsip_auth_create_digest(&auth->response, &chal->nonce,
&auth->nc, &auth->cnonce, &auth->qop,
&auth->uri, &chal->realm, &aka_cred, method);

Expand All @@ -196,7 +197,7 @@ PJ_DEF(pj_status_t) pjsip_auth_create_aka_response(
}

/* Done */
return PJ_SUCCESS;
return status;
}


Expand Down
65 changes: 42 additions & 23 deletions pjsip/src/pjsip/sip_auth_client.c
Expand Up @@ -160,15 +160,15 @@ static void digestNtoStr(const unsigned char digest[], int n, char *output)
* Create response digest based on the parameters and store the
* digest ASCII in 'result'.
*/
PJ_DEF(void) pjsip_auth_create_digest( pj_str_t *result,
const pj_str_t *nonce,
const pj_str_t *nc,
const pj_str_t *cnonce,
const pj_str_t *qop,
const pj_str_t *uri,
const pj_str_t *realm,
const pjsip_cred_info *cred_info,
const pj_str_t *method)
PJ_DEF(pj_status_t) pjsip_auth_create_digest( pj_str_t *result,
const pj_str_t *nonce,
const pj_str_t *nc,
const pj_str_t *cnonce,
const pj_str_t *qop,
const pj_str_t *uri,
const pj_str_t *realm,
const pjsip_cred_info *cred_info,
const pj_str_t *method)
{
char ha1[PJSIP_MD5STRLEN];
char ha2[PJSIP_MD5STRLEN];
Expand All @@ -194,10 +194,18 @@ PJ_DEF(void) pjsip_auth_create_digest( pj_str_t *result,
digestNtoStr(digest, 16, ha1);

} else if ((cred_info->data_type & PASSWD_MASK) == PJSIP_CRED_DATA_DIGEST) {
pj_assert(cred_info->data.slen == 32);
if (cred_info->data.slen != 32) {
pj_assert(!"Invalid cred_info data length");
pj_bzero(result->ptr, result->slen);
result->slen = 0;
return PJ_EINVAL;
}
pj_memcpy( ha1, cred_info->data.ptr, cred_info->data.slen );
} else {
pj_assert(!"Invalid data_type");
pj_bzero(result->ptr, result->slen);
result->slen = 0;
return PJ_EINVAL;
}

AUTH_TRACE_((THIS_FILE, " ha1=%.32s", ha1));
Expand Down Expand Up @@ -245,14 +253,15 @@ PJ_DEF(void) pjsip_auth_create_digest( pj_str_t *result,

AUTH_TRACE_((THIS_FILE, " digest=%.32s", result->ptr));
AUTH_TRACE_((THIS_FILE, "Digest created"));
return PJ_SUCCESS;
}


/*
* Create response SHA-256 digest based on the parameters and store the
* digest ASCII in 'result'.
*/
PJ_DEF(void) pjsip_auth_create_digestSHA256(pj_str_t *result,
PJ_DEF(pj_status_t) pjsip_auth_create_digestSHA256(pj_str_t *result,
const pj_str_t *nonce,
const pj_str_t *nc,
const pj_str_t *cnonce,
Expand Down Expand Up @@ -291,10 +300,18 @@ PJ_DEF(void) pjsip_auth_create_digestSHA256(pj_str_t *result,

} else if ((cred_info->data_type & PASSWD_MASK) == PJSIP_CRED_DATA_DIGEST)
{
pj_assert(cred_info->data.slen == 32);
if (cred_info->data.slen != 64) {
pj_assert(!"Invalid cred_info data length");
pj_bzero(result->ptr, result->slen);
result->slen = 0;
return PJ_EINVAL;
}
pj_memcpy( ha1, cred_info->data.ptr, cred_info->data.slen );
} else {
pj_assert(!"Invalid data_type");
pj_bzero(result->ptr, result->slen);
result->slen = 0;
return PJ_EINVAL;
}

AUTH_TRACE_((THIS_FILE, " ha1=%.64s", ha1));
Expand Down Expand Up @@ -354,6 +371,7 @@ PJ_DEF(void) pjsip_auth_create_digestSHA256(pj_str_t *result,
PJ_UNUSED_ARG(cred_info);
PJ_UNUSED_ARG(method);
#endif
return PJ_SUCCESS;
}


Expand Down Expand Up @@ -408,6 +426,7 @@ static pj_status_t respond_digest( pj_pool_t *pool,
{
const pj_str_t pjsip_AKAv1_MD5_STR = { "AKAv1-MD5", 9 };
pj_bool_t algo_sha256 = PJ_FALSE;
pj_status_t status = PJ_SUCCESS;

/* Check if algo is sha256 */
#if PJSIP_AUTH_HAS_DIGEST_SHA256
Expand Down Expand Up @@ -452,14 +471,14 @@ static pj_status_t respond_digest( pj_pool_t *pool,
else {
/* Convert digest to string and store in chal->response. */
if (algo_sha256) {
pjsip_auth_create_digestSHA256(
status = pjsip_auth_create_digestSHA256(
&cred->response, &cred->nonce, NULL,
NULL, NULL, uri, &chal->realm,
cred_info, method);
} else {
pjsip_auth_create_digest( &cred->response, &cred->nonce, NULL,
NULL, NULL, uri, &chal->realm,
cred_info, method);
status = pjsip_auth_create_digest( &cred->response,
&cred->nonce, NULL, NULL, NULL, uri,
&chal->realm, cred_info, method);
}
}

Expand All @@ -486,18 +505,18 @@ static pj_status_t respond_digest( pj_pool_t *pool,
else {
/* Convert digest to string and store in chal->response. */
if (algo_sha256) {
pjsip_auth_create_digestSHA256(
status = pjsip_auth_create_digestSHA256(
&cred->response, &cred->nonce,
&cred->nc, &cred->cnonce,
&pjsip_AUTH_STR, uri,
&chal->realm, cred_info,
method);
} else {
pjsip_auth_create_digest( &cred->response, &cred->nonce,
&cred->nc, &cred->cnonce,
&pjsip_AUTH_STR, uri,
&chal->realm, cred_info,
method);
status = pjsip_auth_create_digest( &cred->response,
&cred->nonce, &cred->nc,
&cred->cnonce, &pjsip_AUTH_STR,
uri, &chal->realm,
cred_info, method);
}
}

Expand All @@ -508,7 +527,7 @@ static pj_status_t respond_digest( pj_pool_t *pool,
return PJSIP_EINVALIDQOP;
}

return PJ_SUCCESS;
return status;
}

#if defined(PJSIP_AUTH_QOP_SUPPORT) && PJSIP_AUTH_QOP_SUPPORT!=0
Expand Down
6 changes: 5 additions & 1 deletion pjsip/src/pjsip/sip_auth_server.c
Expand Up @@ -79,6 +79,7 @@ static pj_status_t pjsip_auth_verify( const pjsip_authorization_hdr *hdr,
if (pj_stricmp(&hdr->scheme, &pjsip_DIGEST_STR) == 0) {
char digest_buf[PJSIP_MD5STRLEN];
pj_str_t digest;
pj_status_t status;
const pjsip_digest_credential *dig = &hdr->credential.digest;

/* Check that username and realm match.
Expand All @@ -95,7 +96,7 @@ static pj_status_t pjsip_auth_verify( const pjsip_authorization_hdr *hdr,
digest.slen = PJSIP_MD5STRLEN;

/* Create digest for comparison. */
pjsip_auth_create_digest(&digest,
status = pjsip_auth_create_digest(&digest,
&hdr->credential.digest.nonce,
&hdr->credential.digest.nc,
&hdr->credential.digest.cnonce,
Expand All @@ -105,6 +106,9 @@ static pj_status_t pjsip_auth_verify( const pjsip_authorization_hdr *hdr,
cred_info,
method );

if (status != PJ_SUCCESS)
return status;

/* Compare digest. */
return (pj_stricmp(&digest, &hdr->credential.digest.response) == 0) ?
PJ_SUCCESS : PJSIP_EAUTHINVALIDDIGEST;
Expand Down
3 changes: 2 additions & 1 deletion pjsip/src/pjsua-lib/pjsua_core.c
Expand Up @@ -3126,7 +3126,8 @@ void pjsua_init_tpselector(pjsua_transport_id tp_id,
if (tp_id == PJSUA_INVALID_ID)
return;

pj_assert(tp_id >= 0 && tp_id < (int)PJ_ARRAY_SIZE(pjsua_var.tpdata));
PJ_ASSERT_RETURN(tp_id >= 0 &&
tp_id < (int)PJ_ARRAY_SIZE(pjsua_var.tpdata), );
tpdata = &pjsua_var.tpdata[tp_id];

flag = pjsip_transport_get_flag_from_type(tpdata->type);
Expand Down

1 comment on commit d27f79d

@FredSE2021
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pjsip/src/pjsua-lib/pjsua_core.c
PJ_ASSERT_RETURN(tp_id >= 0 && tp_id < (int)PJ_ARRAY_SIZE(pjsua_var.tpdata), );
There is last ", )", is that OK?

Please sign in to comment.