Changeset 5855

Timestamp:

Aug 15, 2018 2:44:53 AM (6 years ago)

Author:

nanang

Message:

Re #2096: Answer with one SRTP keying only when offered both (thanks Wei-Wei for the feedback).

Location:

pjproject/trunk

Files:

• pjmedia/src/pjmedia/transport_srtp.c (modified) (7 diffs)
• pjmedia/src/pjmedia/transport_srtp_dtls.c (modified) (4 diffs)
• pjmedia/src/pjmedia/transport_srtp_sdes.c (modified) (1 diff)
• tests/pjsua/scripts-sipp/uac-srtp-dtls-reinv-sdes.py (added)
• tests/pjsua/scripts-sipp/uac-srtp-dtls-reinv-sdes.xml (added)
• tests/pjsua/scripts-sipp/uac-srtp-dtls.py (added)
• tests/pjsua/scripts-sipp/uac-srtp-dtls.xml (added)
• tests/pjsua/scripts-sipp/uac-srtp-sdes-reinv-dtls.py (added)
• tests/pjsua/scripts-sipp/uac-srtp-sdes-reinv-dtls.xml (added)
• tests/pjsua/scripts-sipp/uac-srtp-sdes.py (added)
• tests/pjsua/scripts-sipp/uac-srtp-sdes.xml (added)

pjproject/trunk/pjmedia/src/pjmedia/transport_srtp.c

@@ -267 +267 @@
     pjmedia_transport   *member_tp; /**< Underlying transport.       */
     pj_bool_t            member_tp_attached;
+    pj_bool_t            started;
 
     /* SRTP usage policy of peer. This field is updated when media is starting.
@@ -1051 +1052 @@
         if (status != PJ_SUCCESS)
             return status;
+
+        PJ_LOG(4, (srtp->pool->obj_name,
+                   "SRTP started, keying=%s, crypto=%s",
+                   (srtp->keying[0]->type==PJMEDIA_SRTP_KEYING_SDES?
+                    "SDES":"DTLS-SRTP"),
+                   srtp->tx_policy.name.ptr));
     }
 
@@ -1579 +1586 @@
     srtp->offerer_side = (sdp_remote == NULL);
 
+    if (!srtp->offerer_side && srtp->started) {
+        /* This is may be incoming reoffer that may change keying */
+        srtp->bypass_srtp = PJ_FALSE;
+        srtp->keying_cnt = srtp->all_keying_cnt;
+        for (i = 0; i < srtp->all_keying_cnt; ++i)
+            srtp->keying[i] = srtp->all_keying[i];
+    }
+
     status = pjmedia_transport_encode_sdp(srtp->member_tp, sdp_pool,
                                           sdp_local, sdp_remote, media_index);
@@ -1611 +1626 @@
             keying_status = st;
             continue;
-        }
-
-        if (!srtp_crypto_empty(&srtp->tx_policy_neg) &&
-            !srtp_crypto_empty(&srtp->rx_policy_neg))
-        {
-            /* SRTP nego is done */
+        } else if (!srtp->offerer_side) {
+            /* Answer with one keying only */
+            srtp->keying[0] = srtp->keying[i];
             srtp->keying_cnt = 1;
-            srtp->keying[0] = srtp->keying[i];
-            srtp->keying_pending_cnt = 0;
             break;
         }
@@ -1629 +1639 @@
     if (srtp->keying_cnt == 0)
         return keying_status;
+
+    /* Bypass SRTP & skip keying as SRTP is disabled and verification on
+     * remote SDP has been done.
+     */
+    if (srtp->setting.use == PJMEDIA_SRTP_DISABLED) {
+        srtp->bypass_srtp = PJ_TRUE;
+        srtp->keying_cnt = 0;
+    }
+
+    if (srtp->keying_cnt != 0) {
+        /* At this point for now, keying count should be 1 */
+        pj_assert(srtp->keying_cnt == 1);
+        PJ_LOG(4, (srtp->pool->obj_name, "SRTP uses keying method %s",
+                   (srtp->keying[0]->type==PJMEDIA_SRTP_KEYING_SDES?
+                    "SDES":"DTLS-SRTP")));
+    }
 
     return PJ_SUCCESS;
@@ -1646 +1672 @@
 
     PJ_ASSERT_RETURN(tp, PJ_EINVAL);
+
+    /* At this point for now, keying count should be 0 or 1 */
+    pj_assert(srtp->keying_cnt <= 1);
+
+    srtp->started = PJ_TRUE;
 
     status = pjmedia_transport_media_start(srtp->member_tp, pool,
@@ -1717 +1748 @@
     PJ_ASSERT_RETURN(tp, PJ_EINVAL);
 
+    srtp->started = PJ_FALSE;
+
     /* Invoke media_stop() of all keying methods */
     for (i=0; i < srtp->keying_cnt; ++i) {

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

@@ -190 +190 @@
 
     pj_ansi_strncpy(ds->base.name, pool->obj_name, PJ_MAX_OBJ_NAME);
-    ds->base.type = PJMEDIA_TRANSPORT_TYPE_SRTP;
+    ds->base.type = PJMEDIA_SRTP_KEYING_DTLS_SRTP;
     ds->base.op = &dtls_op;
     ds->base.user_data = srtp;
@@ -1075 +1075 @@
          */
         pjmedia_sdp_media *m_rem = sdp_remote->media[media_index];
-        pjmedia_sdp_attr *attr_setup, *attr_fp;
+        pjmedia_sdp_attr *attr_fp;
         pj_uint32_t rem_proto = 0;
 
@@ -1095 +1095 @@
         }
 
-        /* Check for a=setup in remote SDP. */
-        attr_setup = pjmedia_sdp_media_find_attr(m_rem, &ID_SETUP, NULL);
-        if (!attr_setup)
-            attr_setup = pjmedia_sdp_attr_find(sdp_remote->attr_count,
-                                      sdp_remote->attr, &ID_SETUP, NULL);
+        /* Check for a=fingerprint in remote SDP. */
         switch (ds->srtp->setting.use) {
             case PJMEDIA_SRTP_DISABLED:
-                if (attr_setup) {
+                if (attr_fp) {
                     status = PJMEDIA_SRTP_ESDPINTRANSPORT;
                     goto on_return;
@@ -1110 +1106 @@
                 break;
             case PJMEDIA_SRTP_MANDATORY:
-                if (!attr_setup) {
+                if (!attr_fp) {
+                    /* Should never reach here, this is already checked */
                     status = PJMEDIA_SRTP_ESDPINTRANSPORT;
                     goto on_return;

pjproject/trunk/pjmedia/src/pjmedia/transport_srtp_sdes.c

@@ -79 +79 @@
     pj_ansi_strncpy(sdes->name, srtp->pool->obj_name, PJ_MAX_OBJ_NAME);
     pj_memcpy(sdes->name, "sdes", 4);
-    sdes->type = PJMEDIA_TRANSPORT_TYPE_SRTP;
+    sdes->type = PJMEDIA_SRTP_KEYING_SDES;
     sdes->op = &sdes_op;
     sdes->user_data = srtp;