Commit 725c27ba authored by Kai Vehmanen's avatar Kai Vehmanen

Fixed checking of USERNAME in connectivity checks.

darcs-hash:20071115164703-77cd4-fd6f9722c4cef782fdff98efbb2bd96250119d7b.gz
parent 8d61b3bf
...@@ -94,7 +94,6 @@ struct _NiceAgent ...@@ -94,7 +94,6 @@ struct _NiceAgent
guint keepalive_timer_id; /**< id of keepalive timer */ guint keepalive_timer_id; /**< id of keepalive timer */
guint64 tie_breaker; /**< tie breaker (ICE sect 5.2 guint64 tie_breaker; /**< tie breaker (ICE sect 5.2
"Determining Role" ID-17) */ "Determining Role" ID-17) */
gchar ufragtmp[NICE_STREAM_MAX_UNAME]; /**< preallocated buffer for uname processing */
/* XXX: add pointer to internal data struct for ABI-safe extensions */ /* XXX: add pointer to internal data struct for ABI-safe extensions */
}; };
......
...@@ -776,8 +776,9 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair) ...@@ -776,8 +776,9 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair)
1, 1,
pair->local->component_id); pair->local->component_id);
gchar uname[NICE_STREAM_MAX_UNAME];
gboolean username_filled = gboolean username_filled =
priv_create_check_username (agent, pair, agent->ufragtmp, NICE_STREAM_MAX_UNAME); priv_create_check_username (agent, pair, uname, sizeof (uname));
const gchar *password = priv_create_check_password (agent, pair); const gchar *password = priv_create_check_password (agent, pair);
bool controlling = agent->controlling_mode; bool controlling = agent->controlling_mode;
...@@ -801,7 +802,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair) ...@@ -801,7 +802,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair)
pair->local->sockptr->fileno, pair->local->sockptr->fileno,
pair->foundation, pair->component_id, pair->foundation, pair->component_id,
(unsigned long long)agent->tie_breaker, (unsigned long long)agent->tie_breaker,
agent->ufragtmp, password, priority); uname, password, priority);
} }
#endif #endif
...@@ -816,7 +817,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair) ...@@ -816,7 +817,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair)
stun_conncheck_start (&pair->stun_ctx, pair->local->sockptr->fileno, stun_conncheck_start (&pair->stun_ctx, pair->local->sockptr->fileno,
&sockaddr, sizeof (sockaddr), &sockaddr, sizeof (sockaddr),
agent->ufragtmp, password, uname, password,
cand_use, controlling, priority, cand_use, controlling, priority,
agent->tie_breaker); agent->tie_breaker);
...@@ -880,7 +881,7 @@ static void priv_update_check_list_state (NiceAgent *agent, Stream *stream) ...@@ -880,7 +881,7 @@ static void priv_update_check_list_state (NiceAgent *agent, Stream *stream)
* *
* @see priv_update_check_list_state_for_component() * @see priv_update_check_list_state_for_component()
*/ */
static void priv_prune_pending_checks (NiceAgent *agent, Stream *stream, guint component_id) static void priv_prune_pending_checks (/*NiceAgent *agent, */Stream *stream, guint component_id)
{ {
GSList *i; GSList *i;
...@@ -927,7 +928,7 @@ static void priv_update_check_list_state_for_component (NiceAgent *agent, Stream ...@@ -927,7 +928,7 @@ static void priv_update_check_list_state_for_component (NiceAgent *agent, Stream
p->state == NICE_CHECK_DISCOVERED) { p->state == NICE_CHECK_DISCOVERED) {
++succeeded; ++succeeded;
if (p->nominated == TRUE) { if (p->nominated == TRUE) {
priv_prune_pending_checks (agent, stream, p->component_id); priv_prune_pending_checks (/*agent, */stream, p->component_id);
agent_signal_component_state_change (agent, agent_signal_component_state_change (agent,
p->stream_id, p->stream_id,
p->component_id, p->component_id,
...@@ -1383,8 +1384,11 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream, Compo ...@@ -1383,8 +1384,11 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream, Compo
ssize_t res; ssize_t res;
size_t rbuf_len = sizeof (rbuf); size_t rbuf_len = sizeof (rbuf);
bool control = agent->controlling_mode; bool control = agent->controlling_mode;
gchar uname[NICE_STREAM_MAX_UNAME];
nice_address_copy_to_sockaddr (from, &sockaddr); nice_address_copy_to_sockaddr (from, &sockaddr);
g_snprintf (uname, sizeof (uname), "%s:%s", stream->local_ufrag,
stream->remote_ufrag);
/* note: contents of 'buf' already validated, so it is /* note: contents of 'buf' already validated, so it is
* a valid and fully received STUN message */ * a valid and fully received STUN message */
...@@ -1394,7 +1398,8 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream, Compo ...@@ -1394,7 +1398,8 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream, Compo
/* note: ICE 7.2. "STUN Server Procedures" (ID-18) */ /* note: ICE 7.2. "STUN Server Procedures" (ID-18) */
res = stun_conncheck_reply (rbuf, &rbuf_len, (const uint8_t*)buf, &sockaddr, sizeof (sockaddr), res = stun_conncheck_reply (rbuf, &rbuf_len, (const uint8_t*)buf, &sockaddr, sizeof (sockaddr),
stream->local_password, &control, agent->tie_breaker); uname, stream->local_password,
&control, agent->tie_breaker);
if (res == EACCES) if (res == EACCES)
priv_check_for_role_conflict (agent, control); priv_check_for_role_conflict (agent, control);
...@@ -1409,13 +1414,7 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream, Compo ...@@ -1409,13 +1414,7 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream, Compo
if (agent->controlling_mode) if (agent->controlling_mode)
use_candidate = TRUE; use_candidate = TRUE;
/* note: verify the USERNAME */ if (priv_verify_inbound_username (stream, uname) != TRUE) {
if (stun_conncheck_username ((const uint8_t*)buf, agent->ufragtmp, NICE_STREAM_MAX_UNAME) == NULL) {
g_debug ("No USERNAME attribute in incoming STUN request, ignoring.");
return FALSE;
}
if (priv_verify_inbound_username (stream, agent->ufragtmp) != TRUE) {
g_debug ("USERNAME does not match local streams, ignoring."); g_debug ("USERNAME does not match local streams, ignoring.");
return FALSE; return FALSE;
} }
......
...@@ -76,9 +76,11 @@ stun_bind_error (uint8_t *buf, size_t *plen, const uint8_t *req, ...@@ -76,9 +76,11 @@ stun_bind_error (uint8_t *buf, size_t *plen, const uint8_t *req,
int int
stun_conncheck_reply (uint8_t *buf, size_t *restrict plen, const uint8_t *msg, stun_conncheck_reply (uint8_t *restrict buf, size_t *restrict plen,
const uint8_t *msg,
const struct sockaddr *restrict src, socklen_t srclen, const struct sockaddr *restrict src, socklen_t srclen,
const char *pass, bool *restrict control, uint64_t tie) const char *username, const char *pass,
bool *restrict control, uint64_t tie)
{ {
size_t len = *plen; size_t len = *plen;
uint64_t q; uint64_t q;
...@@ -119,15 +121,15 @@ stun_conncheck_reply (uint8_t *buf, size_t *restrict plen, const uint8_t *msg, ...@@ -119,15 +121,15 @@ stun_conncheck_reply (uint8_t *buf, size_t *restrict plen, const uint8_t *msg,
/* Short term credentials checking */ /* Short term credentials checking */
val = 0; val = 0;
if (!stun_present (msg, STUN_MESSAGE_INTEGRITY) // FIXME: wrong! if (!stun_present (msg, STUN_MESSAGE_INTEGRITY)
|| !stun_present (msg, STUN_USERNAME)) || !stun_present (msg, STUN_USERNAME))
{ {
DBG (" Missing USERNAME or MESSAGE-INTEGRITY.\n"); DBG (" Missing USERNAME or MESSAGE-INTEGRITY.\n");
val = STUN_BAD_REQUEST; val = STUN_BAD_REQUEST;
} }
else else
/* FIXME: verify USERNAME, return STUN_UNAUTHORIZED if wrong */ if (stun_strcmp (msg, STUN_USERNAME, username)
if (stun_verify_password (msg, pass)) || stun_verify_password (msg, pass))
{ {
DBG (" Integrity check failed.\n"); DBG (" Integrity check failed.\n");
val = STUN_UNAUTHORIZED; val = STUN_UNAUTHORIZED;
...@@ -203,31 +205,6 @@ failure: ...@@ -203,31 +205,6 @@ failure:
#undef err #undef err
/** FIXME: this needs to be merged with stun_conncheck_reply, REALLY! */
char *stun_conncheck_username (const uint8_t *restrict msg,
char *restrict buf, size_t buflen)
{
size_t i;
if ((buflen == 0)
|| stun_find_string (msg, STUN_USERNAME, buf, (buflen - 1) / 6))
return NULL;
for (i = 0; buf[i]; i++)
{
char c = buf[i];
/* ref ICE sect 7.1.1.4. (ID-16) */
if (((c >= '/') && (c <= '9')) || ((c >= 'A') && (c <= 'Z'))
|| ((c >= 'a') && (c <= 'z')) || (c == '+') || (c == ':'))
continue;
return NULL;
}
return buf;
}
uint32_t stun_conncheck_priority (const uint8_t *msg) uint32_t stun_conncheck_priority (const uint8_t *msg)
{ {
uint32_t value; uint32_t value;
......
...@@ -83,6 +83,7 @@ int stun_conncheck_start (stun_bind_t **restrict context, int fd, ...@@ -83,6 +83,7 @@ int stun_conncheck_start (stun_bind_t **restrict context, int fd,
* @param msg pointer to the first byte of the binding request * @param msg pointer to the first byte of the binding request
* @param src socket address the message was received from * @param src socket address the message was received from
* @param srclen byte length of the socket address * @param srclen byte length of the socket address
* @param username STUN username
* @param password HMAC secret password * @param password HMAC secret password
* @param control [IN/OUT] whether we are controlling ICE or not * @param control [IN/OUT] whether we are controlling ICE or not
* @param tie tie breaker value for ICE role determination * @param tie tie breaker value for ICE role determination
...@@ -98,20 +99,11 @@ int stun_conncheck_start (stun_bind_t **restrict context, int fd, ...@@ -98,20 +99,11 @@ int stun_conncheck_start (stun_bind_t **restrict context, int fd,
* response, or 0 if no error response should be sent. * response, or 0 if no error response should be sent.
*/ */
int int
stun_conncheck_reply (uint8_t *buf, size_t *restrict plen, const uint8_t *msg, stun_conncheck_reply (uint8_t *restrict buf, size_t *restrict plen,
const uint8_t *msg,
const struct sockaddr *restrict src, socklen_t srclen, const struct sockaddr *restrict src, socklen_t srclen,
const char *pass, bool *restrict control, uint64_t tie); const char *username, const char *pass,
bool *restrict control, uint64_t tie);
/**
* Extracts the username from a STUN message.
* @param msg pointer to the first byte of the binding request
* @param buf where to store the username as a nul-terminated string
* @param buflen byte length of @a buf buffer
*
* @return @a buf on success, NULL on error
*/
char *stun_conncheck_username (const uint8_t *restrict msg,
char *restrict buf, size_t buflen);
/** /**
* Extracts the priority from a STUN message. * Extracts the priority from a STUN message.
......
...@@ -456,6 +456,9 @@ stun_finish_long (uint8_t *msg, size_t *restrict plen, ...@@ -456,6 +456,9 @@ stun_finish_long (uint8_t *msg, size_t *restrict plen,
if (username != NULL) if (username != NULL)
{ {
if (strlen (username) >= 513)
return EINVAL;
val = stun_append_string (msg, len, STUN_USERNAME, username); val = stun_append_string (msg, len, STUN_USERNAME, username);
if (val) if (val)
return val; return val;
...@@ -463,6 +466,9 @@ stun_finish_long (uint8_t *msg, size_t *restrict plen, ...@@ -463,6 +466,9 @@ stun_finish_long (uint8_t *msg, size_t *restrict plen,
if (nonce != NULL) if (nonce != NULL)
{ {
/*if (utf32_strlen (nonce) > 127))
return EINVAL;*/
val = stun_append_string (msg, len, STUN_NONCE, nonce); val = stun_append_string (msg, len, STUN_NONCE, nonce);
if (val) if (val)
return val; return val;
......
...@@ -82,7 +82,7 @@ int main (void) ...@@ -82,7 +82,7 @@ int main (void)
len = sizeof (resp); len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == EINVAL); assert (val == EINVAL);
assert (len == 0); assert (len == 0);
...@@ -94,7 +94,7 @@ int main (void) ...@@ -94,7 +94,7 @@ int main (void)
len = sizeof (resp); len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == EPROTO); assert (val == EPROTO);
assert (len > 0); assert (len > 0);
...@@ -109,7 +109,7 @@ int main (void) ...@@ -109,7 +109,7 @@ int main (void)
len = sizeof (resp); len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == EPROTO); assert (val == EPROTO);
assert (len > 0); assert (len > 0);
...@@ -121,7 +121,7 @@ int main (void) ...@@ -121,7 +121,7 @@ int main (void)
len = sizeof (resp); len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == EPERM); assert (val == EPERM);
assert (len > 0); assert (len > 0);
assert (stun_match_messages (resp, req, NULL, 0, &code) assert (stun_match_messages (resp, req, NULL, 0, &code)
...@@ -135,34 +135,14 @@ int main (void) ...@@ -135,34 +135,14 @@ int main (void)
len = sizeof (resp); len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == EPERM); assert (val == EPERM);
assert (len > 0); assert (len > 0);
assert (stun_match_messages (resp, req, NULL, 0, &code) assert (stun_match_messages (resp, req, NULL, 0, &code)
&& (code == STUN_BAD_REQUEST)); && (code == STUN_BAD_REQUEST));
assert (stun_conncheck_username (req, NULL, 0) == NULL);
assert (stun_conncheck_username (req, nbuf, sizeof (nbuf)) == NULL);
assert (stun_conncheck_priority (req) == 0); assert (stun_conncheck_priority (req) == 0);
assert (stun_conncheck_use_candidate (req) == false); assert (stun_conncheck_use_candidate (req) == false);
/* Bad username */
stun_init_request (req, STUN_BINDING);
len = sizeof (req);
val = stun_finish_short (req, &len, "ab\xff", pass, NULL);
assert (val == 0);
assert (stun_conncheck_username (req, nbuf, sizeof (nbuf)) == NULL);
/* FIXME: use conncheck_reply */
/* Bad integrity */
len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), "bad", &control, tie);
assert (val == EPERM);
assert (len > 0);
assert (stun_match_messages (resp, req, NULL, 0, &code)
&& (code == STUN_UNAUTHORIZED));
/* Good message */ /* Good message */
stun_init_request (req, STUN_BINDING); stun_init_request (req, STUN_BINDING);
val = stun_append32 (req, sizeof (req), STUN_PRIORITY, 0x12345678); val = stun_append32 (req, sizeof (req), STUN_PRIORITY, 0x12345678);
...@@ -175,7 +155,7 @@ int main (void) ...@@ -175,7 +155,7 @@ int main (void)
len = sizeof (resp); len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == 0); assert (val == 0);
assert (len > 0); assert (len > 0);
assert (stun_match_messages (resp, req, (uint8_t *)pass, assert (stun_match_messages (resp, req, (uint8_t *)pass,
...@@ -184,19 +164,30 @@ int main (void) ...@@ -184,19 +164,30 @@ int main (void)
assert (stun_conncheck_priority (req) == 0x12345678); assert (stun_conncheck_priority (req) == 0x12345678);
assert (stun_conncheck_use_candidate (req) == true); assert (stun_conncheck_use_candidate (req) == true);
/* Error cases for username extraction */ /* Bad username */
assert (stun_conncheck_username (req, NULL, 0) == NULL); len = sizeof (resp);
assert (stun_conncheck_username (req, nbuf, strlen (name) - 1) == NULL); val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
/* Username extraction */ sizeof (ip4), "bad", pass, &control, tie);
strcpy (nbuf, "haxor"); assert (val == EPERM);
assert (stun_conncheck_username (req, nbuf, sizeof (nbuf)) == nbuf); assert (len > 0);
assert (strcmp (nbuf, name) == 0); assert (stun_match_messages (resp, req, NULL, 0, &code)
&& (code == STUN_UNAUTHORIZED));
/* Bad integrity (bad password) */
len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), name, "bad", &control, tie);
assert (val == EPERM);
assert (len > 0);
assert (stun_match_messages (resp, req, NULL, 0, &code)
&& (code == STUN_UNAUTHORIZED));
/* Invalid socket address */ /* Invalid socket address */
ip4.sin_family = AF_UNSPEC; ip4.sin_family = AF_UNSPEC;
len = sizeof (resp); len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == EAFNOSUPPORT); assert (val == EAFNOSUPPORT);
ip4.sin_family = AF_INET; ip4.sin_family = AF_INET;
...@@ -210,7 +201,7 @@ int main (void) ...@@ -210,7 +201,7 @@ int main (void)
len = sizeof (resp); len = sizeof (resp);
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == EINVAL); assert (val == EINVAL);
assert (len == 0); assert (len == 0);
...@@ -225,7 +216,7 @@ int main (void) ...@@ -225,7 +216,7 @@ int main (void)
len = sizeof (resp); len = sizeof (resp);
control = true; control = true;
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == EACCES); assert (val == EACCES);
assert (len > 0); assert (len > 0);
assert (control == false); assert (control == false);
...@@ -244,7 +235,7 @@ int main (void) ...@@ -244,7 +235,7 @@ int main (void)
len = sizeof (resp); len = sizeof (resp);
control = false; control = false;
val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4, val = stun_conncheck_reply (resp, &len, req, (struct sockaddr *)&ip4,
sizeof (ip4), pass, &control, tie); sizeof (ip4), name, pass, &control, tie);
assert (val == 0); assert (val == 0);
assert (len > 0); assert (len > 0);
assert (control == false); assert (control == false);
......
...@@ -304,7 +304,7 @@ static void test_attribute (void) ...@@ -304,7 +304,7 @@ static void test_attribute (void)
static void test_vectors (void) static void test_vectors (void)
{ {
static const char username[] = "evtj:h6vY"; /*static const char username[] = "evtj:h6vY";*/
static const char password[] = "VOkJxbRl1RmTxUk/WvJxBt"; static const char password[] = "VOkJxbRl1RmTxUk/WvJxBt";
/* Request message */ /* Request message */
static const unsigned char req[] = static const unsigned char req[] =
......
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