[asterisk-commits] mmichelson: branch mmichelson/authenticate r381061 - /team/mmichelson/authent...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Feb 7 16:38:43 CST 2013


Author: mmichelson
Date: Thu Feb  7 16:38:40 2013
New Revision: 381061

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=381061
Log:
Completely change the scheme for verifying authentication credentials.

Due to the pure stateless manner in which things had been done, Joshua
had pointed out that we were susceptible to replay attacks. My initial
reaction to this was to send a randomized nonce in challenges, and then
save this value in a container.

This approach presents several issues:
1) Having stuff in a container means there is a contention point between
SIP threads. While testing has not shown this to be a problem, it's easy
to see how this might be a performance bottleneck.
2) There is a tremendous amount of overhead involved in making sure to
expire challenges within a certain amount of time.

After a conversation with Joshua, we now have a new approach.

Now we generate a nonce that is based on a hash of the timestamp, source
IP address and port, an entity ID, and the realm. When we receive an
INVITE with authentication info, we can calculate the nonce a second
time and compare with the nonce they provide in their credentials
to be sure it seems sane.



Modified:
    team/mmichelson/authenticate/res/res_sip_authenticator_digest.c

Modified: team/mmichelson/authenticate/res/res_sip_authenticator_digest.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/authenticate/res/res_sip_authenticator_digest.c?view=diff&rev=381061&r1=381060&r2=381061
==============================================================================
--- team/mmichelson/authenticate/res/res_sip_authenticator_digest.c (original)
+++ team/mmichelson/authenticate/res/res_sip_authenticator_digest.c Thu Feb  7 16:38:40 2013
@@ -24,114 +24,14 @@
 #include "asterisk/res_sip.h"
 #include "asterisk/logger.h"
 #include "asterisk/module.h"
-#include "asterisk/sched.h"
+#include "asterisk/strings.h"
 
 /*** MODULEINFO
 	<depend>res_sip</depend>
 	<support_level>core</support_level>
  ***/
 
-#define CHALLENGE_BUCKETS 83
-#define AUTH_TIMEOUT 32000
-
-struct ast_sched_context *auth_sched;
-
-static pj_status_t digest_lookup(pj_pool_t *pool, const pj_str_t *realm,
-		const pj_str_t *acc_name, pjsip_cred_info *info);
-
-struct ao2_container *challenges;
-
-static int response_id_counter;
-
-/*!
- * \brief data used to help match incoming credentials to an authentication challenge we sent
- *
- * This challenge represents a WWW-Authenticate header sent in a 401 response. Its keys are
- * the nonce and the realm. Using these two items, we can be sure to find if incoming credentials
- * match a challenge we sent
- *
- * The response_id is an integer that helps to identify which 401 response the challenge was sent
- * on. Since we can send multiple authentication challenges in a single response, multiple
- * challenges can have the same response_id.
- */
-struct challenge {
-	/*! The nonce sent in the WWW-Authenticate header */
-	char nonce[AST_UUID_STR_LEN];
-	/*! The realm sent in the WWW-Authenticate header */
-	const char *realm;
-	/*! The response in which this challenge was sent */
-	unsigned int response_id;
-};
-
-static int challenge_hash(const void *obj, int flags)
-{
-	const struct challenge *chall = obj;
-	int hash = ast_str_hash(chall->nonce);
-	hash = ast_str_hash_add(chall->realm, hash);
-	return hash;
-}
-
-static int challenge_cmp(void *obj, void *arg, int flags)
-{
-	struct challenge *chall1 = obj;
-	struct challenge *chall2 = arg;
-
-	return (!strcmp(chall1->nonce, chall2->nonce) && !strcmp(chall1->realm, chall2->realm)) ? CMP_MATCH : 0;
-}
-
-static void challenge_destructor(void *obj)
-{
-	struct challenge *chall = obj;
-
-	ast_free((char *) chall->realm);
-}
-
-static struct challenge *create_challenge(struct ast_sip_auth *auth, int response_id)
-{
-	struct challenge *chall = ao2_alloc(sizeof(*chall), challenge_destructor);
-	struct ast_uuid *uuid;
-
-	if (!chall) {
-		return NULL;
-	}
-
-	uuid = ast_uuid_generate();
-	if (!uuid) {
-		ao2_cleanup(chall);
-		return NULL;
-	}
-	ast_uuid_to_str(uuid, chall->nonce, sizeof(chall->nonce));
-
-	chall->realm = ast_strdup(auth->realm);
-	if (!chall->realm) {
-		ao2_cleanup(chall);
-		return NULL;
-	}
-	chall->response_id = response_id;
-
-	return chall;
-}
-
-static int match_response_id(void *obj, void *arg, int flags)
-{
-	struct challenge *chall = obj;
-	int *response_id = arg;
-
-	return chall->response_id == *response_id ? CMP_MATCH : 0;
-}
-
-static void remove_challenges(int response_id)
-{
-	ao2_callback(challenges, OBJ_MULTIPLE | OBJ_UNLINK | OBJ_NODATA, match_response_id, &response_id);
-}
-
-static int auth_timeout(const void *data)
-{
-	const int *response_id = data;
-	remove_challenges(*response_id);
-	ast_free((int *) response_id);
-	return 0;
-}
+char entity_id[AST_UUID_STR_LEN];
 
 /*!
  * \brief Determine if authentication is required
@@ -274,25 +174,59 @@
 	return PJ_SUCCESS;
 }
 
-static struct challenge *find_challenge(pjsip_rx_data *rdata, struct ast_sip_auth *auth)
+static int build_nonce(struct ast_str **nonce, char *timestamp, pjsip_rx_data *rdata, const char *realm)
+{
+	struct ast_str *str = ast_str_create(64);
+	char hash[32];
+
+	ast_str_append(&str, 0, "%s", timestamp);
+	ast_str_append(&str, 0, ":%s", rdata->pkt_info.src_name);
+	ast_str_append(&str, 0, ":%d", rdata->pkt_info.src_port);
+	ast_str_append(&str, 0, ":%s", entity_id);
+	ast_str_append(&str, 0, ":%s", realm);
+	ast_md5_hash(hash, ast_str_buffer(str));
+
+	ast_str_append(nonce, 0, "%s/%s\n", timestamp, hash);
+	return 0;
+}
+
+static int check_nonce(const char *candidate, pjsip_rx_data *rdata, const char *realm)
+{
+	char *copy = ast_strdupa(candidate);
+	char *timestamp = strsep(&copy, "/");
+	struct ast_str *calculated = ast_str_create(64);
+	if (!copy) {
+		/* Clearly a bad nonce! */
+		return 0;
+	}
+
+	build_nonce(&calculated, timestamp, rdata, realm);
+	ast_debug(3, "Calculated nonce %s. Actual nonce is %s\n", ast_str_buffer(calculated), candidate);
+	if (strcmp(ast_str_buffer(calculated), candidate)) {
+		return 0;
+	}
+	return 1;
+}
+
+static int find_challenge(pjsip_rx_data *rdata, struct ast_sip_auth *auth)
 {
 	struct pjsip_authorization_hdr *auth_hdr = (pjsip_authorization_hdr *) &rdata->msg_info.msg->hdr;
-	struct challenge *chall = NULL;
-	struct challenge finder;
-
-	finder.realm = auth->realm;
+	int challenge_found = 0;
+	char nonce[64];
 
 	for (auth_hdr = (pjsip_authorization_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_AUTHORIZATION, auth_hdr->next);
 			auth_hdr != (pjsip_authorization_hdr *) &rdata->msg_info.msg->hdr;
 			auth_hdr = (pjsip_authorization_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_AUTHORIZATION,  auth_hdr->next)) {
-		ast_copy_pj_str(finder.nonce, &auth_hdr->credential.digest.nonce, sizeof(finder.nonce));
-		chall = ao2_find(challenges, &finder, OBJ_POINTER);
-		if (chall) {
+		/* Let's do a nonce check! */
+		ast_copy_pj_str(nonce, &auth_hdr->credential.digest.nonce, sizeof(nonce));
+		if (check_nonce(nonce, rdata, auth->realm) && !pj_strcmp2(&auth_hdr->credential.digest.realm, auth->realm)) {
+			/* Sane nonce found! */
+			challenge_found = 1;
 			break;
 		}
 	}
 
-	return chall;
+	return challenge_found;
 }
 
 /*!
@@ -317,7 +251,6 @@
  */
 static int verify(void *obj, void *arg, void *data, int flags)
 {
-	RAII_VAR(struct challenge *, chall, NULL, ao2_cleanup);
 	struct ast_sip_auth *auth = obj;
 	pj_pool_t *pool = data;
 	pjsip_rx_data *rdata = arg;
@@ -325,8 +258,9 @@
 	int response_code;
 	pjsip_auth_srv auth_server;
 
-	chall = find_challenge(rdata, auth);
-	if (!chall) {
+	if (!find_challenge(rdata, auth)) {
+		/* Couldn't find a challenge with a sane nonce */
+		/* XXX It may be worthwhile to add some "stale" checks here */
 		return 0;
 	}
 
@@ -338,12 +272,6 @@
 
 	remove_auth();
 
-	if (authed) {
-		/* Hey neat! They managed to authenticate. Now we need to delete
-		 * all challenges with the winning challenge's response ID
-		 */
-		remove_challenges(chall->response_id);
-	}
 	return authed == PJ_SUCCESS ? CMP_MATCH : 0;
 }
 
@@ -357,23 +285,24 @@
 static int challenge(void *obj, void *arg, void *data, int flags)
 {
 	struct ast_sip_auth *auth = obj;
-	RAII_VAR(struct challenge *, chall, NULL, ao2_cleanup);
 	pjsip_tx_data *tdata = arg;
+	pjsip_rx_data *rdata = data;
 	pj_str_t qop;
-	pj_str_t nonce;
+	pj_str_t pj_nonce;
 	pjsip_auth_srv auth_server;
-	int *response_id = data;
-
-	chall = create_challenge(auth, *response_id);
-	if (!chall) {
-		return 0;
-	}
+	struct ast_str *nonce;
+	char time_buf[32];
+	time_t timestamp = time(NULL);
+	snprintf(time_buf, sizeof(time_buf), "%d", (int) timestamp);
+
+	nonce = ast_str_create(64);
+	build_nonce(&nonce, time_buf, rdata, auth->realm);
 
 	setup_auth_srv(tdata->pool, &auth_server, auth);
 
-	pj_cstr(&nonce, chall->nonce);
+	pj_cstr(&pj_nonce, ast_str_buffer(nonce));
 	pj_cstr(&qop, "auth");
-	pjsip_auth_srv_challenge(&auth_server, &qop, &nonce, NULL, PJ_FALSE, tdata);
+	pjsip_auth_srv_challenge(&auth_server, &qop, &pj_nonce, NULL, PJ_FALSE, tdata);
 	return 0;
 }
 
@@ -390,7 +319,6 @@
 		pjsip_rx_data *rdata, pjsip_tx_data *tdata)
 {
 	struct ast_sip_auth *auth;
-	int *response_id;
 
 	auth = ao2_callback_data(endpoint->sip_auths, 0, verify, rdata, tdata->pool);
 	if (auth) {
@@ -398,12 +326,7 @@
 		return AST_SIP_AUTHENTICATION_SUCCESS;
 	}
 	
-	response_id = ast_malloc(sizeof(*response_id));
-	*response_id = ast_atomic_fetchadd_int(&response_id_counter, +1);
-	ao2_callback_data(endpoint->sip_auths, 0, challenge, tdata, response_id);
-	if (ast_sched_add(auth_sched, AUTH_TIMEOUT, auth_timeout, response_id) == -1) {
-		return AST_SIP_AUTHENTICATION_ERROR;
-	}
+	ao2_callback_data(endpoint->sip_auths, 0, challenge, tdata, rdata);
 	return AST_SIP_AUTHENTICATION_CHALLENGE;
 }
 
@@ -412,34 +335,30 @@
 	.check_authentication = digest_check_auth,
 };
 
+static int build_entity_id(void)
+{
+	struct ast_uuid *uu = ast_uuid_generate();
+	if (!uu) {
+		return -1;
+	}
+	ast_uuid_to_str(uu, entity_id, sizeof(entity_id));
+	return 0;
+}
+
 static int load_module(void)
 {
-	auth_sched = ast_sched_context_create();
-	if (!auth_sched) {
+	if (build_entity_id()) {
 		return AST_MODULE_LOAD_DECLINE;
 	}
-	if (ast_sched_start_thread(auth_sched)) {
-		ast_sched_context_destroy(auth_sched);
+	if (ast_sip_register_authenticator(&digest_authenticator)) {
 		return AST_MODULE_LOAD_DECLINE;
 	}
-	challenges = ao2_container_alloc(CHALLENGE_BUCKETS, challenge_hash, challenge_cmp);
-	if (!challenges) {
-		ast_sched_context_destroy(auth_sched);
-		return AST_MODULE_LOAD_DECLINE;
-	}
-	if (ast_sip_register_authenticator(&digest_authenticator)) {
-		ao2_cleanup(challenges);
-		ast_sched_context_destroy(auth_sched);
-		return AST_MODULE_LOAD_DECLINE;
-	}
 	return AST_MODULE_LOAD_SUCCESS;
 }
 
 static int unload_module(void)
 {
-	ao2_cleanup(challenges);
 	ast_sip_unregister_authenticator(&digest_authenticator);
-	ast_sched_context_destroy(auth_sched);
 	return 0;
 }
 




More information about the asterisk-commits mailing list