From 0cfab30b284286b74a4c58b040364b246a9680d7 Mon Sep 17 00:00:00 2001 From: Jacek Konieczny Date: Fri, 25 Mar 2016 16:59:05 +0100 Subject: 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 --- include/asterisk/rtp_engine.h | 6 ++++-- main/rtp_engine.c | 29 ++++++++++++++++++++++------- main/sdp_srtp.c | 2 +- res/res_rtp_asterisk.c | 27 +++++++++++++++------------ 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h index c79554bf0..206909010 100644 --- 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 6ff8f5286..462d4c530 100644 --- 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 2c49fd240..d77d4630d 100644 --- 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 611920e80..b22e9cb7f 100644 --- 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; -- cgit v1.2.3