Changeset 5847 for pjproject

Timestamp:

Jul 27, 2018 6:16:26 AM (6 years ago)

Author:

nanang

Message:

Re #2096:

• Updated DTLS-SRTP to ignore stray packet after DTLS nego has been completed.
• Added few debugging logs.
• Fixed false SSL error.

File:

• pjproject/trunk/pjmedia/src/pjmedia/transport_srtp_dtls.c (modified) (13 diffs)

pjproject/trunk/pjmedia/src/pjmedia/transport_srtp_dtls.c

@@ -107 +107 @@
     pj_bool_t            use_ice;
     pj_bool_t            nego_started;
+    pj_bool_t            nego_completed;
     pj_str_t             rem_fingerprint;   /* Remote fingerprint in SDP    */
     pj_status_t          rem_fprint_status; /* Fingerprint verif. status    */
@@ -229 +230 @@
         status = ERR_GET_REASON(err);
 
-    status += PJ_SSL_ERRNO_START;
+    if (status != PJ_SUCCESS)
+        status += PJ_SSL_ERRNO_START;
+
     ds->last_err = err;
     return status;
@@ -642 +645 @@
     }
 
-    /* Check if handshake has been completed */
-    if (!SSL_is_init_finished(ds->ossl_ssl))
+    /* Just return if handshake completion procedure (key parsing, fingerprint
+     * verification, etc) has been done or handshake is still in progress.
+     */
+    if (ds->nego_completed || !SSL_is_init_finished(ds->ossl_ssl))
         return PJ_SUCCESS;
 
     /* Yes, SSL handshake is done! */
+    ds->nego_completed = PJ_TRUE;
     PJ_LOG(2,(ds->base.name, "DTLS-SRTP negotiation completed!"));
 
-    /* Stop the retrans clock */
+    /* Stop the retransmission clock. Note that the clock may not be stopped
+     * if this function is called from clock thread context. We'll try again
+     * later in socket context.
+     */
     if (ds->clock)
         pjmedia_clock_stop(ds->clock);
@@ -963 +972 @@
     dtls_srtp *ds = (dtls_srtp*)tp;
 
+    /* Destroy the retransmission clock if handshake has been completed. */
+    if (ds->clock && ds->nego_completed) {
+        pjmedia_clock_destroy(ds->clock);
+        ds->clock = NULL;
+    }
+
     if (size < 1 || !IS_DTLS_PKT(pkt, size))
         return PJ_EIGNORED;
@@ -970 +985 @@
 #endif
 
-    /* This is DTLS packet, let's process it */
+    /* This is DTLS packet, let's process it. Note that if DTLS nego has
+     * been completed, this may be a retransmission (e.g: remote didn't
+     * receive our last handshake packet) or just a stray.
+     */
 
     /* Check remote address info, reattach member tp if changed */
-    if (!ds->use_ice) {
+    if (!ds->use_ice && !ds->nego_completed) {
         pjmedia_transport_info info;
         pjmedia_transport_get_info(ds->srtp->member_tp, &info);
@@ -999 +1017 @@
 
             pjmedia_transport_attach2(&ds->srtp->base, &ap);
+
+#if DTLS_DEBUG
+            {
+                char addr[PJ_INET6_ADDRSTRLEN];
+                PJ_LOG(2,(ds->base.name, "Re-attached transport to update "
+                          "remote addr=%s:%d",
+                          pj_sockaddr_print(&ap.rem_addr, addr,
+                                            sizeof(addr), 2),
+                          pj_sockaddr_get_port(&ap.rem_addr)));
+            }
+#endif
         }
     }
@@ -1005 +1034 @@
      * so let's update setup to PASSIVE and initiate DTLS handshake.
      */
-    if (ds->setup == DTLS_SETUP_ACTPASS || ds->setup == DTLS_SETUP_PASSIVE)
+    if (!ds->nego_started &&
+        (ds->setup == DTLS_SETUP_ACTPASS || ds->setup == DTLS_SETUP_PASSIVE))
     {
         pj_status_t status;
@@ -1167 +1197 @@
             ssl_destroy(ds);
             ds->nego_started = PJ_FALSE;
+            ds->nego_completed = PJ_FALSE;
             ds->got_keys = PJ_FALSE;
             ds->rem_fprint_status = PJ_EPENDING;
@@ -1198 +1229 @@
     }
 
-    if (ds->got_keys) {
-        /* This is subsequent SDP offer/answer and we already got SRTP keys */
+    if (ds->nego_completed) {
+        /* This is subsequent SDP offer/answer and no DTLS re-nego has been
+         * signalled.
+         */
         goto on_return;
     }
@@ -1247 +1280 @@
          */
         udp_member_transport_media_start(ds);
+
+#if DTLS_DEBUG
+        {
+            char addr[PJ_INET6_ADDRSTRLEN];
+            PJ_LOG(2,(ds->base.name, "Attached transport, remote addr=%s:%d",
+                      pj_sockaddr_print(&ap.rem_addr, addr, sizeof(addr), 2),
+                      pj_sockaddr_get_port(&ap.rem_addr)));
+        }
+#endif
     }
 
@@ -1317 +1359 @@
             ssl_destroy(ds);
             ds->nego_started = PJ_FALSE;
+            ds->nego_completed = PJ_FALSE;
             ds->got_keys = PJ_FALSE;
             ds->rem_fprint_status = PJ_EPENDING;
@@ -1401 +1444 @@
             if (status != PJ_SUCCESS)
                 goto on_return;
+#if DTLS_DEBUG
+            {
+                char addr[PJ_INET6_ADDRSTRLEN];
+                PJ_LOG(2,(ds->base.name, "Attached transport, "
+                          "remote addr=%s:%d",
+                          pj_sockaddr_print(&ap.rem_addr, addr,
+                          sizeof(addr), 2),
+                          pj_sockaddr_get_port(&ap.rem_addr)));
+            }
+#endif
             
             status = ssl_handshake(ds);
@@ -1518 +1571 @@
         goto on_return;
 
+#if DTLS_DEBUG
+    {
+        char addr[PJ_INET6_ADDRSTRLEN];
+        PJ_LOG(2,(ds->base.name, "Attached transport, remote addr=%s:%d",
+                  pj_sockaddr_print(&ap.rem_addr, addr, sizeof(addr), 2),
+                  pj_sockaddr_get_port(&ap.rem_addr)));
+    }
+#endif
+
     /* Start DTLS handshake */
     pj_bzero(&srtp->rx_policy_neg, sizeof(srtp->rx_policy_neg));