From e1faefeecdbacae77f3d9f7ae0afea1421622ec4 Mon Sep 17 00:00:00 2001 From: bossiel Date: Fri, 25 Jul 2014 03:22:49 +0000 Subject: [PATCH] Fix BFCP revocation issue on Radvision TelePresence system --- .../tinyBFCP/include/tinybfcp/tbfcp_session.h | 1 + .../2.0/doubango/tinyBFCP/src/tbfcp_session.c | 25 ++++ .../2.0/doubango/tinyBFCP/src/tbfcp_utils.c | 4 +- .../tinyDAV/src/bfcp/tdav_session_bfcp.c | 128 +++++++++++++++--- 4 files changed, 138 insertions(+), 20 deletions(-) diff --git a/branches/2.0/doubango/tinyBFCP/include/tinybfcp/tbfcp_session.h b/branches/2.0/doubango/tinyBFCP/include/tinybfcp/tbfcp_session.h index 6343ec11..af5669ff 100644 --- a/branches/2.0/doubango/tinyBFCP/include/tinybfcp/tbfcp_session.h +++ b/branches/2.0/doubango/tinyBFCP/include/tinybfcp/tbfcp_session.h @@ -64,6 +64,7 @@ TINYBFCP_API int tbfcp_session_get_local_setup(const struct tbfcp_session_s* pc_ TINYBFCP_API int tbfcp_session_get_local_address(const struct tbfcp_session_s* pc_self, const char** ppc_ip, tnet_port_t *pu_port); TINYBFCP_API int tbfcp_session_create_pkt_Hello(struct tbfcp_session_s* p_self, struct tbfcp_pkt_s** pp_pkt); TINYBFCP_API int tbfcp_session_create_pkt_FloorRequest(struct tbfcp_session_s* p_self, struct tbfcp_pkt_s** pp_pkt); +TINYBFCP_API int tbfcp_session_create_pkt_FloorRelease(struct tbfcp_session_s* p_self, struct tbfcp_pkt_s** pp_pkt); TINYBFCP_API int tbfcp_session_send_pkt(struct tbfcp_session_s* p_self, const struct tbfcp_pkt_s* pc_pkt); TBFCP_END_DECLS diff --git a/branches/2.0/doubango/tinyBFCP/src/tbfcp_session.c b/branches/2.0/doubango/tinyBFCP/src/tbfcp_session.c index cb281887..c29e1f5d 100644 --- a/branches/2.0/doubango/tinyBFCP/src/tbfcp_session.c +++ b/branches/2.0/doubango/tinyBFCP/src/tbfcp_session.c @@ -381,6 +381,10 @@ int tbfcp_session_stop(tbfcp_session_t* p_self) TSK_DEBUG_INFO("BFCP session already stopped"); goto bail; } + + // remove all pending udp packets + tsk_list_clear_items(p_self->p_list_udp_pkts); + p_self->b_stopping = tsk_true; // this is a global timer shared by many components -> stopping it won't remove // all scheduled items as it could continue running if still used @@ -564,6 +568,25 @@ bail: return ret; } +int tbfcp_session_create_pkt_FloorRelease(struct tbfcp_session_s* p_self, struct tbfcp_pkt_s** pp_pkt) +{ + int ret; + if (!p_self || !pp_pkt) { + TSK_DEBUG_ERROR("Invalid parameter"); + return -1; + } + // lock() + tsk_safeobj_lock(p_self); + if ((ret = tbfcp_pkt_create_FloorRelease_2(p_self->conf_ids.u_conf_id, tbfcp_utils_rand_u16(), p_self->conf_ids.u_user_id, p_self->conf_ids.u_floor_id, pp_pkt))) { + goto bail; + } + +bail: + // lock() + tsk_safeobj_unlock(p_self); + return ret; +} + static int _tbfcp_session_send_buff(tbfcp_session_t* p_self, const void* pc_buff_ptr, tsk_size_t u_buff_size) { int ret = 0; @@ -850,6 +873,7 @@ static int _tbfcp_session_timer_callback(const void* pc_arg, tsk_timer_id_t time tbfcp_session_t* p_session = (tbfcp_session_t*)pc_arg; const tsk_list_item_t* pc_item; tsk_safeobj_lock(p_session); // must + if (!p_session->b_started) goto bail; pc_item = tsk_list_find_item_by_pred(p_session->p_list_udp_pkts, __pred_find_udp_pkt_by_timer, &timer_id); if (pc_item) { tbfcp_udp_pkt_t* pc_udp_pkt = (tbfcp_udp_pkt_t*)pc_item->data; @@ -873,6 +897,7 @@ static int _tbfcp_session_timer_callback(const void* pc_arg, tsk_timer_id_t time // OnExpire(session, EVENT_REPORT); } #endif +bail: tsk_safeobj_unlock(p_session); return 0; } diff --git a/branches/2.0/doubango/tinyBFCP/src/tbfcp_utils.c b/branches/2.0/doubango/tinyBFCP/src/tbfcp_utils.c index d50c125d..7c0ff254 100644 --- a/branches/2.0/doubango/tinyBFCP/src/tbfcp_utils.c +++ b/branches/2.0/doubango/tinyBFCP/src/tbfcp_utils.c @@ -185,5 +185,7 @@ int tbfcp_utils_is_setup_acceptable(enum tbfcp_setup_e e_setup_local, enum tbfcp uint16_t tbfcp_utils_rand_u16() { - return (rand() ^ rand()) % 0xFFFF; + static long __rand = 0; + long num = tsk_atomic_inc(&__rand); + return ((num % 0xFF) << 8) | (tsk_time_epoch() % 0xFF); } diff --git a/branches/2.0/doubango/tinyDAV/src/bfcp/tdav_session_bfcp.c b/branches/2.0/doubango/tinyDAV/src/bfcp/tdav_session_bfcp.c index 6900ef7a..8cfc82bc 100644 --- a/branches/2.0/doubango/tinyDAV/src/bfcp/tdav_session_bfcp.c +++ b/branches/2.0/doubango/tinyDAV/src/bfcp/tdav_session_bfcp.c @@ -47,7 +47,10 @@ typedef struct tdav_session_bfcp_s struct tbfcp_pkt_s* p_pkt_FloorRelease; struct tbfcp_pkt_s* p_pkt_Hello; + tsk_bool_t b_started; tsk_bool_t b_use_ipv6; + tsk_bool_t b_revoked_handled; + tsk_bool_t b_conf_idf_changed; char* p_local_ip; //uint16_t local_port; @@ -69,7 +72,7 @@ typedef struct tdav_session_bfcp_s tdav_session_bfcp_t; static int _tdav_session_bfcp_notif(const struct tbfcp_session_event_xs *e); - +static int _tdav_session_bfcp_send_Hello(tdav_session_bfcp_t* p_bfcp); /* ============ Plugin interface ================= */ @@ -159,6 +162,8 @@ static int _tdav_session_bfcp_prepare(tmedia_session_t* p_self) } TSK_OBJECT_SAFE_FREE(p_bfcp->p_pkt_Hello); TSK_OBJECT_SAFE_FREE(p_bfcp->p_pkt_FloorRequest); + TSK_OBJECT_SAFE_FREE(p_bfcp->p_pkt_FloorRelease); + p_bfcp->b_revoked_handled = tsk_false; return ret; } @@ -183,15 +188,11 @@ static int _tdav_session_bfcp_start(tmedia_session_t* p_self) if ((ret = tbfcp_session_start(p_bfcp->p_bfcp_s))) { return ret; } + if ((ret = _tdav_session_bfcp_send_Hello(p_bfcp))) { + return ret; + } - // Create Hello if not already done - if (!p_bfcp->p_pkt_Hello && (ret = tbfcp_session_create_pkt_Hello(p_bfcp->p_bfcp_s, &p_bfcp->p_pkt_Hello))) { - return ret; - } - // Send Hello - if ((ret = tbfcp_session_send_pkt(p_bfcp->p_bfcp_s, p_bfcp->p_pkt_Hello))) { - return ret; - } + p_bfcp->b_started = tsk_true; return ret; } @@ -231,10 +232,22 @@ static int _tdav_session_bfcp_stop(tmedia_session_t* p_self) p_bfcp = (tdav_session_bfcp_t*)p_self; + if (p_bfcp->b_started) { +#if 0 // Must not ... because could be caused by an incoming reINVITE. + if (p_bfcp->p_pkt_FloorRelease) { + if ((ret = tbfcp_session_send_pkt(p_bfcp->p_bfcp_s, p_bfcp->p_pkt_FloorRelease))) { + + } + } +#endif + } + if (p_bfcp->p_bfcp_s) { ret = tbfcp_session_stop(p_bfcp->p_bfcp_s); } + p_bfcp->b_started = tsk_false; + return ret; } @@ -359,16 +372,19 @@ static int _tdav_session_bfcp_set_ro(tmedia_session_t* p_self, const tsdp_header // https://tools.ietf.org/html/rfc4583 { + p_bfcp->b_conf_idf_changed = tsk_false; if ((A = tsdp_header_M_findA(m, "floorctrl"))) { if ((ret = tbfcp_utils_parse_role(A->value, &e_remote_role))) { return ret; } } if ((A = tsdp_header_M_findA(m, "confid"))) { + p_bfcp->b_conf_idf_changed |= !tsk_striequals(p_bfcp->rfc4583.confid, A->value); tsk_strupdate(&p_bfcp->rfc4583.confid, A->value); u_remote_conf_id = (uint32_t)tsk_atoi64(p_bfcp->rfc4583.confid); } if ((A = tsdp_header_M_findA(m, "userid"))) { + p_bfcp->b_conf_idf_changed |= !tsk_striequals(p_bfcp->rfc4583.userid, A->value); tsk_strupdate(&p_bfcp->rfc4583.userid, A->value); u_remote_user_id = (uint16_t)tsk_atoi64(p_bfcp->rfc4583.userid); } @@ -376,11 +392,13 @@ static int _tdav_session_bfcp_set_ro(tmedia_session_t* p_self, const tsdp_header char tmp_str[256]; if (sscanf(A->value, "%255s %*s", tmp_str) != EOF) { char *pch, *saveptr; + p_bfcp->b_conf_idf_changed |= !tsk_striequals(p_bfcp->rfc4583.floorid, tmp_str); tsk_strupdate(&p_bfcp->rfc4583.floorid, tmp_str); u_remote_floor_id = (uint16_t)tsk_atoi64(p_bfcp->rfc4583.floorid); pch = tsk_strtok_r(&A->value[tsk_strlen(tmp_str) + 1], " ", &saveptr); while (pch) { if (sscanf(pch, "mstrm: %255s", tmp_str) != EOF) { + p_bfcp->b_conf_idf_changed |= !tsk_striequals(p_bfcp->rfc4583.mstrm, tmp_str); tsk_strupdate(&p_bfcp->rfc4583.mstrm, tmp_str); break; } @@ -415,10 +433,29 @@ static int _tdav_session_bfcp_set_ro(tmedia_session_t* p_self, const tsdp_header return ret; } +static int _tdav_session_bfcp_send_Hello(tdav_session_bfcp_t* p_bfcp) +{ + int ret = 0; + if (!p_bfcp) { + TSK_DEBUG_ERROR("Invalid parameter"); + return -1; + } + + if (!p_bfcp->p_pkt_Hello && (ret = tbfcp_session_create_pkt_Hello(p_bfcp->p_bfcp_s, &p_bfcp->p_pkt_Hello))) { + return ret; + } + if ((ret = tbfcp_session_send_pkt(p_bfcp->p_bfcp_s, p_bfcp->p_pkt_Hello))) { + return ret; + } + return ret; +} + static int _tdav_session_bfcp_notif(const struct tbfcp_session_event_xs *e) { tdav_session_bfcp_t* p_bfcp = tsk_object_ref(TSK_OBJECT(e->pc_usr_data)); int ret = 0; + static const char* kErrTextGlobalError = "Global error"; + static const int kErrCodeGlobalError = -56; static const char* kErrTextTimeout = "Timeout"; static const int kErrCodeTimeout = -57; static const char* kErrTextUnExpectedIncomingMsg = "Unexpected incoming BFCP message"; @@ -448,11 +485,21 @@ static int _tdav_session_bfcp_notif(const struct tbfcp_session_event_xs *e) TSK_OBJECT_SAFE_FREE(p_bfcp->p_pkt_Hello); if (e->pc_pkt->hdr.primitive == tbfcp_primitive_HelloAck) { if (!p_bfcp->p_pkt_FloorRequest) { - if ((ret = tbfcp_session_create_pkt_FloorRequest(p_bfcp->p_bfcp_s, &p_bfcp->p_pkt_FloorRequest))) { - goto bail; + if (p_bfcp->b_conf_idf_changed) { + // Create the "FloorRelease" for this "FloorRequest" + TSK_OBJECT_SAFE_FREE(p_bfcp->p_pkt_FloorRelease); + if ((ret = tbfcp_session_create_pkt_FloorRelease(p_bfcp->p_bfcp_s, &p_bfcp->p_pkt_FloorRelease))) { + goto raise_err; + } + if ((ret = tbfcp_session_create_pkt_FloorRequest(p_bfcp->p_bfcp_s, &p_bfcp->p_pkt_FloorRequest))) { + goto raise_err; + } + if ((ret = tbfcp_session_send_pkt(p_bfcp->p_bfcp_s, p_bfcp->p_pkt_FloorRequest))) { + goto raise_err; + } } - if ((ret = tbfcp_session_send_pkt(p_bfcp->p_bfcp_s, p_bfcp->p_pkt_FloorRequest))) { - goto bail; + else { + TSK_DEBUG_INFO("No change to BFCP session... do not send FloorRequest"); } } } @@ -461,7 +508,8 @@ static int _tdav_session_bfcp_notif(const struct tbfcp_session_event_xs *e) _RAISE_ERR_AND_GOTO_BAIL(kErrCodeUnExpectedIncomingMsg, kErrTextUnExpectedIncomingMsg); } } - else if(p_bfcp->p_pkt_FloorRequest && p_bfcp->p_pkt_FloorRequest->hdr.transac_id == e->pc_pkt->hdr.transac_id && p_bfcp->p_pkt_FloorRequest->hdr.user_id == e->pc_pkt->hdr.user_id && p_bfcp->p_pkt_FloorRequest->hdr.conf_id == e->pc_pkt->hdr.conf_id) { + else if(p_bfcp->p_pkt_FloorRequest /*&& p_bfcp->p_pkt_FloorRequest->hdr.transac_id == e->pc_pkt->hdr.transac_id*/ && p_bfcp->p_pkt_FloorRequest->hdr.user_id == e->pc_pkt->hdr.user_id && p_bfcp->p_pkt_FloorRequest->hdr.conf_id == e->pc_pkt->hdr.conf_id) { + tsk_bool_t transac_id_matched = (p_bfcp->p_pkt_FloorRequest->hdr.transac_id == e->pc_pkt->hdr.transac_id); if (e->pc_pkt->hdr.primitive == tbfcp_primitive_FloorRequestStatus) { tsk_size_t u_index0, u_index1, u_index2, u_index3; const tbfcp_attr_grouped_t *pc_attr_FloorRequestInformation = tsk_null, @@ -510,10 +558,40 @@ static int _tdav_session_bfcp_notif(const struct tbfcp_session_event_xs *e) } } - if (pc_attr_RequestStatus) { + if (pc_attr_RequestStatus) { // https://tools.ietf.org/html/rfc4582#section-5.2.5 uint16_t u_status = pc_attr_RequestStatus->OctetString16[0] + (pc_attr_RequestStatus->OctetString16[1] << 8); - _RAISE_FLREQ(u_status, kInfoTextFloorReqStatus); + if (transac_id_matched) { + if (u_status == tbfcp_reqstatus_Revoked && !p_bfcp->b_revoked_handled) { // revoked + TSK_OBJECT_SAFE_FREE(p_bfcp->p_pkt_FloorRequest); // free the FloorRequest and ask new one once HelloAck is received + // Radvision sends a Revoke after a reINVITE to ask for new negotiation. + if (p_bfcp->p_pkt_FloorRelease) { + if ((ret = tbfcp_session_send_pkt(p_bfcp->p_bfcp_s, p_bfcp->p_pkt_FloorRelease))) { + goto raise_err; + } + } + if ((ret = _tdav_session_bfcp_send_Hello(p_bfcp))) { + goto raise_err; + } + p_bfcp->b_revoked_handled = tsk_true; + } + else { + _RAISE_FLREQ(u_status, kInfoTextFloorReqStatus); + } + } + else { //!transac_id_matched + // Status from old FloorRequest + tbfcp_pkt_t* p_pkt = tsk_null; + TSK_DEBUG_INFO("Status from old Request"); + if ((ret = tbfcp_pkt_create_FloorRelease_2(e->pc_pkt->hdr.conf_id, e->pc_pkt->hdr.transac_id, e->pc_pkt->hdr.user_id, pc_attr_FloorRequestStatus->extra_hdr.FloorID, &p_pkt))) { + goto raise_err; + } + ret = tbfcp_session_send_pkt(p_bfcp->p_bfcp_s, p_pkt); + TSK_OBJECT_SAFE_FREE(p_pkt); + if (ret) { + goto raise_err; + } + } } else { /* /!\ No RequestStatus attribute in FloorRequestStatus */ @@ -523,8 +601,15 @@ static int _tdav_session_bfcp_notif(const struct tbfcp_session_event_xs *e) } } else { - TSK_DEBUG_ERROR("%s", kErrTextUnExpectedIncomingMsg); - _RAISE_ERR_AND_GOTO_BAIL(kErrCodeUnExpectedIncomingMsg, kErrTextUnExpectedIncomingMsg); + switch (e->pc_pkt->hdr.primitive) { + case tbfcp_primitive_Hello: break; // already handled in "_tbfcp_session_process_incoming_pkt()" + default: + { + TSK_DEBUG_ERROR("%s", kErrTextUnExpectedIncomingMsg); + _RAISE_ERR_AND_GOTO_BAIL(kErrCodeUnExpectedIncomingMsg, kErrTextUnExpectedIncomingMsg); + break; + } + } } } break; @@ -537,8 +622,13 @@ static int _tdav_session_bfcp_notif(const struct tbfcp_session_event_xs *e) break; } } +raise_err: + if (ret) { + TSK_DEBUG_ERROR("%s", kErrTextGlobalError); + _RAISE_ERR_AND_GOTO_BAIL(kErrCodeGlobalError, kErrTextGlobalError); + } bail: - + TSK_OBJECT_SAFE_FREE(p_bfcp); return ret; }