Commit cf5ae1d4 authored by Pekka Pessi's avatar Pekka Pessi

Fixed memory management problems in iptsec module.

The authenticator client in auth_client.c leaked memory when
re-challenged. The client did not duplicate strings from
challenge, and tried to use freed values after challenge was
freed.
Now we are actually running the tests in test_auth_digest.c, too.
The problem was reported and patch submitted by Colin Whittaker.

darcs-hash:20060502152735-65a35-b8b44614125f15084464588719afe9d595354dc4.gz
parent fb762c4b
......@@ -78,6 +78,7 @@ Bugs fixed in this release
- other bugs as fixed in CVS/darcs
/>
- Fixed memory management problems in iptsec, reported by Colin Whittaker.
- Fixed some locking problems in sresolv, reported by Thomas Rosenblatt
- Fixed nua_handle_t leaks in various cases, reported by Colin Whittaker
- Fixed #1468407 nua_unregister() crashing when called without preceding
......
......@@ -51,13 +51,13 @@ HTTPSOURCES = auth_module_http.c
COVERAGE_INPUT = $(libiptsec_la_SOURCES) $(include_sofia_HEADERS)
LDADD = libiptsec.la \
../ipt/libipt.la \
../http/libhttp.la \
../msg/libmsg.la \
../nta/libnta.la \
../sip/libsip.la \
../msg/libmsg.la \
../url/liburl.la \
../bnf/libbnf.la \
../ipt/libipt.la \
../su/libsu.la
test_auth_digest_LDFLAGS = -static
......
......@@ -183,34 +183,36 @@ int auc_challenge(auth_client_t **auc_list,
auth_client_t **cca;
int retval = 0;
/* Go through each challenge in Authenticate or Proxy-Authenticate headers */
for (; ch; ch = ch->au_next) {
char const *scheme = ch->au_scheme;
char const *realm = msg_header_find_param(ch->au_common, "realm=");
int updated = 0, updated0;
int matched = 0, updated;
if (!scheme || !realm)
continue;
/* Update matching authenticator */
for (cca = auc_list; (*cca); cca = &(*cca)->ca_next) {
updated0 = ca_challenge((*cca), ch, crcl, scheme, realm);
if (updated0 < 0)
continue;
updated = 1;
retval = retval || updated0 > 0;
updated = ca_challenge((*cca), ch, crcl, scheme, realm);
if (updated < 0)
continue; /* No match, next */
matched = 1;
if (updated > 0)
retval = 1; /* Updated authenticator */
}
if (!updated) {
if (!matched) {
/* There was no matching authenticator, create a new one */
*cca = ca_create(home);
if (ca_challenge((*cca), ch, crcl, scheme, realm) != -1) {
updated = 1;
} else {
retval = 1; /* Updated authenticator */
}
else {
ca_destroy(home, *cca), *cca = NULL;
retval = -1;
break;
return -1;
}
}
retval = retval || updated;
}
return retval;
......@@ -270,11 +272,14 @@ int ca_challenge(auth_client_t *ca,
ca->ca_scheme = ca->ca_challenge->au_scheme;
ca->ca_realm = msg_header_find_param(ca->ca_challenge->au_common, "realm=");
#else
ca->ca_scheme = su_strdup(home, ch->au_scheme);
ca->ca_realm = su_strdup(home,
msg_header_find_param(ch->au_common, "realm"));
if (!ca->ca_scheme[0])
ca->ca_scheme = su_strdup(home, scheme);
if (!ca->ca_realm[0])
ca->ca_realm = su_strdup(home, realm);
#endif
auth_digest_challenge_free_params(home, ac);
if (auth_digest_challenge_get(home, ac, ch->au_params) < 0)
return -1;
......@@ -285,16 +290,18 @@ int ca_challenge(auth_client_t *ca,
return -1;
if (ac->ac_qop && (ca->ca_cnonce == NULL || ac->ac_stale)) {
/* XXX - generate cnonce */
su_guid_t guid[1];
char *cnonce;
if (ca->ca_cnonce != NULL)
/* Free the old one if we are updating after stale=true */
su_free(home, (void *)ca->ca_cnonce);
su_guid_generate(guid);
ca->ca_cnonce = cnonce = su_alloc(home, BASE64_SIZE(sizeof(guid)) + 1);
base64_e(cnonce, BASE64_SIZE(sizeof(guid)) + 1, guid, sizeof(guid));
ca->ca_ncount = 0;
}
return !existing || ac->ac_stale != NULL;
return !existing || ac->ac_stale;
}
/**Feed authentication data to the authenticator.
......
......@@ -39,6 +39,7 @@
#include <sofia-sip/su_md5.h>
#include "sofia-sip/auth_digest.h"
#include "sofia-sip/msg_header.h"
#include "iptsec_debug.h"
......@@ -60,12 +61,11 @@ static inline int has_token(char const *qstring, char const *token);
*/
int auth_get_params(su_home_t *home,
char const * const params[], ...
/* char const * name, char const **return_value */)
/* char const *fmt, char const **return_value */)
{
int n, j, len, namelen, matched;
int value_match;
char const *name, *p;
char const *value, **return_value;
int n, j, len, namelen;
char const *fmt, *expected;
char const *value, *p, **return_value;
va_list(ap);
assert(params);
......@@ -74,53 +74,66 @@ int auth_get_params(su_home_t *home,
va_start(ap, params);
for (n = 0; (name = va_arg(ap, char const *));) {
for (n = 0; (fmt = va_arg(ap, char const *));) {
return_value = va_arg(ap, char const **);
len = strlen(name);
len = strlen(fmt);
if (!len)
continue;
namelen = strcspn(name, "=");
value_match = len - 1 != namelen;
namelen = strcspn(fmt, "=");
expected = fmt + namelen + 1;
value = NULL;
if (expected[0]) {
/* value match: format is name=expected,
if expected is found in parameter value,
return non-NULL pointer in *return_value */
for (j = 0; (p = params[j++]);) {
if (value_match) {
if (strcasecmp(p, name) == 0) {
if (strcasecmp(p, fmt) == 0) {
/* Matched the whole parameter with fmt name=expected */
value = p;
break;
}
else if (strncasecmp(p, name, namelen) == 0) {
for (matched = namelen; strchr(" \t\r\n", p[matched]); matched++)
;
if (p[matched++] != '=')
continue;
for (; strchr(" \t\r\n", p[matched]); matched++)
;
if (p[matched] == '"' && has_token(p + matched, name + namelen + 1))
value = p + matched;
else if (strcasecmp(p + matched, name + namelen + 1) == 0)
value = p + matched;
else
continue;
}
else {
else if (strncasecmp(p, fmt, namelen) ||
p[namelen] != '=')
continue;
p = p + namelen + 1;
if (p[0] == '"' && has_token(p, expected)) {
/* Quoted parameter value has expected value,
* e.g., qop=auth matches qop="auth,auth-int" */
value = p;
break;
}
else if (strcasecmp(p, expected) == 0) {
/* Parameter value matches with extected value
* e.g., qop=auth matches qop=auth */
value = p;
break;
}
else if (strncasecmp(p, name, len) == 0) {
if (home && p[len] == '"') {
int quoted = strcspn(p + len + 1, "\"");
value = su_strndup(home, p + len + 1, quoted);
}
else {
value = p + len;
}
}
else {
/* format is name= , return unquoted parameter value after = */
for (j = 0; (p = params[j++]);) {
if (strncasecmp(p, fmt, len))
continue;
if (p[len] == '"')
value = msg_unquote_dup(home, p + len);
else
value = su_strdup(home, p + len);
if (value == NULL)
return -1;
break;
}
}
if (value) {
*return_value = value;
n++;
break;
}
}
......@@ -193,6 +206,7 @@ int auth_digest_challenge_get(su_home_t *home,
int n;
auth_challenge_t ac[1] = {{ 0 }};
char const *md5 = NULL, *md5sess = NULL, *sha1 = NULL,
*stale = NULL,
*qop_auth = NULL, *qop_auth_int = NULL;
ac->ac_size = sizeof(ac);
......@@ -208,21 +222,19 @@ int auth_digest_challenge_get(su_home_t *home,
"domain=", &ac->ac_domain,
"nonce=", &ac->ac_nonce,
"opaque=", &ac->ac_opaque,
"stale=", &ac->ac_stale,
"algorithm=", &ac->ac_algorithm,
"qop=", &ac->ac_qop,
"algorithm=md5", &md5,
"algorithm=md5-sess", &md5sess,
"algorithm=sha1", &sha1,
"stale=true", &stale,
"qop=auth", &qop_auth,
"qop=auth-int", &qop_auth_int,
NULL);
if (n < 0)
return n;
if (ac->ac_stale && strcasecmp(ac->ac_stale, "true") != 0)
ac->ac_stale = NULL;
ac->ac_stale = stale != NULL;
ac->ac_md5 = md5 != NULL || ac->ac_algorithm == NULL;
ac->ac_md5sess = md5sess != NULL;
ac->ac_sha1 = sha1 != NULL;
......@@ -236,6 +248,23 @@ int auth_digest_challenge_get(su_home_t *home,
return n;
}
/** Free challenge parameters */
void auth_digest_challenge_free_params(su_home_t *home, auth_challenge_t *ac)
{
if (ac->ac_realm)
su_free(home, (void *)ac->ac_realm), ac->ac_realm = NULL;
if (ac->ac_domain)
su_free(home, (void *)ac->ac_domain), ac->ac_domain = NULL;
if (ac->ac_nonce)
su_free(home, (void *)ac->ac_nonce), ac->ac_nonce = NULL;
if (ac->ac_opaque)
su_free(home, (void *)ac->ac_opaque), ac->ac_opaque = NULL;
if (ac->ac_algorithm)
su_free(home, (void *)ac->ac_algorithm), ac->ac_algorithm = NULL;
if (ac->ac_qop)
su_free(home, (void *)ac->ac_qop), ac->ac_qop = NULL;
}
/**Get digest-response parameters.
*
* The function auth_response_get() searches for the digest authentication
......
......@@ -77,12 +77,12 @@ typedef struct {
char const *ac_domain; /**< domain */
char const *ac_nonce; /**< nonce */
char const *ac_opaque; /**< opaque */
char const *ac_stale; /**< stale */
char const *ac_algorithm; /**< algorithm */
char const *ac_qop; /**< qop */
unsigned ac_md5 : 1; /**< MS5 algorithm */
unsigned ac_md5sess : 1; /**< MD5-sess algorithm */
unsigned ac_sha1 : 1; /**< SSA Hash Algorithm */
unsigned ac_stale : 1; /**< stale=true */
unsigned ac_md5 : 1; /**< algorithm=MS5 (or missing) */
unsigned ac_md5sess : 1; /**< algorithm=MD5-sess */
unsigned ac_sha1 : 1; /**< algorithm=sha1 (SSA Hash) */
unsigned ac_auth : 1; /**< qop=auth */
unsigned ac_auth_int : 1; /**< qop=auth-int */
unsigned : 0;
......@@ -135,9 +135,11 @@ typedef struct {
typedef char auth_hexmd5_t[33];
int auth_digest_challenge_get(su_home_t *, auth_challenge_t *,
SOFIAPUBFUN int auth_digest_challenge_get(su_home_t *, auth_challenge_t *,
char const * const params[]);
int auth_digest_response_get(su_home_t *, auth_response_t *,
SOFIAPUBFUN void auth_digest_challenge_free_params(su_home_t *home,
auth_challenge_t *ac);
SOFIAPUBFUN int auth_digest_response_get(su_home_t *, auth_response_t *,
char const * const params[]);
int auth_digest_a1(auth_response_t *ar,
......
......@@ -38,15 +38,13 @@
#include <stdio.h>
#include <string.h>
#if HAVE_SOFIA_SIP && 0
#if HAVE_SOFIA_SIP
#define PROTOCOL "SIP/2.0"
#define GOT_HEADERS 1
#include <sofia-sip/sip.h>
#include <sofia-sip/sip_header.h>
#include <sip_hclass.h>
#elif HAVE_SOFIA_HTTP
#include <sofia-sip/sip_hclasses.h>
#else
#define PROTOCOL "HTTP/1.1"
#define GOT_HEADERS 1
#include <sofia-sip/http.h>
#include <sofia-sip/http_header.h>
#define sip_authentication_info_class http_authentication_info_class
......@@ -69,8 +67,6 @@
#define sip_www_authenticate http_www_authenticate
#define sip_www_authenticate_make http_www_authenticate_make
#define sip_www_authenticate_t http_www_authenticate_t
#else
#undef GOT_HEADERS
#endif
#include <sofia-sip/auth_digest.h>
......@@ -103,7 +99,6 @@ void su_time(su_time_t *tv)
tv->tv_usec = 555555;
}
#if GOT_HEADERS
int test_digest()
{
char challenge[] = "Digest "
......@@ -195,7 +190,7 @@ int test_digest()
TEST0(strcmp(hresponse, "dd22a698b1a9510c4237c52e0e2cbfac") == 0);
TEST0(pa = sip_proxy_authenticate_make(home, proxy_authenticate));
TEST(auth_digest_challenge_get(home, ac, pa->au_params), 10);
TEST(auth_digest_challenge_get(home, ac, pa->au_params), 9);
TEST_S(ac->ac_realm, "IndigoSw");
TEST_1(ac->ac_auth);
......@@ -535,6 +530,7 @@ int test_digest_client()
TEST(auc_challenge(&aucs, home, sip->sip_www_authenticate,
sip_authorization_class), 1);
msg_destroy(m1);
TEST(auc_all_credentials(&aucs, "Digest", "\"ims3.so.noklab.net\"",
"surf3.private@ims3.so.noklab.net", "1234"), 1);
......@@ -656,6 +652,7 @@ int test_digest_client()
TEST(as->as_status, 401);
TEST(auc_challenge(&aucs, home, (msg_auth_t *)as->as_response,
sip_authorization_class), 1);
reinit_as(as);
TEST(auc_all_credentials(&aucs, "Digest", "\"ims3.so.noklab.net\"",
"user1", "secret"), 1);
......@@ -670,7 +667,6 @@ int test_digest_client()
TEST_1(msg_params_find(sip->sip_authorization->au_params, "cnonce=") == 0);
TEST_1(msg_params_find(sip->sip_authorization->au_params, "nc=") == 0);
reinit_as(as);
auth_mod_check_client(am, as, sip->sip_authorization, ach);
TEST(as->as_status, 0);
TEST_1(as->as_info); /* challenge for next round */
......@@ -683,13 +679,30 @@ int test_digest_client()
AUTHTAG_DB(testpasswd),
AUTHTAG_ALGORITHM("MD5-sess"),
AUTHTAG_QOP("auth"),
AUTHTAG_OPAQUE("opaque=="),
TAG_END()));
reinit_as(as);
auth_mod_check_client(am, as, NULL, ach); TEST(as->as_status, 401);
TEST(auc_challenge(&aucs, home, (msg_auth_t *)as->as_response,
sip_authorization_class), 1);
{
msg_auth_t *au = (msg_auth_t *)as->as_response;
int i;
char *equal;
if (au->au_params)
for (i = 0; au->au_params[i]; i++) {
if (strncasecmp(au->au_params[i], "realm=", 6) == 0)
continue;
equal = strchr(au->au_params[i], '=');
if (equal)
msg_unquote(equal + 1, equal + 1);
}
TEST(auc_challenge(&aucs, home, au, sip_authorization_class), 1);
reinit_as(as);
}
TEST(auc_all_credentials(&aucs, "Digest", "\"ims3.so.noklab.net\"",
"user1", "secret"), 1);
msg_header_remove(m2, (void *)sip, (void *)sip->sip_authorization);
......@@ -698,7 +711,6 @@ int test_digest_client()
sip->sip_payload), 1);
TEST_1(sip->sip_authorization);
reinit_as(as);
auth_mod_check_client(am, as, sip->sip_authorization, ach);
TEST(as->as_status, 0);
TEST_1(as->as_info == NULL); /* No challenge for next round */
......@@ -835,6 +847,8 @@ int test_digest_client()
auth_mod_check_client(am, as, NULL, ach); TEST(as->as_status, 401);
TEST(auc_challenge(&aucs, home, (msg_auth_t *)as->as_response,
sip_authorization_class), 1);
reinit_as(as);
TEST(auc_all_credentials(&aucs, "Digest", "\"ims3.so.noklab.net\"",
"anonymous", ""), 1);
msg_header_remove(m2, (void *)sip, (void *)sip->sip_authorization);
......@@ -843,7 +857,6 @@ int test_digest_client()
sip->sip_payload), 1);
TEST_1(sip->sip_authorization);
reinit_as(as);
auth_mod_check_client(am, as, sip->sip_authorization, ach);
TEST(as->as_status, 0);
auth_mod_destroy(am); aucs = NULL;
......@@ -861,6 +874,8 @@ int test_digest_client()
TEST(auc_challenge(&aucs, home, (msg_auth_t *)as->as_response,
sip_authorization_class), 1);
reinit_as(as);
TEST(auc_all_credentials(&aucs, "Basic", "\"ims3.so.noklab.net\"",
"user1", "secret"), 1);
msg_header_remove(m2, (void *)sip, (void *)sip->sip_authorization);
......@@ -869,7 +884,6 @@ int test_digest_client()
sip->sip_payload), 1);
TEST_1(sip->sip_authorization);
reinit_as(as);
auth_mod_check_client(am, as, sip->sip_authorization, ach);
TEST(as->as_status, 0);
......@@ -886,7 +900,6 @@ int test_digest_client()
AUTHTAG_REMOTE((void *)"http://localhost:9"),
TAG_END()));
reinit_as(as);
as->as_callback = test_callback;
as->as_magic = root;
......@@ -897,6 +910,11 @@ int test_digest_client()
TEST(auc_challenge(&aucs, home, (msg_auth_t *)as->as_response,
sip_authorization_class), 1);
reinit_as(as);
as->as_callback = test_callback;
as->as_magic = root;
TEST(auc_all_credentials(&aucs, "Digest", "\"ims3.so.noklab.net\"",
"user1", "secret"), 1);
msg_header_remove(m2, (void *)sip, (void *)sip->sip_authorization);
......@@ -905,9 +923,6 @@ int test_digest_client()
sip->sip_payload), 1);
TEST_1(sip->sip_authorization);
reinit_as(as);
as->as_callback = test_callback;
as->as_magic = root;
auth_mod_check_client(am, as, sip->sip_authorization, ach);
TEST(as->as_status, 100);
su_root_run(root);
......@@ -916,7 +931,6 @@ int test_digest_client()
auth_mod_destroy(am); aucs = NULL;
deinit_as(as);
msg_destroy(m1);
msg_destroy(m2);
su_root_destroy(root);
......@@ -926,7 +940,6 @@ int test_digest_client()
END();
}
#endif
#if HAVE_FLOCK
#include <sys/file.h>
......@@ -1105,10 +1118,8 @@ int main(int argc, char *argv[])
else
su_log_soft_set_level(iptsec_log, 0);
#if GOT_HEADERS
retval |= test_digest();
retval |= test_digest_client();
#endif
retval |= test_module_io();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment