Fixed bug with authenticating STUN messages when unrecognized/unknown non-mandatory STUN attribute is present in the message
git-svn-id: https://svn.pjsip.org/repos/pjproject/trunk@2041 74dad513-b988-da41-8d7b-12977e46ad98
diff --git a/pjnath/include/pjnath/stun_msg.h b/pjnath/include/pjnath/stun_msg.h
index 9c5602f..61cff15 100644
--- a/pjnath/include/pjnath/stun_msg.h
+++ b/pjnath/include/pjnath/stun_msg.h
@@ -582,6 +582,12 @@
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 1b11f89..71775f6 100644
--- a/pjnath/src/pjnath-test/stun.c
+++ b/pjnath/src/pjnath-test/stun.c
@@ -163,7 +163,7 @@
"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 @@
/* 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 @@
}
+/* 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 @@
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 cab0f11..2bbc2fd 100644
--- a/pjnath/src/pjnath/stun_msg.c
+++ b/pjnath/src/pjnath/stun_msg.c
@@ -1656,6 +1656,8 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
/* 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;
+ }
+ }
}