Allow SRTP policies to be reloaded

Currently, when using res_srtp, once the SRTP policy has been added to the
current session the policy is locked into place.  Any attempt to replace an
existing policy, which would be needed if the remote endpoint negotiated a new
cryptographic key, is instead rejected in res_srtp.  This happens in particular
in transfer scenarios, where the endpoint that Asterisk is communicating with
changes but uses the same RTP session.

This patch modifies res_srtp to allow remote and local policies to be reloaded
in the underlying SRTP library.  From the perspective of users of the SRTP API,
the only change is that the adding of remote and local policies are now added
in a single method call, whereas they previously were added separately.  This
was changed to account for the differences in handling remote and local
policies in libsrtp.

Review: https://reviewboard.asterisk.org/r/1741/

(closes issue ASTERISK-19253)
Reported by: Thomas Arimont
Tested by: Thomas Arimont
Patches:
  srtp_renew_keys_2012_02_22.diff uploaded by Matt Jordan (license 6283)
  (with some small modifications for this check-in)
........

Merged revisions 356604 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 356605 from http://svn.asterisk.org/svn/asterisk/branches/10

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@356606 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/channels/sip/sdp_crypto.c b/channels/sip/sdp_crypto.c
index bf3b2cd..85adf83 100644 (file)
--- a/channels/sip/sdp_crypto.c
+++ b/channels/sip/sdp_crypto.c
@@ -162,15 +162,9 @@ static int sdp_crypto_activate(struct sdp_crypto *p, int suite_val, unsigned cha
                goto err;
        }
 
-       /* FIXME MIKMA */
-       /* ^^^ I wish I knew what needed fixing... */
-       if (ast_rtp_instance_add_srtp_policy(rtp, local_policy)) {
-               ast_log(LOG_WARNING, "Could not set local SRTP policy\n");
-               goto err;
-       }
-
-       if (ast_rtp_instance_add_srtp_policy(rtp, remote_policy)) {
-               ast_log(LOG_WARNING, "Could not set remote SRTP policy\n");
+       /* Add the SRTP policies */
+       if (ast_rtp_instance_add_srtp_policy(rtp, remote_policy, local_policy)) {
+               ast_log(LOG_WARNING, "Could not set SRTP policies\n");
                goto err;
        }
 
@@ -279,10 +273,8 @@ int sdp_crypto_process(struct sdp_crypto *p, const char *attr, struct ast_rtp_in
                        ast_log(LOG_ERROR, "Could not allocate memory for a_crypto\n");
                        return -1;
                }
-
                snprintf(p->a_crypto, attr_len + 10, "a=crypto:%s %s inline:%s\r\n", tag, suite, p->local_key64);
        }
-
        return 0;
 }
 

diff --git a/include/asterisk/res_srtp.h b/include/asterisk/res_srtp.h
index 4aa8309..c7fdc40 100644 (file)
--- a/include/asterisk/res_srtp.h
+++ b/include/asterisk/res_srtp.h
@@ -30,13 +30,23 @@ struct ast_srtp_cb {
 };
 
 struct ast_srtp_res {
+       /*! Create a new SRTP session for an RTP instance with a default policy */
        int (*create)(struct ast_srtp **srtp, struct ast_rtp_instance *rtp, struct ast_srtp_policy *policy);
+       /* Replace an existing SRTP session with a new session, along with a new default policy */
+       int (*replace)(struct ast_srtp **srtp, struct ast_rtp_instance *rtp, struct ast_srtp_policy *policy);
+       /*! Destroy an SRTP session, along with all associated policies */
        void (*destroy)(struct ast_srtp *srtp);
+       /* Add a new stream to an existing SRTP session.  Note that the policy cannot be for a wildcard SSRC */
        int (*add_stream)(struct ast_srtp *srtp, struct ast_srtp_policy *policy);
+       /* Change the source on an existing SRTP session. */
        int (*change_source)(struct ast_srtp *srtp, unsigned int from_ssrc, unsigned int to_ssrc);
+       /* Set a callback function */
        void (*set_cb)(struct ast_srtp *srtp, const struct ast_srtp_cb *cb, void *data);
+       /* Unprotect SRTP data */
        int (*unprotect)(struct ast_srtp *srtp, void *buf, int *size, int rtcp);
+       /* Protect RTP data */
        int (*protect)(struct ast_srtp *srtp, void **buf, int *size, int rtcp);
+       /* Obtain a random cryptographic key */
        int (*get_random)(unsigned char *key, size_t len);
 };
 

diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h
index 7e27f5a..54fccb1 100644 (file)
--- a/include/asterisk/rtp_engine.h
+++ b/include/asterisk/rtp_engine.h
@@ -1861,7 +1861,25 @@ struct ast_channel *ast_rtp_instance_get_chan(struct ast_rtp_instance *instance)
  */
 int ast_rtp_instance_sendcng(struct ast_rtp_instance *instance, int level);
 
-int ast_rtp_instance_add_srtp_policy(struct ast_rtp_instance *instance, struct ast_srtp_policy *policy);
+/*!
+ * \brief Add or replace the SRTP policies for the given RTP instance
+ *
+ * \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
+ *
+ * \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);
+
+/*!
+ * \brief Obtain the SRTP instance associated with an RTP instance
+ *
+ * \param instance the RTP instance
+ * \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);
 
 /*! \brief Custom formats declared in codecs.conf at startup must be communicated to the rtp_engine

diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 80f154f..d60790c 100644 (file)
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -1856,17 +1856,24 @@ 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 *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 res = 0;
+
        if (!res_srtp) {
                return -1;
        }
 
        if (!instance->srtp) {
-               return res_srtp->create(&instance->srtp, instance, policy);
+               res = res_srtp->create(&instance->srtp, instance, remote_policy);
        } else {
-               return res_srtp->add_stream(instance->srtp, policy);
+               res = res_srtp->replace(&instance->srtp, instance, remote_policy);
+       }
+       if (!res) {
+               res = res_srtp->add_stream(instance->srtp, local_policy);
        }
+
+       return res;
 }
 
 struct ast_srtp *ast_rtp_instance_get_srtp(struct ast_rtp_instance *instance)

diff --git a/res/res_srtp.c b/res/res_srtp.c
index a22a020..9f9bf75 100644 (file)
--- a/res/res_srtp.c
+++ b/res/res_srtp.c
@@ -63,10 +63,12 @@ struct ast_srtp_policy {
        srtp_policy_t sp;
 };
 
+/*! Tracks whether or not we've initialized the libsrtp library */
 static int g_initialized = 0;
 
 /* SRTP functions */
 static int ast_srtp_create(struct ast_srtp **srtp, struct ast_rtp_instance *rtp, struct ast_srtp_policy *policy);
+static int ast_srtp_replace(struct ast_srtp **srtp, struct ast_rtp_instance *rtp, struct ast_srtp_policy *policy);
 static void ast_srtp_destroy(struct ast_srtp *srtp);
 static int ast_srtp_add_stream(struct ast_srtp *srtp, struct ast_srtp_policy *policy);
 static int ast_srtp_change_source(struct ast_srtp *srtp, unsigned int from_ssrc, unsigned int to_ssrc);
@@ -85,6 +87,7 @@ static void ast_srtp_policy_set_ssrc(struct ast_srtp_policy *policy, unsigned lo
 
 static struct ast_srtp_res srtp_res = {
        .create = ast_srtp_create,
+       .replace = ast_srtp_replace,
        .destroy = ast_srtp_destroy,
        .add_stream = ast_srtp_add_stream,
        .change_source = ast_srtp_change_source,
@@ -343,35 +346,33 @@ tryagain:
        }
 
        if (retry == 0  && res == err_status_replay_old) {
-               ast_log(LOG_WARNING, "SRTP unprotect: %s\n", srtp_errstr(res));
+               ast_log(AST_LOG_NOTICE, "SRTP unprotect failed with %s, retrying\n", srtp_errstr(res));
 
                if (srtp->session) {
                        struct ast_srtp_policy *policy;
                        struct ao2_iterator it;
                        int policies_count;
 
-                       // dealloc first
-                       ast_log(LOG_WARNING, "SRTP destroy before re-create\n");
+                       /* dealloc first */
+                       ast_debug(5, "SRTP destroy before re-create\n");
                        srtp_dealloc(srtp->session);
 
-                       // get the count
+                       /* get the count */
                        policies_count = ao2_container_count(srtp->policies);
 
-                       // get the first to build up
+                       /* get the first to build up */
                        it = ao2_iterator_init(srtp->policies, 0);
                        policy = ao2_iterator_next(&it);
 
-                       ast_log(LOG_WARNING, "SRTP try to re-create\n");
+                       ast_debug(5, "SRTP try to re-create\n");
                        if (policy) {
                                if (srtp_create(&srtp->session, &policy->sp) == err_status_ok) {
-                                       ast_log(LOG_WARNING, "SRTP re-created with first policy\n");
-
-                                       // unref first element
+                                       ast_debug(5, "SRTP re-created with first policy\n");
                                        ao2_t_ref(policy, -1, "Unreffing first policy for re-creating srtp session");
 
-                                       // if we have more than one policy, add them afterwards
+                                       /* if we have more than one policy, add them */
                                        if (policies_count > 1) {
-                                               ast_log(LOG_WARNING, "Add all the other %d policies\n",
+                                               ast_debug(5, "Add all the other %d policies\n",
                                                        policies_count - 1);
                                                while ((policy = ao2_iterator_next(&it))) {
                                                        srtp_add_stream(srtp->session, &policy->sp);
@@ -391,7 +392,7 @@ tryagain:
 
        if (res != err_status_ok && res != err_status_replay_fail ) {
                if ((srtp->warned >= 10) && !((srtp->warned - 10) % 100)) {
-                       ast_log(LOG_WARNING, "SRTP unprotect: %s %d\n", srtp_errstr(res), srtp->warned);
+                       ast_log(AST_LOG_WARNING, "SRTP unprotect failed with: %s %d\n", srtp_errstr(res), srtp->warned);
                        srtp->warned = 11;
                } else {
                        srtp->warned++;
@@ -446,6 +447,14 @@ static int ast_srtp_create(struct ast_srtp **srtp, struct ast_rtp_instance *rtp,
        return 0;
 }
 
+static int ast_srtp_replace(struct ast_srtp **srtp, struct ast_rtp_instance *rtp, struct ast_srtp_policy *policy)
+{
+       if ((*srtp) != NULL) {
+               ast_srtp_destroy(*srtp);
+       }
+       return ast_srtp_create(srtp, rtp, policy);
+}
+
 static void ast_srtp_destroy(struct ast_srtp *srtp)
 {
        if (srtp->session) {
@@ -463,13 +472,28 @@ static int ast_srtp_add_stream(struct ast_srtp *srtp, struct ast_srtp_policy *po
 {
        struct ast_srtp_policy *match;
 
+       /* For existing streams, replace if its an SSRC stream, or bail if its a wildcard */
        if ((match = find_policy(srtp, &policy->sp, OBJ_POINTER))) {
-               ast_debug(3, "Policy already exists, not re-adding\n");
-               ao2_t_ref(match, -1, "Unreffing already existing policy");
-               return -1;
+               if (policy->sp.ssrc.type != ssrc_specific) {
+                       ast_log(AST_LOG_WARNING, "Cannot replace an existing wildcard policy");
+                       ao2_t_ref(match, -1, "Unreffing already existing policy");
+                       return -1;
+               } else {
+                       if (srtp_remove_stream(srtp->session, match->sp.ssrc.value) != err_status_ok) {
+                               ast_log(AST_LOG_WARNING, "Failed to remove SRTP stream for SSRC %d\n", match->sp.ssrc.value);
+                       }
+                       ao2_t_unlink(srtp->policies, match, "Remove existing match policy");
+                       ao2_t_ref(match, -1, "Unreffing already existing policy");
+               }
        }
 
+       ast_debug(3, "Adding new policy for %s %d\n",
+               policy->sp.ssrc.type == ssrc_specific ? "SSRC" : "type",
+               policy->sp.ssrc.type == ssrc_specific ? policy->sp.ssrc.value : policy->sp.ssrc.type);
        if (srtp_add_stream(srtp->session, &policy->sp) != err_status_ok) {
+               ast_log(AST_LOG_WARNING, "Failed to add SRTP stream for %s %d\n",
+                       policy->sp.ssrc.type == ssrc_specific ? "SSRC" : "type",
+                       policy->sp.ssrc.type == ssrc_specific ? policy->sp.ssrc.value : policy->sp.ssrc.type);
                return -1;
        }
 
@@ -487,7 +511,7 @@ static int ast_srtp_change_source(struct ast_srtp *srtp, unsigned int from_ssrc,
        };
        err_status_t status;
 
-       /* If we find a mach, return and unlink it from the container so we
+       /* If we find a match, return and unlink it from the container so we
         * can change the SSRC (which is part of the hash) and then have
         * ast_srtp_add_stream link it back in if all is well */
        if ((match = find_policy(srtp, &sp, OBJ_POINTER | OBJ_UNLINK))) {
@@ -503,6 +527,16 @@ static int ast_srtp_change_source(struct ast_srtp *srtp, unsigned int from_ssrc,
        return 0;
 }
 
+static void res_srtp_shutdown(void)
+{
+       srtp_install_event_handler(NULL);
+       if (srtp_shutdown() != err_status_ok) {
+               ast_log(AST_LOG_WARNING, "Failed to de-initialize libsrtp\n");
+       }
+       ast_rtp_engine_unregister_srtp();
+       g_initialized = 0;
+}
+
 static int res_srtp_init(void)
 {
        if (g_initialized) {
@@ -510,12 +544,20 @@ static int res_srtp_init(void)
        }
 
        if (srtp_init() != err_status_ok) {
+               ast_log(AST_LOG_WARNING, "Failed to initialize libsrtp\n");
                return -1;
        }
 
        srtp_install_event_handler(srtp_event_cb);
 
-       return ast_rtp_engine_register_srtp(&srtp_res, &policy_res);
+       if (ast_rtp_engine_register_srtp(&srtp_res, &policy_res)) {
+               ast_log(AST_LOG_WARNING, "Failed to register SRTP with rtp engine\n");
+               res_srtp_shutdown();
+               return -1;
+       }
+
+       g_initialized = 1;
+       return 0;
 }
 
 /*
@@ -529,7 +571,7 @@ static int load_module(void)
 
 static int unload_module(void)
 {
-       ast_rtp_engine_unregister_srtp();
+       res_srtp_shutdown();
        return 0;
 }