Index: pjproject/trunk/pjnath/src/pjnath/stun_auth.c
===================================================================
--- pjproject/trunk/pjnath/src/pjnath/stun_auth.c	(revision 1374)
+++ pjproject/trunk/pjnath/src/pjnath/stun_auth.c	(revision 1439)
@@ -60,9 +60,16 @@
 
 
+PJ_INLINE(void) PUT_VAL16(pj_uint8_t *buf, unsigned pos, pj_uint16_t hval)
+{
+    buf[pos+0] = (pj_uint8_t) ((hval & 0xFF00) >> 8);
+    buf[pos+1] = (pj_uint8_t) ((hval & 0x00FF) >> 0);
+}
+
+
 /* Send 401 response */
 static pj_status_t create_challenge(pj_pool_t *pool,
 				    const pj_stun_msg *msg,
 				    int err_code,
-				    const pj_str_t *err_msg,
+				    const char *errstr,
 				    const pj_str_t *realm,
 				    const pj_str_t *nonce,
@@ -71,8 +78,10 @@
     pj_stun_msg *response;
     pj_str_t tmp_nonce;
+    pj_str_t err_msg;
     pj_status_t rc;
 
-    rc = pj_stun_msg_create_response(pool, msg, 
-				     err_code,  err_msg, &response);
+    rc = pj_stun_msg_create_response(pool, msg, err_code, 
+			             (errstr?pj_cstr(&err_msg, errstr):NULL), 
+				     &response);
     if (rc != PJ_SUCCESS)
 	return rc;
@@ -117,5 +126,6 @@
     pj_str_t realm, nonce, password;
     const pj_stun_msgint_attr *amsgi;
-    unsigned amsgi_pos;
+    unsigned i, amsgi_pos;
+    pj_bool_t has_attr_beyond_mi;
     const pj_stun_username_attr *auser;
     pj_bool_t username_ok;
@@ -139,5 +149,5 @@
 	p_response = NULL;
 
-    /* Get realm and nonce */
+    /* Get realm and nonce from credential */
     realm.slen = nonce.slen = 0;
     if (cred->type == PJ_STUN_AUTH_CRED_STATIC) {
@@ -154,13 +164,32 @@
     }
 
-    /* First check that MESSAGE-INTEGRITY is present */
-    amsgi = (const pj_stun_msgint_attr*)
-	    pj_stun_msg_find_attr(msg, PJ_STUN_ATTR_MESSAGE_INTEGRITY, 0);
+    /* Look for MESSAGE-INTEGRITY while counting the position */
+    amsgi_pos = 0;
+    has_attr_beyond_mi = PJ_FALSE;
+    amsgi = NULL;
+    for (i=0; i<msg->attr_count; ++i) {
+	if (msg->attr[i]->type == PJ_STUN_ATTR_MESSAGE_INTEGRITY) {
+	    amsgi = (const pj_stun_msgint_attr*) msg->attr[i];
+	} else if (amsgi) {
+	    has_attr_beyond_mi = PJ_TRUE;
+	    break;
+	} else {
+	    amsgi_pos += ((msg->attr[i]->length+3) & ~0x03) + 4;
+	}
+    }
+
     if (amsgi == NULL) {
+	/* According to rfc3489bis-10 Sec 10.1.2/10.2.2, we should return 400
+	   for short term, and 401 for long term.
+	   The rule has been changed from rfc3489bis-06
+	*/
+	int code;
+
+	code = realm.slen ? PJ_STUN_SC_UNAUTHORIZED : PJ_STUN_SC_BAD_REQUEST;
 	if (p_response) {
-	    create_challenge(pool, msg, PJ_STUN_SC_UNAUTHORIZED, NULL,
+	    create_challenge(pool, msg, code, NULL,
 			     &realm, &nonce, p_response);
 	}
-	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_INTEGRITY_CHECK_FAILURE);
+	return PJ_STATUS_FROM_STUN_CODE(code);
     }
 
@@ -169,9 +198,14 @@
 	    pj_stun_msg_find_attr(msg, PJ_STUN_ATTR_USERNAME, 0);
     if (auser == NULL) {
+	/* According to rfc3489bis-10 Sec 10.1.2/10.2.2, we should return 400
+	   for both short and long term, since M-I is present.
+	   The rule has been changed from rfc3489bis-06
+	*/
+	int code = PJ_STUN_SC_BAD_REQUEST;
 	if (p_response) {
-	    create_challenge(pool, msg, PJ_STUN_SC_MISSING_USERNAME, NULL,
+	    create_challenge(pool, msg, code, "Missing USERNAME",
 			     &realm, &nonce, p_response);
 	}
-	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_MISSING_USERNAME);
+	return PJ_STATUS_FROM_STUN_CODE(code);
     }
 
@@ -201,9 +235,12 @@
     if (!username_ok) {
 	/* Username mismatch */
+	/* According to rfc3489bis-10 Sec 10.1.2/10.2.2, we should 
+	 * return 401 
+	 */
 	if (p_response) {
-	    create_challenge(pool, msg, PJ_STUN_SC_UNKNOWN_USERNAME, NULL,
+	    create_challenge(pool, msg, PJ_STUN_SC_UNAUTHORIZED, NULL,
 			     &realm, &nonce, p_response);
 	}
-	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_UNKNOWN_USERNAME);
+	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_UNAUTHORIZED);
     }
 
@@ -217,8 +254,9 @@
 	/* Long term credential is required and REALM is not present */
 	if (p_response) {
-	    create_challenge(pool, msg, PJ_STUN_SC_MISSING_REALM, NULL,
+	    create_challenge(pool, msg, PJ_STUN_SC_BAD_REQUEST, 
+			     "Missing REALM",
 			     &realm, &nonce, p_response);
 	}
-	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_MISSING_REALM);
+	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_BAD_REQUEST);
 
     } else if (realm.slen != 0 && arealm != NULL) {
@@ -228,8 +266,8 @@
 	if (anonce == NULL && nonce.slen) {
 	    if (p_response) {
-		create_challenge(pool, msg, PJ_STUN_SC_MISSING_NONCE, 
-				 NULL, &realm, &nonce, p_response);
+		create_challenge(pool, msg, PJ_STUN_SC_BAD_REQUEST, 
+				 "Missing NONCE", &realm, &nonce, p_response);
 	    }
-	    return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_MISSING_NONCE);
+	    return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_BAD_REQUEST);
 	}
 
@@ -238,8 +276,8 @@
 	    /* REALM doesn't match */
 	    if (p_response) {
-		create_challenge(pool, msg, PJ_STUN_SC_MISSING_REALM, 
-				 NULL, &realm, &nonce, p_response);
+		create_challenge(pool, msg, PJ_STUN_SC_UNAUTHORIZED, 
+				 "Invalid REALM", &realm, &nonce, p_response);
 	    }
-	    return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_MISSING_REALM);
+	    return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_UNAUTHORIZED);
 	}
 
@@ -261,8 +299,8 @@
 	if (nonce.slen != 0) {
 	    if (p_response) {
-		create_challenge(pool, msg, PJ_STUN_SC_MISSING_NONCE, 
-				 NULL, &realm, &nonce, p_response);
+		create_challenge(pool, msg, PJ_STUN_SC_UNAUTHORIZED, 
+				 "NONCE required", &realm, &nonce, p_response);
 	    }
-	    return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_MISSING_NONCE);
+	    return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_UNAUTHORIZED);
 	}
     }
@@ -295,28 +333,6 @@
 				 NULL, &realm, &nonce, p_response);
 	    }
-	    return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_MISSING_NONCE);
-	}
-    }
-
-    /* Get the position of MESSAGE-INTEGRITY in the packet */
-    amsgi_pos = 20+msg->hdr.length-24;
-    if (GET_VAL16(pkt, amsgi_pos) == PJ_STUN_ATTR_MESSAGE_INTEGRITY) {
-	/* Found MESSAGE-INTEGRITY as the last attribute */
-    } else {
-	amsgi_pos = 0;
-    }
-    
-    if (amsgi_pos==0) {
-	amsgi_pos = 20+msg->hdr.length-8-24;
-	if (GET_VAL16(pkt, amsgi_pos) == PJ_STUN_ATTR_MESSAGE_INTEGRITY) {
-	    /* Found MESSAGE-INTEGRITY before FINGERPRINT */
-	} else {
-	    amsgi_pos = 0;
-	}
-    }
-
-    if (amsgi_pos==0) {
-	pj_assert(!"Unable to find MESSAGE-INTEGRITY in the message!");
-	return PJ_EBUG;
+	    return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_STALE_NONCE);
+	}
     }
 
@@ -324,24 +340,40 @@
     pj_stun_create_key(pool, &key, &realm, &auser->value, &password);
 
-    /* Now calculate HMAC of the message, adding zero padding if necessary
-     * to make the input 64 bytes aligned.
+    /* Now calculate HMAC of the message. */
+    pj_hmac_sha1_init(&ctx, (pj_uint8_t*)key.ptr, key.slen);
+
+    /* First calculate HMAC for the header.
+     * The calculation is different depending on whether FINGERPRINT attribute
+     * is present in the message.
      */
-    pj_hmac_sha1_init(&ctx, (pj_uint8_t*)key.ptr, key.slen);
-    pj_hmac_sha1_update(&ctx, pkt, amsgi_pos);
-    if (amsgi_pos & 63) {
-	pj_uint8_t zeroes[64];
-	pj_bzero(zeroes, sizeof(zeroes));
-	pj_hmac_sha1_update(&ctx, zeroes, 64-(amsgi_pos & 63));
-    }
+    if (has_attr_beyond_mi) {
+	pj_uint8_t hdr_copy[20];
+	pj_memcpy(hdr_copy, pkt, 20);
+	PUT_VAL16(hdr_copy, 2, (pj_uint16_t)(amsgi_pos + 24));
+	pj_hmac_sha1_update(&ctx, hdr_copy, 20);
+    } else {
+	pj_hmac_sha1_update(&ctx, pkt, 20);
+    }
+
+    /* Now update with the message body */
+    pj_hmac_sha1_update(&ctx, pkt+20, amsgi_pos);
+    // This is no longer necessary as per rfc3489bis-08
+    //if (amsgi_pos & 0x3F) {
+    //	pj_uint8_t zeroes[64];
+    //	pj_bzero(zeroes, sizeof(zeroes));
+    //	pj_hmac_sha1_update(&ctx, zeroes, 64-(amsgi_pos & 0x3F));
+    //}
     pj_hmac_sha1_final(&ctx, digest);
+
 
     /* Compare HMACs */
     if (pj_memcmp(amsgi->hmac, digest, 20)) {
 	/* HMAC value mismatch */
+	/* According to rfc3489bis-10 Sec 10.1.2 we should return 401 */
 	if (p_response) {
-	    create_challenge(pool, msg, PJ_STUN_SC_INTEGRITY_CHECK_FAILURE,
+	    create_challenge(pool, msg, PJ_STUN_SC_UNAUTHORIZED,
 			     NULL, &realm, &nonce, p_response);
 	}
-	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_INTEGRITY_CHECK_FAILURE);
+	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_UNAUTHORIZED);
     }
 
@@ -382,9 +414,9 @@
     case PJ_STUN_SC_BAD_REQUEST:	    /* 400 (Bad Request)	    */
     case PJ_STUN_SC_UNAUTHORIZED:	    /* 401 (Unauthorized)	    */
-    case PJ_STUN_SC_STALE_CREDENTIALS:	    /* 430 (Stale Credential)	    */
-    case PJ_STUN_SC_MISSING_USERNAME:	    /* 432 (Missing Username)	    */
-    case PJ_STUN_SC_MISSING_REALM:	    /* 434 (Missing Realm)	    */
-    case PJ_STUN_SC_UNKNOWN_USERNAME:	    /* 436 (Unknown Username)	    */
-    case PJ_STUN_SC_INTEGRITY_CHECK_FAILURE:/* 431 (Integrity Check Fail)   */
+    //case PJ_STUN_SC_STALE_CREDENTIALS:    /* 430 (Stale Credential)	    */
+    //case PJ_STUN_SC_MISSING_USERNAME:	    /* 432 (Missing Username)	    */
+    //case PJ_STUN_SC_MISSING_REALM:	    /* 434 (Missing Realm)	    */
+    //case PJ_STUN_SC_UNKNOWN_USERNAME:	    /* 436 (Unknown Username)	    */
+    //case PJ_STUN_SC_INTEGRITY_CHECK_FAILURE:/* 431 (Integrity Check Fail) */
 	return PJ_FALSE;
     default:
@@ -401,5 +433,6 @@
 {
     const pj_stun_msgint_attr *amsgi;
-    unsigned amsgi_pos;
+    unsigned i, amsgi_pos;
+    pj_bool_t has_attr_beyond_mi;
     pj_hmac_sha1_context ctx;
     pj_uint8_t digest[PJ_SHA1_DIGEST_SIZE];
@@ -411,5 +444,5 @@
 	    pj_stun_msg_find_attr(msg, PJ_STUN_ATTR_MESSAGE_INTEGRITY, 0);
     if (amsgi == NULL) {
-	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_INTEGRITY_CHECK_FAILURE);
+	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_UNAUTHORIZED);
     }
 
@@ -420,40 +453,47 @@
     }
 
-    /* Get the position of MESSAGE-INTEGRITY in the packet */
-    amsgi_pos = 20+msg->hdr.length-24;
-    if (GET_VAL16(pkt, amsgi_pos) == PJ_STUN_ATTR_MESSAGE_INTEGRITY) {
-	/* Found MESSAGE-INTEGRITY as the last attribute */
+    /* Look for MESSAGE-INTEGRITY while counting the position */
+    amsgi_pos = 0;
+    has_attr_beyond_mi = PJ_FALSE;
+    amsgi = NULL;
+    for (i=0; i<msg->attr_count; ++i) {
+	if (msg->attr[i]->type == PJ_STUN_ATTR_MESSAGE_INTEGRITY) {
+	    amsgi = (const pj_stun_msgint_attr*) msg->attr[i];
+	} else if (amsgi) {
+	    has_attr_beyond_mi = PJ_TRUE;
+	    break;
+	} else {
+	    amsgi_pos += ((msg->attr[i]->length+3) & ~0x03) + 4;
+	}
+    }
+
+    if (amsgi == NULL) {
+	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_BAD_REQUEST);
+    }
+
+    /* Now calculate HMAC of the message. */
+    pj_hmac_sha1_init(&ctx, (pj_uint8_t*)key->ptr, key->slen);
+
+    /* First calculate HMAC for the header.
+     * The calculation is different depending on whether FINGERPRINT attribute
+     * is present in the message.
+     */
+    if (has_attr_beyond_mi) {
+	pj_uint8_t hdr_copy[20];
+	pj_memcpy(hdr_copy, pkt, 20);
+	PUT_VAL16(hdr_copy, 2, (pj_uint16_t)(amsgi_pos+24));
+	pj_hmac_sha1_update(&ctx, hdr_copy, 20);
     } else {
-	amsgi_pos = 0;
-    }
-    
-    if (amsgi_pos==0) {
-	/* Check that message length is valid */
-	if (msg->hdr.length < 32) {
-	    return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_INTEGRITY_CHECK_FAILURE);
-	}
-
-	amsgi_pos = 20+msg->hdr.length-8-24;
-	if (GET_VAL16(pkt, amsgi_pos) == PJ_STUN_ATTR_MESSAGE_INTEGRITY) {
-	    /* Found MESSAGE-INTEGRITY before FINGERPRINT */
-	} else {
-	    amsgi_pos = 0;
-	}
-    }
-
-    if (amsgi_pos==0) {
-	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_INTEGRITY_CHECK_FAILURE);
-    }
-
-    /* Now calculate HMAC of the message, adding zero padding if necessary
-     * to make the input 64 bytes aligned.
-     */
-    pj_hmac_sha1_init(&ctx, (pj_uint8_t*)key->ptr, key->slen);
-    pj_hmac_sha1_update(&ctx, pkt, amsgi_pos);
-    if (amsgi_pos & 0x3F) {
-	pj_uint8_t zeroes[64];
-	pj_bzero(zeroes, sizeof(zeroes));
-	pj_hmac_sha1_update(&ctx, zeroes, 64-(amsgi_pos & 0x3F));
-    }
+	pj_hmac_sha1_update(&ctx, pkt, 20);
+    }
+
+    /* Now update with the message body */
+    pj_hmac_sha1_update(&ctx, pkt+20, amsgi_pos);
+    // This is no longer necessary as per rfc3489bis-08
+    //if (amsgi_pos & 0x3F) {
+    //	pj_uint8_t zeroes[64];
+    //	pj_bzero(zeroes, sizeof(zeroes));
+    //	pj_hmac_sha1_update(&ctx, zeroes, 64-(amsgi_pos & 0x3F));
+    //}
     pj_hmac_sha1_final(&ctx, digest);
 
@@ -461,5 +501,5 @@
     if (pj_memcmp(amsgi->hmac, digest, 20)) {
 	/* HMAC value mismatch */
-	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_INTEGRITY_CHECK_FAILURE);
+	return PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_UNAUTHORIZED);
     }
 
