res_rtp_asterisk: Use separate SRTP session for RTCP with DTLS

Asterisk uses separate UDP ports for RTP and RTCP traffic and RFC 5764
explicitly states:

  There MUST be a separate DTLS-SRTP session for each distinct pair of
  source and destination ports used by a media session

This means RTP keying material cannot be used for DTLS RTCP, which was
the reason why RTCP encryption would fail.

ASTERISK-25642

Change-Id: I7e8779d8b63e371088081bb113131361b2847e3a

diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h
index c79554bf0c634c69909f46c5d7e27a4d7f30be0f..20690901041fcbd59ce2668550a3cc18fdbe1db6 100644 (file)
--- a/include/asterisk/rtp_engine.h
+++ b/include/asterisk/rtp_engine.h
@@ -2191,20 +2191,22 @@ int ast_rtp_instance_sendcng(struct ast_rtp_instance *instance, int level);
  * \param instance the RTP instance
  * \param remote_policy the remote endpoint's policy
  * \param local_policy our policy for this RTP instance's remote endpoint
+ * \param rtcp 1 for dedicated RTCP policies
  *
  * \retval 0 Success
  * \retval non-zero Failure
  */
-int ast_rtp_instance_add_srtp_policy(struct ast_rtp_instance *instance, struct ast_srtp_policy* remote_policy, struct ast_srtp_policy *local_policy);
+int ast_rtp_instance_add_srtp_policy(struct ast_rtp_instance *instance, struct ast_srtp_policy* remote_policy, struct ast_srtp_policy *local_policy, int rtcp);
 
 /*!
  * \brief Obtain the SRTP instance associated with an RTP instance
  *
  * \param instance the RTP instance
+ * \param rtcp 1 to request instance for RTCP
  * \retval the SRTP instance on success
  * \retval NULL if no SRTP instance exists
  */
-struct ast_srtp *ast_rtp_instance_get_srtp(struct ast_rtp_instance *instance);
+struct ast_srtp *ast_rtp_instance_get_srtp(struct ast_rtp_instance *instance, int rtcp);
 
 /*! \brief Custom formats declared in codecs.conf at startup must be communicated to the rtp_engine
  * so their mime type can payload number can be initialized. */

diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 6ff8f5286f5a8c750d3bf793f8ba11f78cae28ce..462d4c530ae54ab07e31be8ae8567f4ddc0c13f4 100644 (file)
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -188,6 +188,8 @@ struct ast_rtp_instance {
        struct ast_rtp_glue *glue;
        /*! SRTP info associated with the instance */
        struct ast_srtp *srtp;
+       /*! SRTP info dedicated for RTCP associated with the instance */
+       struct ast_srtp *rtcp_srtp;
        /*! Channel unique ID */
        char channel_uniqueid[AST_MAX_UNIQUEID];
        /*! Time of last packet sent */
@@ -364,6 +366,10 @@ static void instance_destructor(void *obj)
                res_srtp->destroy(instance->srtp);
        }
 
+       if (instance->rtcp_srtp) {
+               res_srtp->destroy(instance->rtcp_srtp);
+       }
+
        ast_rtp_codecs_payloads_destroy(&instance->codecs);
 
        /* Drop our engine reference */
@@ -1568,29 +1574,38 @@ int ast_rtp_engine_srtp_is_registered(void)
        return res_srtp && res_srtp_policy;
 }
 
-int ast_rtp_instance_add_srtp_policy(struct ast_rtp_instance *instance, struct ast_srtp_policy *remote_policy, struct ast_srtp_policy *local_policy)
+int ast_rtp_instance_add_srtp_policy(struct ast_rtp_instance *instance, struct ast_srtp_policy *remote_policy, struct ast_srtp_policy *local_policy, int rtcp)
 {
        int res = 0;
+       struct ast_srtp **srtp;
 
        if (!res_srtp) {
                return -1;
        }
 
-       if (!instance->srtp) {
-               res = res_srtp->create(&instance->srtp, instance, remote_policy);
+
+       srtp = rtcp ? &instance->rtcp_srtp : &instance->srtp;
+
+       if (!*srtp) {
+               res = res_srtp->create(srtp, instance, remote_policy);
        } else {
-               res = res_srtp->replace(&instance->srtp, instance, remote_policy);
+               res = res_srtp->replace(srtp, instance, remote_policy);
        }
        if (!res) {
-               res = res_srtp->add_stream(instance->srtp, local_policy);
+               res = res_srtp->add_stream(*srtp, local_policy);
        }
 
        return res;
 }
 
-struct ast_srtp *ast_rtp_instance_get_srtp(struct ast_rtp_instance *instance)
+struct ast_srtp *ast_rtp_instance_get_srtp(struct ast_rtp_instance *instance, int rtcp)
 {
-       return instance->srtp;
+       if (rtcp && instance->rtcp_srtp) {
+               return instance->rtcp_srtp;
+       }
+       else {
+               return instance->srtp;
+       }
 }
 
 int ast_rtp_instance_sendcng(struct ast_rtp_instance *instance, int level)

diff --git a/main/sdp_srtp.c b/main/sdp_srtp.c
index 2c49fd240dde934d90555544642b37f726517a79..d77d4630d3e3d5f3338e118e6376adcc964cc2ac 100644 (file)
--- a/main/sdp_srtp.c
+++ b/main/sdp_srtp.c
@@ -183,7 +183,7 @@ static int crypto_activate(struct ast_sdp_crypto *p, int suite_val, unsigned cha
        }
 
        /* Add the SRTP policies */
-       if (ast_rtp_instance_add_srtp_policy(rtp, remote_policy, local_policy)) {
+       if (ast_rtp_instance_add_srtp_policy(rtp, remote_policy, local_policy, 0)) {
                ast_log(LOG_WARNING, "Could not set SRTP policies\n");
                goto err;
        }

diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 611920e807ce1937c94a9c1512bc420b772a7099..b22e9cb7fdee8503a74ed4b27fbe9da62df4de3d 100644 (file)
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -1982,19 +1982,20 @@ static int dtls_srtp_renegotiate(const void *data)
        return 0;
 }
 
-static int dtls_srtp_setup(struct ast_rtp *rtp, struct ast_srtp *srtp, struct ast_rtp_instance *instance)
+static int dtls_srtp_setup(struct ast_rtp *rtp, struct ast_srtp *srtp, struct ast_rtp_instance *instance, int rtcp)
 {
        unsigned char material[SRTP_MASTER_LEN * 2];
        unsigned char *local_key, *local_salt, *remote_key, *remote_salt;
        struct ast_srtp_policy *local_policy, *remote_policy = NULL;
        struct ast_rtp_instance_stats stats = { 0, };
        int res = -1;
+       struct dtls_details *dtls = !rtcp ? &rtp->dtls : &rtp->rtcp->dtls;
 
        /* If a fingerprint is present in the SDP make sure that the peer certificate matches it */
        if (rtp->dtls_verify & AST_RTP_DTLS_VERIFY_FINGERPRINT) {
                X509 *certificate;
 
-               if (!(certificate = SSL_get_peer_certificate(rtp->dtls.ssl))) {
+               if (!(certificate = SSL_get_peer_certificate(dtls->ssl))) {
                        ast_log(LOG_WARNING, "No certificate was provided by the peer on RTP instance '%p'\n", instance);
                        return -1;
                }
@@ -2028,14 +2029,14 @@ static int dtls_srtp_setup(struct ast_rtp *rtp, struct ast_srtp *srtp, struct as
        }
 
        /* Ensure that certificate verification was successful */
-       if ((rtp->dtls_verify & AST_RTP_DTLS_VERIFY_CERTIFICATE) && SSL_get_verify_result(rtp->dtls.ssl) != X509_V_OK) {
+       if ((rtp->dtls_verify & AST_RTP_DTLS_VERIFY_CERTIFICATE) && SSL_get_verify_result(dtls->ssl) != X509_V_OK) {
                ast_log(LOG_WARNING, "Peer certificate on RTP instance '%p' failed verification test\n",
                        instance);
                return -1;
        }
 
        /* Produce key information and set up SRTP */
-       if (!SSL_export_keying_material(rtp->dtls.ssl, material, SRTP_MASTER_LEN * 2, "EXTRACTOR-dtls_srtp", 19, NULL, 0, 0)) {
+       if (!SSL_export_keying_material(dtls->ssl, material, SRTP_MASTER_LEN * 2, "EXTRACTOR-dtls_srtp", 19, NULL, 0, 0)) {
                ast_log(LOG_WARNING, "Unable to extract SRTP keying material from DTLS-SRTP negotiation on RTP instance '%p'\n",
                        instance);
                return -1;
@@ -2090,7 +2091,7 @@ static int dtls_srtp_setup(struct ast_rtp *rtp, struct ast_srtp *srtp, struct as
 
        res_srtp_policy->set_ssrc(remote_policy, 0, 1);
 
-       if (ast_rtp_instance_add_srtp_policy(instance, remote_policy, local_policy)) {
+       if (ast_rtp_instance_add_srtp_policy(instance, remote_policy, local_policy, rtcp)) {
                ast_log(LOG_WARNING, "Could not set policies when setting up DTLS-SRTP on '%p'\n", rtp);
                goto error;
        }
@@ -2121,7 +2122,7 @@ static int __rtp_recvfrom(struct ast_rtp_instance *instance, void *buf, size_t s
 {
        int len;
        struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
-       struct ast_srtp *srtp = ast_rtp_instance_get_srtp(instance);
+       struct ast_srtp *srtp = ast_rtp_instance_get_srtp(instance, rtcp);
        char *in = buf;
 #ifdef HAVE_PJPROJECT
        struct ast_sockaddr *loop = rtcp ? &rtp->rtcp_loop : &rtp->rtp_loop;
@@ -2178,10 +2179,8 @@ static int __rtp_recvfrom(struct ast_rtp_instance *instance, void *buf, size_t s
                if (SSL_is_init_finished(dtls->ssl)) {
                        /* Any further connections will be existing since this is now established */
                        dtls->connection = AST_RTP_DTLS_CONNECTION_EXISTING;
-                       if (!rtcp) {
-                               /* Use the keying material to set up key/salt information */
-                               res = dtls_srtp_setup(rtp, srtp, instance);
-                       }
+                       /* Use the keying material to set up key/salt information */
+                       res = dtls_srtp_setup(rtp, srtp, instance, rtcp);
                } else {
                        /* Since we've sent additional traffic start the timeout timer for retransmission */
                        dtls_srtp_start_timeout_timer(instance, rtp, rtcp);
@@ -2250,7 +2249,7 @@ static int __rtp_sendto(struct ast_rtp_instance *instance, void *buf, size_t siz
        int len = size;
        void *temp = buf;
        struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
-       struct ast_srtp *srtp = ast_rtp_instance_get_srtp(instance);
+       struct ast_srtp *srtp = ast_rtp_instance_get_srtp(instance, rtcp);
        int res;
 
        *ice = 0;
@@ -2950,7 +2949,8 @@ static void ast_rtp_update_source(struct ast_rtp_instance *instance)
 static void ast_rtp_change_source(struct ast_rtp_instance *instance)
 {
        struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
-       struct ast_srtp *srtp = ast_rtp_instance_get_srtp(instance);
+       struct ast_srtp *srtp = ast_rtp_instance_get_srtp(instance, 0);
+       struct ast_srtp *rtcp_srtp = ast_rtp_instance_get_srtp(instance, 1);
        unsigned int ssrc = ast_random();
 
        if (!rtp->lastts) {
@@ -2966,6 +2966,9 @@ static void ast_rtp_change_source(struct ast_rtp_instance *instance)
        if (srtp) {
                ast_debug(3, "Changing ssrc for SRTP from %u to %u\n", rtp->ssrc, ssrc);
                res_srtp->change_source(srtp, rtp->ssrc, ssrc);
+               if (rtcp_srtp != srtp) {
+                       res_srtp->change_source(srtp, rtp->ssrc, ssrc);
+               }
        }
 
        rtp->ssrc = ssrc;