diff options
author | Benny Prijono <bennylp@teluu.com> | 2008-06-21 12:36:56 +0000 |
---|---|---|
committer | Benny Prijono <bennylp@teluu.com> | 2008-06-21 12:36:56 +0000 |
commit | 0eac2d8e551d69072fb8a60be35c3867fe96b793 (patch) | |
tree | d287b9d43354d3246293fec29e38a31f9973ef99 /pjnath | |
parent | 96f36004c27455545b9b02ba03d59a5e3d7bf4d8 (diff) |
Fixed bug with authenticating STUN messages when unrecognized/unknown non-mandatory STUN attribute is present in the message
git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@2041 74dad513-b988-da41-8d7b-12977e46ad98
Diffstat (limited to 'pjnath')
-rw-r--r-- | pjnath/include/pjnath/stun_msg.h | 6 | ||||
-rw-r--r-- | pjnath/src/pjnath-test/stun.c | 115 | ||||
-rw-r--r-- | pjnath/src/pjnath/stun_msg.c | 63 |
3 files changed, 175 insertions, 9 deletions
diff --git a/pjnath/include/pjnath/stun_msg.h b/pjnath/include/pjnath/stun_msg.h index 9c5602f1..61cff153 100644 --- a/pjnath/include/pjnath/stun_msg.h +++ b/pjnath/include/pjnath/stun_msg.h @@ -582,6 +582,12 @@ typedef struct pj_stun_binary_attr pj_stun_attr_hdr hdr; /** + * Special signature to indicate that this is a valid attribute even + * though we don't have meta-data to describe this attribute. + */ + pj_uint32_t magic; + + /** * Length of the data. */ unsigned length; diff --git a/pjnath/src/pjnath-test/stun.c b/pjnath/src/pjnath-test/stun.c index 1b11f89b..71775f6b 100644 --- a/pjnath/src/pjnath-test/stun.c +++ b/pjnath/src/pjnath-test/stun.c @@ -163,7 +163,7 @@ static struct test "Unknown but non-mandatory should be okay", "\x00\x01\x00\x08\x21\x12\xa4\x42" "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" - "\x80\xff\x00\x04\x00\x00\x00\x00", + "\x80\xff\x00\x03\x00\x00\x00\x00", 28, NULL, PJ_SUCCESS, @@ -369,10 +369,31 @@ static int verify1(pj_stun_msg *msg) /* Attribute count should be zero since unknown attribute is not parsed */ static int verify2(pj_stun_msg *msg) { - if (msg->attr_count != 0) { - PJ_LOG(1,(THIS_FILE, " expecting zero attribute count")); + pj_stun_binary_attr *bin_attr; + + if (msg->attr_count != 1) { + PJ_LOG(1,(THIS_FILE, " expecting one attribute count")); return -200; } + + bin_attr = (pj_stun_binary_attr*)msg->attr[0]; + if (bin_attr->hdr.type != 0x80ff) { + PJ_LOG(1,(THIS_FILE, " expecting attribute type 0x80ff")); + return -210; + } + if (bin_attr->hdr.length != 3) { + PJ_LOG(1,(THIS_FILE, " expecting attribute length = 4")); + return -220; + } + if (bin_attr->magic != PJ_STUN_MAGIC) { + PJ_LOG(1,(THIS_FILE, " expecting PJ_STUN_MAGIC for unknown attr")); + return -230; + } + if (bin_attr->length != 3) { + PJ_LOG(1,(THIS_FILE, " expecting data length 4")); + return -240; + } + return 0; } @@ -688,6 +709,90 @@ static pj_stun_msg* create_msgint2(pj_pool_t *pool, test_vector *v) } +/* Compare two messages */ +static int cmp_msg(const pj_stun_msg *msg1, const pj_stun_msg *msg2) +{ + unsigned i; + + if (msg1->hdr.type != msg2->hdr.type) + return -10; + if (msg1->hdr.length != msg2->hdr.length) + return -20; + if (msg1->hdr.magic != msg2->hdr.magic) + return -30; + if (pj_memcmp(msg1->hdr.tsx_id, msg2->hdr.tsx_id, sizeof(msg1->hdr.tsx_id))) + return -40; + if (msg1->attr_count != msg2->attr_count) + return -50; + + for (i=0; i<msg1->attr_count; ++i) { + const pj_stun_attr_hdr *a1 = msg1->attr[i]; + const pj_stun_attr_hdr *a2 = msg2->attr[i]; + + if (a1->type != a2->type) + return -60; + if (a1->length != a2->length) + return -70; + } + + return 0; +} + +/* Decode and authenticate message with unknown non-mandatory attribute */ +static int handle_unknown_non_mandatory(void) +{ + pj_pool_t *pool = pj_pool_create(mem, NULL, 1000, 1000, NULL); + pj_stun_msg *msg0, *msg1, *msg2; + pj_uint8_t data[] = { 1, 2, 3, 4, 5, 6}; + pj_uint8_t packet[500]; + pj_stun_auth_cred cred; + unsigned len; + pj_status_t rc; + + PJ_LOG(3,(THIS_FILE, " handling unknown non-mandatory attr")); + + PJ_LOG(3,(THIS_FILE, " encoding")); + rc = pj_stun_msg_create(pool, PJ_STUN_BINDING_REQUEST, PJ_STUN_MAGIC, NULL, &msg0); + rc += pj_stun_msg_add_string_attr(pool, msg0, PJ_STUN_ATTR_USERNAME, &USERNAME); + rc += pj_stun_msg_add_binary_attr(pool, msg0, 0x80ff, data, sizeof(data)); + rc += pj_stun_msg_add_msgint_attr(pool, msg0); + rc += pj_stun_msg_encode(msg0, packet, sizeof(packet), 0, &PASSWORD, &len); + +#if 0 + if (1) { + unsigned i; + puts(""); + printf("{ "); + for (i=0; i<len; ++i) printf("0x%02x, ", packet[i]); + puts(" }"); + } +#endif + + PJ_LOG(3,(THIS_FILE, " decoding")); + rc += pj_stun_msg_decode(pool, packet, len, PJ_STUN_IS_DATAGRAM | PJ_STUN_CHECK_PACKET, + &msg1, NULL, NULL); + + rc += cmp_msg(msg0, msg1); + + pj_bzero(&cred, sizeof(cred)); + cred.type = PJ_STUN_AUTH_CRED_STATIC; + cred.data.static_cred.username = USERNAME; + cred.data.static_cred.data_type = PJ_STUN_PASSWD_PLAIN; + cred.data.static_cred.data = PASSWORD; + + PJ_LOG(3,(THIS_FILE, " authenticating")); + rc += pj_stun_authenticate_request(packet, len, msg1, &cred, pool, NULL, NULL); + + PJ_LOG(3,(THIS_FILE, " clone")); + msg2 = pj_stun_msg_clone(pool, msg1); + rc += cmp_msg(msg0, msg2); + + pj_pool_release(pool); + + return rc==0 ? 0 : -4410; +} + + int stun_test(void) { int pad, rc; @@ -706,6 +811,10 @@ int stun_test(void) if (rc != 0) goto on_return; + rc = handle_unknown_non_mandatory(); + if (rc != 0) + goto on_return; + on_return: pj_stun_set_padding_char(pad); return rc; diff --git a/pjnath/src/pjnath/stun_msg.c b/pjnath/src/pjnath/stun_msg.c index cab0f11b..2bbc2fdf 100644 --- a/pjnath/src/pjnath/stun_msg.c +++ b/pjnath/src/pjnath/stun_msg.c @@ -1656,6 +1656,8 @@ PJ_DEF(pj_status_t) pj_stun_binary_attr_create(pj_pool_t *pool, attr = PJ_POOL_ZALLOC_T(pool, pj_stun_binary_attr); INIT_ATTR(attr, attr_type, length); + attr->magic = PJ_STUN_MAGIC; + if (data && length) { attr->length = length; attr->data = (pj_uint8_t*) pj_pool_alloc(pool, length); @@ -2023,6 +2025,7 @@ PJ_DEF(pj_status_t) pj_stun_msg_decode(pj_pool_t *pool, if (adesc == NULL) { /* Unrecognized attribute */ + pj_stun_binary_attr *attr; PJ_LOG(4,(THIS_FILE, "Unrecognized attribute type %d", attr_type)); @@ -2047,6 +2050,36 @@ PJ_DEF(pj_status_t) pj_stun_msg_decode(pj_pool_t *pool, return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_UNKNOWN_ATTRIBUTE); } + /* Make sure we have rooms for the new attribute */ + if (msg->attr_count >= PJ_STUN_MAX_ATTR) { + if (p_response) { + pj_stun_msg_create_response(pool, msg, + PJ_STUN_SC_SERVER_ERROR, + NULL, p_response); + } + return PJNATH_ESTUNTOOMANYATTR; + } + + /* Create binary attribute to represent this */ + status = pj_stun_binary_attr_create(pool, attr_type, pdu+4, + GETVAL16H(pdu, 2), &attr); + if (status != PJ_SUCCESS) { + if (p_response) { + pj_stun_msg_create_response(pool, msg, + PJ_STUN_SC_SERVER_ERROR, + NULL, p_response); + } + + PJ_LOG(4,(THIS_FILE, + "Error parsing unknown STUN attribute type %d", + attr_type)); + + return status; + } + + /* Add the attribute */ + msg->attr[msg->attr_count++] = &attr->hdr; + } else { void *attr; char err_msg1[PJ_ERR_MSG_SIZE], @@ -2124,7 +2157,7 @@ PJ_DEF(pj_status_t) pj_stun_msg_decode(pj_pool_t *pool, if (msg->attr_count >= PJ_STUN_MAX_ATTR) { if (p_response) { pj_stun_msg_create_response(pool, msg, - PJ_STUN_SC_BAD_REQUEST, + PJ_STUN_SC_SERVER_ERROR, NULL, p_response); } return PJNATH_ESTUNTOOMANYATTR; @@ -2134,6 +2167,7 @@ PJ_DEF(pj_status_t) pj_stun_msg_decode(pj_pool_t *pool, msg->attr[msg->attr_count++] = (pj_stun_attr_hdr*)attr; } + /* Next attribute */ if (attr_val_len + 4 >= pdu_len) { pdu += pdu_len; pdu_len = 0; @@ -2239,9 +2273,16 @@ PJ_DEF(pj_status_t) pj_stun_msg_encode(pj_stun_msg *msg, } adesc = find_attr_desc(attr_hdr->type); - PJ_ASSERT_RETURN(adesc != NULL, PJ_EBUG); + if (adesc) { + status = adesc->encode_attr(attr_hdr, buf, buf_size, &printed); + } else { + /* This may be a generic attribute */ + const pj_stun_binary_attr *bin_attr = (const pj_stun_binary_attr*) + attr_hdr; + PJ_ASSERT_RETURN(bin_attr->magic == PJ_STUN_MAGIC, PJ_EBUG); + status = encode_binary_attr(bin_attr, buf, buf_size, &printed); + } - status = adesc->encode_attr(attr_hdr, buf, buf_size, &printed); if (status != PJ_SUCCESS) return status; @@ -2414,9 +2455,19 @@ PJ_DEF(pj_stun_attr_hdr*) pj_stun_attr_clone( pj_pool_t *pool, /* Get the attribute descriptor */ adesc = find_attr_desc(attr->type); - PJ_ASSERT_RETURN(adesc != NULL, NULL); - - return (pj_stun_attr_hdr*) (*adesc->clone_attr)(pool, attr); + if (adesc) { + return (pj_stun_attr_hdr*) (*adesc->clone_attr)(pool, attr); + } else { + /* Clone generic attribute */ + const pj_stun_binary_attr *bin_attr = (const pj_stun_binary_attr*) + attr; + PJ_ASSERT_RETURN(bin_attr->magic == PJ_STUN_MAGIC, NULL); + if (bin_attr->magic == PJ_STUN_MAGIC) { + return (pj_stun_attr_hdr*) clone_binary_attr(pool, attr); + } else { + return NULL; + } + } } |