Added ICE-CONTROLLED and ICE-CONTROLLING attribute, handle ICE 487 error, and add response source address checking

git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@1141 74dad513-b988-da41-8d7b-12977e46ad98

10 files changed, 156 insertions, 29 deletions

diff --git a/pjnath/include/pjnath/errno.h b/pjnath/include/pjnath/errno.h
index cb470747..4327cd46 100644
--- a/pjnath/include/pjnath/errno.h
+++ b/pjnath/include/pjnath/errno.h
@@ -144,6 +144,13 @@
 #define PJNATH_EICEINCANDID	    (PJNATH_ERRNO_START+87) /* 370087 */
 /**
  * @hideinitializer
+ * Source address mismatch. This error occurs if the source address
+ * of the response for ICE connectivity check is different than
+ * the destination address of the request.
+ */
+#define PJNATH_EICEINSRCADDR	    (PJNATH_ERRNO_START+88) /* 370088 */
+/**
+ * @hideinitializer
  * Missing ICE SDP attribute
  */
 #define PJNATH_EICEMISSINGSDP	    (PJNATH_ERRNO_START+90) /* 370090 */
diff --git a/pjnath/include/pjnath/ice_session.h b/pjnath/include/pjnath/ice_session.h
index 761153d8..42bcff4a 100644
--- a/pjnath/include/pjnath/ice_session.h
+++ b/pjnath/include/pjnath/ice_session.h
@@ -403,6 +403,11 @@ typedef enum pj_ice_sess_role
     /**
      * The ICE agent is in controlled role.
      */
+    PJ_ICE_SESS_ROLE_UNKNOWN,
+
+    /**
+     * The ICE agent is in controlled role.
+     */
     PJ_ICE_SESS_ROLE_CONTROLLED,
 
     /**
diff --git a/pjnath/include/pjnath/stun_session.h b/pjnath/include/pjnath/stun_session.h
index 6b45adf2..0cd81f98 100644
--- a/pjnath/include/pjnath/stun_session.h
+++ b/pjnath/include/pjnath/stun_session.h
@@ -109,13 +109,25 @@ typedef struct pj_stun_session_cb
      *			    PJ_SUCCESS, the transaction has timed-out
      *			    or other error has occurred, and the response
      *			    argument may be NULL.
+     *			    Note that when the status is not success, the
+     *			    response may contain non-NULL value if the 
+     *			    response contains STUN ERROR-CODE attribute.
      * @param request	    The original STUN request.
-     * @param response	    The response message, on successful transaction.
+     * @param response	    The response message, on successful transaction,
+     *			    or otherwise MAY BE NULL if status is not success.
+     *			    Note that when the status is not success, this
+     *			    argument may contain non-NULL value if the 
+     *			    response contains STUN ERROR-CODE attribute.
+     * @param src_addr	    The source address where the response was 
+     *			    received, or NULL if the response is NULL.
+     * @param src_addr_len  The length of the source  address.
      */
     void (*on_request_complete)(pj_stun_session *sess,
 			        pj_status_t status,
 			        pj_stun_tx_data *tdata,
-			        const pj_stun_msg *response);
+			        const pj_stun_msg *response,
+				const pj_sockaddr_t *src_addr,
+				unsigned src_addr_len);
 
 
     /**
diff --git a/pjnath/include/pjnath/stun_transaction.h b/pjnath/include/pjnath/stun_transaction.h
index 23f63771..32dd0c41 100644
--- a/pjnath/include/pjnath/stun_transaction.h
+++ b/pjnath/include/pjnath/stun_transaction.h
@@ -70,10 +70,15 @@ typedef struct pj_stun_tsx_cb
      *			    response.
      * @param response	    The STUN response, which value may be NULL if
      *			    \a status is not PJ_SUCCESS.
+     * @param src_addr	    The source address of the response, if response 
+     *			    is not NULL.
+     * @param src_addr_len  The length of the source address.
      */
     void	(*on_complete)(pj_stun_client_tsx *tsx,
 			       pj_status_t status, 
-			       const pj_stun_msg *response);
+			       const pj_stun_msg *response,
+			       const pj_sockaddr_t *src_addr,
+			       unsigned src_addr_len);
 
     /**
      * This callback is called by the STUN transaction when it wants to send
@@ -228,11 +233,15 @@ PJ_DECL(pj_status_t) pj_stun_client_tsx_send_msg(pj_stun_client_tsx *tsx,
  *
  * @param tsx		The STUN client transaction instance.
  * @param msg		The incoming STUN message.
+ * @param src_addr	The source address of the packet.
+ * @param src_addr_len	The length of the source address.
  *
  * @return		PJ_SUCCESS on success or the appropriate error code.
  */
 PJ_DECL(pj_status_t) pj_stun_client_tsx_on_rx_msg(pj_stun_client_tsx *tsx,
-						  const pj_stun_msg *msg);
+						  const pj_stun_msg *msg,
+						  const pj_sockaddr_t*src_addr,
+						  unsigned src_addr_len);
 
 
 /**
diff --git a/pjnath/src/pjnath/errno.c b/pjnath/src/pjnath/errno.c
index ed1f658f..88fc7494 100644
--- a/pjnath/src/pjnath/errno.c
+++ b/pjnath/src/pjnath/errno.c
@@ -56,6 +56,7 @@ static const struct
     PJ_BUILD_ERR( PJNATH_EICEFAILED,	    "All ICE checklists failed"),
     PJ_BUILD_ERR( PJNATH_EICEINCOMPID,	    "Invalid ICE component ID"),
     PJ_BUILD_ERR( PJNATH_EICEINCANDID,	    "Invalid ICE candidate ID"),
+    PJ_BUILD_ERR( PJNATH_EICEINSRCADDR,	    "Source address mismatch"),
     PJ_BUILD_ERR( PJNATH_EICEMISSINGSDP,    "Missing ICE SDP attribute"),
     PJ_BUILD_ERR( PJNATH_EICEINCANDSDP,	    "Invalid SDP \"candidate\" attribute"),
 
diff --git a/pjnath/src/pjnath/ice_session.c b/pjnath/src/pjnath/ice_session.c
index f3b9ce87..4cae89af 100644
--- a/pjnath/src/pjnath/ice_session.c
+++ b/pjnath/src/pjnath/ice_session.c
@@ -115,7 +115,9 @@ static pj_status_t on_stun_rx_request(pj_stun_session *sess,
 static void on_stun_request_complete(pj_stun_session *stun_sess,
 				     pj_status_t status,
 				     pj_stun_tx_data *tdata,
-				     const pj_stun_msg *response);
+				     const pj_stun_msg *response,
+				     const pj_sockaddr_t *src_addr,
+				     unsigned src_addr_len);
 static pj_status_t on_stun_rx_indication(pj_stun_session *sess,
 					 const pj_uint8_t *pkt,
 					 unsigned pkt_len,
@@ -1206,13 +1208,25 @@ static pj_status_t perform_check(pj_ice_sess *ice,
     pj_stun_msg_add_uint_attr(check->tdata->pool, check->tdata->msg, 
 			      PJ_STUN_ATTR_PRIORITY, prio);
 
-    /* Add USE-CANDIDATE and set this check to nominated */
+    /* Add USE-CANDIDATE and set this check to nominated.
+     * Also add ICE-CONTROLLING or ICE-CONTROLLED
+     */
     if (ice->role == PJ_ICE_SESS_ROLE_CONTROLLING) {
 	pj_stun_msg_add_empty_attr(check->tdata->pool, check->tdata->msg, 
 				   PJ_STUN_ATTR_USE_CANDIDATE);
 	check->nominated = PJ_TRUE;
+
+	pj_stun_msg_add_uint64_attr(check->tdata->pool, check->tdata->msg, 
+				    PJ_STUN_ATTR_ICE_CONTROLLING,
+				    &ice->tie_breaker);
+
+    } else {
+	pj_stun_msg_add_uint64_attr(check->tdata->pool, check->tdata->msg, 
+				    PJ_STUN_ATTR_ICE_CONTROLLED,
+				    &ice->tie_breaker);
     }
 
+
     /* Note that USERNAME and MESSAGE-INTEGRITY will be added by the 
      * STUN session.
      */
@@ -1428,7 +1442,9 @@ static pj_status_t on_stun_send_msg(pj_stun_session *sess,
 static void on_stun_request_complete(pj_stun_session *stun_sess,
 				     pj_status_t status,
 				     pj_stun_tx_data *tdata,
-				     const pj_stun_msg *response)
+				     const pj_stun_msg *response,
+				     const pj_sockaddr_t *src_addr,
+				     unsigned src_addr_len)
 {
     struct req_data *rd = (struct req_data*) tdata->user_data;
     pj_ice_sess *ice;
@@ -1440,6 +1456,7 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
     unsigned i;
 
     PJ_UNUSED_ARG(stun_sess);
+    PJ_UNUSED_ARG(src_addr_len);
 
     ice = rd->ice;
     check = &rd->clist->checks[rd->ckid];
@@ -1456,26 +1473,84 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
      */
     lcand = NULL;
 
-    LOG4((ice->obj_name, 
-	 "Check %s%s: connectivity check %s",
-	 dump_check(buffer, sizeof(buffer), &ice->clist, check),
-	 (check->nominated ? " (nominated)" : " (not nominated)"),
-	 (status==PJ_SUCCESS ? "SUCCESS" : "FAILED")));
-
     if (status != PJ_SUCCESS) {
+	char errmsg[PJ_ERR_MSG_SIZE];
+
+	if (status==PJ_STATUS_FROM_STUN_CODE(PJ_STUN_SC_ROLE_CONFLICT)) {
+	    /* Role conclict response.
+	     * 7.1.2.1.  Failure Cases:
+	     * If the request had contained the ICE-CONTROLLED attribute, 
+	     * the agent MUST switch to the controlling role if it has not
+	     * already done so.  If the request had contained the 
+	     * ICE-CONTROLLING attribute, the agent MUST switch to the 
+	     * controlled role if it has not already done so.  Once it has
+	     * switched, the agent MUST immediately retry the request with
+	     * the ICE-CONTROLLING or ICE-CONTROLLED attribute reflecting 
+	     * its new role.
+	     */
+	    pj_ice_sess_role new_role = PJ_ICE_SESS_ROLE_UNKNOWN;
+	    pj_stun_msg *req = tdata->msg;
+
+	    if (pj_stun_msg_find_attr(req, PJ_STUN_ATTR_ICE_CONTROLLING, 0)) {
+		new_role = PJ_ICE_SESS_ROLE_CONTROLLED;
+	    } else if (pj_stun_msg_find_attr(req, PJ_STUN_ATTR_ICE_CONTROLLED, 
+					     0)) {
+		new_role = PJ_ICE_SESS_ROLE_CONTROLLING;
+	    } else {
+		pj_assert(!"We should have put CONTROLLING/CONTROLLED attr!");
+		new_role = PJ_ICE_SESS_ROLE_CONTROLLED;
+	    }
+
+	    if (new_role != ice->role) {
+		LOG4((ice->obj_name, 
+		      "Changing role because of role conflict"));
+		pj_ice_sess_change_role(ice, new_role);
+	    }
+
+	    /* Resend request */
+	    LOG4((ice->obj_name, "Resending check because of role conflict"));
+	    check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_WAITING, 0);
+	    perform_check(ice, clist, rd->ckid);
+	    pj_mutex_unlock(ice->mutex);
+	    return;
+	}
+
+	pj_strerror(status, errmsg, sizeof(errmsg));
+	LOG4((ice->obj_name, 
+	     "Check %s%s: connectivity check FAILED: %s",
+	     dump_check(buffer, sizeof(buffer), &ice->clist, check),
+	     (check->nominated ? " (nominated)" : " (not nominated)"),
+	     errmsg));
+
 	check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_FAILED, status);
 	on_check_complete(ice, check);
 	pj_mutex_unlock(ice->mutex);
 	return;
     }
 
+
     /* The agent MUST check that the source IP address and port of the
      * response equals the destination IP address and port that the Binding
      * Request was sent to, and that the destination IP address and port of
      * the response match the source IP address and port that the Binding
      * Request was sent from.
      */
-    PJ_TODO(ICE_CHECK_RESPONSE_SOURCE_ADDRESS);
+    if (sockaddr_cmp(&check->rcand->addr, src_addr) != 0) {
+	status = PJNATH_EICEINSRCADDR;
+	LOG4((ice->obj_name, 
+	     "Check %s%s: connectivity check FAILED: source address mismatch",
+	     dump_check(buffer, sizeof(buffer), &ice->clist, check),
+	     (check->nominated ? " (nominated)" : " (not nominated)")));
+	check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_FAILED, status);
+	on_check_complete(ice, check);
+	pj_mutex_unlock(ice->mutex);
+	return;
+    }
+
+    LOG4((ice->obj_name, 
+	 "Check %s%s: connectivity check SUCCESS",
+	 dump_check(buffer, sizeof(buffer), &ice->clist, check),
+	 (check->nominated ? " (nominated)" : " (not nominated)")));
 
     /* Get the STUN XOR-MAPPED-ADDRESS attribute. */
     xaddr = (pj_stun_xor_mapped_addr_attr*)
diff --git a/pjnath/src/pjnath/ice_strans.c b/pjnath/src/pjnath/ice_strans.c
index 297e1f86..5f7d59b6 100644
--- a/pjnath/src/pjnath/ice_strans.c
+++ b/pjnath/src/pjnath/ice_strans.c
@@ -66,7 +66,9 @@ static pj_status_t stun_on_send_msg(pj_stun_session *sess,
 static void stun_on_request_complete(pj_stun_session *sess,
 				     pj_status_t status,
 				     pj_stun_tx_data *tdata,
-				     const pj_stun_msg *response);
+				     const pj_stun_msg *response,
+				     const pj_sockaddr_t *src_addr,
+				     unsigned src_addr_len);
 
 /* Keep-alive timer */
 static void start_ka_timer(pj_ice_strans *ice_st);
@@ -1045,7 +1047,9 @@ static pj_status_t stun_on_send_msg(pj_stun_session *sess,
 static void stun_on_request_complete(pj_stun_session *sess,
 				     pj_status_t status,
 				     pj_stun_tx_data *tdata,
-				     const pj_stun_msg *response)
+				     const pj_stun_msg *response,
+				     const pj_sockaddr_t *src_addr,
+				     unsigned src_addr_len)
 {
     pj_ice_strans_comp *comp;
     pj_ice_strans_cand *cand = NULL;
@@ -1056,6 +1060,9 @@ static void stun_on_request_complete(pj_stun_session *sess,
     comp = (pj_ice_strans_comp*) pj_stun_session_get_user_data(sess);
     cand = (pj_ice_strans_cand*) tdata->user_data;
 
+    PJ_UNUSED_ARG(src_addr);
+    PJ_UNUSED_ARG(src_addr_len);
+
     if (cand == NULL) {
 	/* This is keep-alive */
 	if (status != PJ_SUCCESS) {
diff --git a/pjnath/src/pjnath/stun_msg.c b/pjnath/src/pjnath/stun_msg.c
index da919f8f..1f17171f 100644
--- a/pjnath/src/pjnath/stun_msg.c
+++ b/pjnath/src/pjnath/stun_msg.c
@@ -1095,7 +1095,7 @@ pj_stun_uint64_attr_create(pj_pool_t *pool,
     PJ_ASSERT_RETURN(pool && p_attr, PJ_EINVAL);
 
     attr = PJ_POOL_ZALLOC_T(pool, pj_stun_uint64_attr);
-    INIT_ATTR(attr, attr_type, 4);
+    INIT_ATTR(attr, attr_type, 8);
 
     if (value) {
 	attr->value.u32.hi = value->u32.hi;
@@ -1154,7 +1154,7 @@ static pj_status_t encode_uint64_attr(const void *a, pj_uint8_t *buf,
 	return PJ_ETOOSMALL;
 
     PUTVAL16H(buf, 0, ca->hdr.type);
-    PUTVAL16H(buf, 2, ca->hdr.length);
+    PUTVAL16H(buf, 2, (pj_uint16_t)8);
     PUTVAL64H(buf, 4, &ca->value);
 
     /* Done */
diff --git a/pjnath/src/pjnath/stun_session.c b/pjnath/src/pjnath/stun_session.c
index db1339d7..760aa76d 100644
--- a/pjnath/src/pjnath/stun_session.c
+++ b/pjnath/src/pjnath/stun_session.c
@@ -51,7 +51,9 @@ struct pj_stun_session
 
 static void stun_tsx_on_complete(pj_stun_client_tsx *tsx,
 				 pj_status_t status, 
-				 const pj_stun_msg *response);
+				 const pj_stun_msg *response,
+				 const pj_sockaddr_t *src_addr,
+				 unsigned src_addr_len);
 static pj_status_t stun_tsx_on_send_msg(pj_stun_client_tsx *tsx,
 					const void *stun_pkt,
 					pj_size_t pkt_size);
@@ -325,7 +327,9 @@ static pj_status_t apply_msg_options(pj_stun_session *sess,
 
 static void stun_tsx_on_complete(pj_stun_client_tsx *tsx,
 				 pj_status_t status, 
-				 const pj_stun_msg *response)
+				 const pj_stun_msg *response,
+				 const pj_sockaddr_t *src_addr,
+				 unsigned src_addr_len)
 {
     pj_stun_tx_data *tdata;
 
@@ -333,7 +337,8 @@ static void stun_tsx_on_complete(pj_stun_client_tsx *tsx,
 
     if (tdata->sess->cb.on_request_complete) {
 	(*tdata->sess->cb.on_request_complete)(tdata->sess, status, tdata, 
-					       response);
+					       response, 
+					       src_addr, src_addr_len);
     }
 }
 
@@ -696,7 +701,8 @@ PJ_DEF(pj_status_t) pj_stun_session_cancel_req( pj_stun_session *sess,
     pj_mutex_lock(sess->mutex);
 
     if (notify) {
-	(sess->cb.on_request_complete)(sess, notify_status, tdata, NULL);
+	(sess->cb.on_request_complete)(sess, notify_status, tdata, NULL,
+				       NULL, 0);
     }
 
     /* Just destroy tdata. This will destroy the transaction as well */
@@ -775,7 +781,9 @@ static pj_status_t authenticate_msg(pj_stun_session *sess,
 
 /* Handle incoming response */
 static pj_status_t on_incoming_response(pj_stun_session *sess,
-					pj_stun_msg *msg)
+					pj_stun_msg *msg,
+					const pj_sockaddr_t *src_addr,
+					unsigned src_addr_len)
 {
     pj_stun_tx_data *tdata;
     pj_status_t status;
@@ -792,7 +800,8 @@ static pj_status_t on_incoming_response(pj_stun_session *sess,
      * If the message is accepted, transaction callback will be called,
      * and this will call the session callback too.
      */
-    status = pj_stun_client_tsx_on_rx_msg(tdata->client_tsx, msg);
+    status = pj_stun_client_tsx_on_rx_msg(tdata->client_tsx, msg, 
+					  src_addr, src_addr_len);
     if (status != PJ_SUCCESS) {
 	return status;
     }
@@ -958,7 +967,7 @@ PJ_DEF(pj_status_t) pj_stun_session_on_rx_pkt(pj_stun_session *sess,
     if (PJ_STUN_IS_SUCCESS_RESPONSE(msg->hdr.type) ||
 	PJ_STUN_IS_ERROR_RESPONSE(msg->hdr.type))
     {
-	status = on_incoming_response(sess, msg);
+	status = on_incoming_response(sess, msg, src_addr, src_addr_len);
 
     } else if (PJ_STUN_IS_REQUEST(msg->hdr.type)) {
 
diff --git a/pjnath/src/pjnath/stun_transaction.c b/pjnath/src/pjnath/stun_transaction.c
index 9c5bc11a..32cdec4d 100644
--- a/pjnath/src/pjnath/stun_transaction.c
+++ b/pjnath/src/pjnath/stun_transaction.c
@@ -275,7 +275,7 @@ static void retransmit_timer_callback(pj_timer_heap_t *timer_heap,
 	if (!tsx->complete) {
 	    tsx->complete = PJ_TRUE;
 	    if (tsx->cb.on_complete) {
-		tsx->cb.on_complete(tsx, PJNATH_ESTUNTIMEDOUT, NULL);
+		tsx->cb.on_complete(tsx, PJNATH_ESTUNTIMEDOUT, NULL, NULL, 0);
 	    }
 	}
 	return;
@@ -288,7 +288,7 @@ static void retransmit_timer_callback(pj_timer_heap_t *timer_heap,
 	if (!tsx->complete) {
 	    tsx->complete = PJ_TRUE;
 	    if (tsx->cb.on_complete) {
-		tsx->cb.on_complete(tsx, status, NULL);
+		tsx->cb.on_complete(tsx, status, NULL, NULL, 0);
 	    }
 	}
     }
@@ -313,7 +313,9 @@ static void destroy_timer_callback(pj_timer_heap_t *timer_heap,
  * Notify the STUN transaction about the arrival of STUN response.
  */
 PJ_DEF(pj_status_t) pj_stun_client_tsx_on_rx_msg(pj_stun_client_tsx *tsx,
-						 const pj_stun_msg *msg)
+						 const pj_stun_msg *msg,
+						 const pj_sockaddr_t *src_addr,
+						 unsigned src_addr_len)
 {
     pj_stun_errcode_attr *err_attr;
     pj_status_t status;
@@ -363,7 +365,7 @@ PJ_DEF(pj_status_t) pj_stun_client_tsx_on_rx_msg(pj_stun_client_tsx *tsx,
     if (!tsx->complete) {
 	tsx->complete = PJ_TRUE;
 	if (tsx->cb.on_complete) {
-	    tsx->cb.on_complete(tsx, status, msg);
+	    tsx->cb.on_complete(tsx, status, msg, src_addr, src_addr_len);
 	}
     }