From f0a1e43d5a2ea367575cbd40927a1bafe83959f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Blin?= <sebastien.blin@savoirfairelinux.com> Date: Thu, 27 Aug 2020 10:03:24 -0400 Subject: [PATCH] pjproject: correctly wait that the peer authorize ourself When connecting to a TURN server, the peer needs to authorize ours candidates to access to that server. This check existed, but not at the good place. Indeed, the connection' status is given by ice_sess_on_peer_connection, not by the first message we send (because the first message is sent when the connection already exists). I still don't have any clue why this problem was frequent only for the first connection between two peers. Gitlab: #227 Change-Id: I7b6e6b7b47a05c9463233761ef562820bed1521a --- contrib/src/pjproject/0001-rfc6544.patch | 51 ++++++++++++------------ 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/contrib/src/pjproject/0001-rfc6544.patch b/contrib/src/pjproject/0001-rfc6544.patch index fe8e05a1e2..7bc2ac0886 100644 --- a/contrib/src/pjproject/0001-rfc6544.patch +++ b/contrib/src/pjproject/0001-rfc6544.patch @@ -28,7 +28,7 @@ on behalf of Savoir-faire Linux. pjnath/src/pjnath-test/concur_test.c | 5 +- pjnath/src/pjnath-test/sess_auth.c | 14 +- pjnath/src/pjnath-test/stun_sock_test.c | 7 +- - pjnath/src/pjnath/ice_session.c | 491 +++++++++-- + pjnath/src/pjnath/ice_session.c | 490 +++++++++-- pjnath/src/pjnath/ice_strans.c | 745 +++++++++++++--- pjnath/src/pjnath/nat_detect.c | 7 +- pjnath/src/pjnath/stun_session.c | 15 +- @@ -41,7 +41,7 @@ on behalf of Savoir-faire Linux. pjnath/src/pjturn-srv/server.c | 2 +- pjsip-apps/src/samples/icedemo.c | 116 ++- pjsip/src/pjsua-lib/pjsua_core.c | 2 +- - 21 files changed, 2430 insertions(+), 409 deletions(-) + 21 files changed, 2429 insertions(+), 409 deletions(-) diff --git a/pjnath/include/pjnath/ice_session.h b/pjnath/include/pjnath/ice_session.h index 8971220f0..4cccd7c64 100644 @@ -648,7 +648,7 @@ index fff4fad26..e7f8b84eb 100644 if (status != PJ_SUCCESS && status != PJ_EPENDING) { app_perror(" error: server sending data", status); diff --git a/pjnath/src/pjnath/ice_session.c b/pjnath/src/pjnath/ice_session.c -index 2a4125bc5..c3a06fbc4 100644 +index 2a4125bc5..3f8c03cd2 100644 --- a/pjnath/src/pjnath/ice_session.c +++ b/pjnath/src/pjnath/ice_session.c @@ -18,6 +18,7 @@ @@ -1129,7 +1129,7 @@ index 2a4125bc5..c3a06fbc4 100644 } pj_grp_lock_release(ice->grp_lock); -@@ -2181,6 +2393,179 @@ static pj_status_t on_stun_send_msg(pj_stun_session *sess, +@@ -2181,6 +2393,178 @@ static pj_status_t on_stun_send_msg(pj_stun_session *sess, return status; } @@ -1184,7 +1184,23 @@ index 2a4125bc5..c3a06fbc4 100644 + } + } + -+ if (status != PJ_SUCCESS) { ++ const pj_ice_sess_cand *rcand = check->rcand; ++ if ((status == 120104 || status == 130054)/* CONNECTION RESET BY PEER */ ++ && rcand->type == PJ_ICE_CAND_TYPE_RELAYED) { ++ /** ++ * This part of the code is triggered when using ICE over TCP via TURN ++ * In fact, the other peer has to authorize this peer to connect to ++ * the relayed candidate. This is done by set_perm from the other case. ++ * But from this side, we can't know if the peer has authorized us. If it's ++ * not the case, the connection will got a CONNECTION RESET BY PEER status. ++ * In this case, we can try to reconnect a bit after and this until the check ++ * reached its timeout. ++ */ ++ check->state = PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY; ++ check_set_state(ice, check,PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY, ++ status); ++ return; ++ } else if (status != PJ_SUCCESS) { + check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_FAILED, status); + on_check_complete(ice, check); + return; @@ -1192,7 +1208,6 @@ index 2a4125bc5..c3a06fbc4 100644 + + // TCP is correctly connected. Craft the message to send + const pj_ice_sess_cand *lcand = check->lcand; -+ const pj_ice_sess_cand *rcand = check->rcand; + if (check->tdata == NULL) { + LOG5((ice->obj_name, "Error sending STUN request, empty data")); + return; @@ -1227,22 +1242,6 @@ index 2a4125bc5..c3a06fbc4 100644 + check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY, + status_send_msg); + return; -+ } -+ -+ if ((status_send_msg == 120104 || status_send_msg == 130054)/* CONNECTION RESET BY PEER */ -+ && rcand->type == PJ_ICE_CAND_TYPE_RELAYED) { -+ /** -+ * This part of the code is triggered when using ICE over TCP via TURN -+ * In fact, the other peer has to authorize this peer to connect to -+ * the relayed candidate. This is done by set_perm from the other case. -+ * But from this side, we can't know if the peer has authorized us. If it's -+ * not the case, the connection will got a CONNECTION RESET BY PEER status. -+ * In this case, we can try to reconnect a bit after and this until the check -+ * reached its timeout. -+ */ -+ check->state = PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY; -+ check_set_state(ice, check,PJ_ICE_SESS_CHECK_STATE_NEEDS_RETRY, -+ status_send_msg); + } else if (status_send_msg != PJ_SUCCESS) { + check->tdata = NULL; + pjnath_perror(ice->obj_name, "Error sending STUN request", status_send_msg); @@ -1309,7 +1308,7 @@ index 2a4125bc5..c3a06fbc4 100644 /* This callback is called when outgoing STUN request completed */ static void on_stun_request_complete(pj_stun_session *stun_sess, -@@ -2411,7 +2796,9 @@ static void on_stun_request_complete(pj_stun_session *stun_sess, +@@ -2411,7 +2795,9 @@ static void on_stun_request_complete(pj_stun_session *stun_sess, &check->lcand->base_addr, &check->lcand->base_addr, pj_sockaddr_get_len(&xaddr->sockaddr), @@ -1320,7 +1319,7 @@ index 2a4125bc5..c3a06fbc4 100644 if (status != PJ_SUCCESS) { check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_FAILED, status); -@@ -2474,11 +2861,7 @@ static void on_stun_request_complete(pj_stun_session *stun_sess, +@@ -2474,11 +2860,7 @@ static void on_stun_request_complete(pj_stun_session *stun_sess, /* Perform 7.1.2.2.2. Updating Pair States. * This may terminate ICE processing. */ @@ -1333,7 +1332,7 @@ index 2a4125bc5..c3a06fbc4 100644 pj_grp_lock_release(ice->grp_lock); } -@@ -2673,7 +3056,9 @@ static pj_status_t on_stun_rx_request(pj_stun_session *sess, +@@ -2673,7 +3055,9 @@ static pj_status_t on_stun_rx_request(pj_stun_session *sess, msg_data->has_req_data = PJ_FALSE; /* Send the response */ @@ -1344,7 +1343,7 @@ index 2a4125bc5..c3a06fbc4 100644 src_addr, src_addr_len, tdata); -@@ -2794,12 +3179,12 @@ static void handle_incoming_check(pj_ice_sess *ice, +@@ -2794,12 +3178,12 @@ static void handle_incoming_check(pj_ice_sess *ice, /* Just get candidate with the highest priority and same transport ID * for the specified component ID in the checklist. */ -- GitLab