From 89175c856411dbe84ae5c9be0cbcffe95e946284 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Fri, 15 May 2020 11:01:40 -0400 Subject: [PATCH] TT#81850 fix SRTP re-offer and RTP to SRTP switch 1) In an SRTP re-invite offer, fixes not a full set of crypto suites being present 2) In a re-invite offer that switches from RTP to SRTP, fixes SRTP not being initialised at all Change-Id: I911442d2cba17ecf6af482cfe922d4e9db2eda8d --- daemon/call.c | 108 +++++++++++------- t/auto-daemon-tests.pl | 251 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 317 insertions(+), 42 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index f40a0e77a..9a2071a7f 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -1317,7 +1317,7 @@ static void __generate_crypto(const struct sdp_ng_flags *flags, struct call_medi MEDIA_CLEAR(this, SETUP_PASSIVE); } - if (!MEDIA_ISSET(this, INITIALIZED)) { + if (!MEDIA_ARESET2(this, DTLS, SDES) && flags->opmode == OP_OFFER) { /* we offer both DTLS and SDES by default */ /* unless this is overridden by flags */ if (!flags->dtls_off) @@ -1344,60 +1344,84 @@ static void __generate_crypto(const struct sdp_ng_flags *flags, struct call_medi /* SDES parameters below */ if (flags->opmode == OP_OFFER) { - if (!cpq->head) { - // generate a new set of params - // if we were offered some crypto suites, copy those first into our offer - unsigned int c_tag = 1; // tag for next crypto suite generated by us - unsigned long types_offered = 0; + // generate full set of params + // re-create the entire list - steal for later flushing + GQueue cpq_orig = *cpq; + g_queue_init(cpq); - // make sure our bit field is large enough - assert(num_crypto_suites <= sizeof(types_offered) * 8); + // if we were offered some crypto suites, copy those first into our offer + unsigned int c_tag = 1; // tag for next crypto suite generated by us + unsigned long types_offered = 0; - for (GList *l = offered_cpq->head; l; l = l->next) { - struct crypto_params_sdes *offered_cps = l->data; + // make sure our bit field is large enough + assert(num_crypto_suites <= sizeof(types_offered) * 8); - struct crypto_params_sdes *cps = g_slice_alloc0(sizeof(*cps)); - g_queue_push_tail(cpq, cps); + for (GList *l = offered_cpq->head; l; l = l->next) { + struct crypto_params_sdes *offered_cps = l->data; - cps->tag = offered_cps->tag; - // our own offered tags will be higher than the ones we received - if (cps->tag >= c_tag) - c_tag = cps->tag + 1; - crypto_params_copy(&cps->params, &offered_cps->params, 1); + struct crypto_params_sdes *cps = g_slice_alloc0(sizeof(*cps)); + g_queue_push_tail(cpq, cps); + + cps->tag = offered_cps->tag; + // our own offered tags will be higher than the ones we received + if (cps->tag >= c_tag) + c_tag = cps->tag + 1; + crypto_params_copy(&cps->params, &offered_cps->params, 1); - // we use a bit field to keep track of which types we've seen here - types_offered |= 1 << cps->params.crypto_suite->idx; + // we use a bit field to keep track of which types we've seen here + types_offered |= 1 << cps->params.crypto_suite->idx; + + __sdes_flags(cps, flags); + } - __sdes_flags(cps, flags); + // if we had any suites added before, re-add those that aren't there yet + struct crypto_params_sdes *cps_orig; + while ((cps_orig = g_queue_pop_head(&cpq_orig))) { + if ((types_offered & (1 << cps_orig->params.crypto_suite->idx))) { + crypto_params_sdes_free(cps_orig); + continue; } - // generate crypto suite offers for any types that we haven't seen above - for (unsigned int i = 0; i < num_crypto_suites; i++) { - if ((types_offered & (1 << i))) - continue; + // make sure our tag is higher than what we've seen + if (cps_orig->tag < c_tag) + cps_orig->tag = c_tag; + if (cps_orig->tag >= c_tag) + c_tag = cps_orig->tag + 1; - if (flags->sdes_no && g_hash_table_lookup(flags->sdes_no, - &crypto_suites[i].name_str)) - { - ilog(LOG_DEBUG, "Not offering crypto suite '%s' " - "due to 'SDES-no' option", - crypto_suites[i].name); - continue; - } + g_queue_push_tail(cpq, cps_orig); - struct crypto_params_sdes *cps = g_slice_alloc0(sizeof(*cps)); - g_queue_push_tail(cpq, cps); + types_offered |= 1 << cps_orig->params.crypto_suite->idx; + } - cps->tag = c_tag++; - cps->params.crypto_suite = &crypto_suites[i]; - random_string((unsigned char *) cps->params.master_key, - cps->params.crypto_suite->master_key_len); - random_string((unsigned char *) cps->params.master_salt, - cps->params.crypto_suite->master_salt_len); - /* mki = mki_len = 0 */ + // generate crypto suite offers for any types that we haven't seen above + // XXX for re-invites, this always creates new crypto keys for suites + // that weren't accepted before, instead of re-using the same keys (and + // suites) that were previously offered but not accepted + for (unsigned int i = 0; i < num_crypto_suites; i++) { + if ((types_offered & (1 << i))) + continue; - __sdes_flags(cps, flags); + if (flags->sdes_no && g_hash_table_lookup(flags->sdes_no, + &crypto_suites[i].name_str)) + { + ilog(LOG_DEBUG, "Not offering crypto suite '%s' " + "due to 'SDES-no' option", + crypto_suites[i].name); + continue; } + + struct crypto_params_sdes *cps = g_slice_alloc0(sizeof(*cps)); + g_queue_push_tail(cpq, cps); + + cps->tag = c_tag++; + cps->params.crypto_suite = &crypto_suites[i]; + random_string((unsigned char *) cps->params.master_key, + cps->params.crypto_suite->master_key_len); + random_string((unsigned char *) cps->params.master_salt, + cps->params.crypto_suite->master_salt_len); + /* mki = mki_len = 0 */ + + __sdes_flags(cps, flags); } } else { // OP_ANSWER diff --git a/t/auto-daemon-tests.pl b/t/auto-daemon-tests.pl index 4b570971f..4cfc761ab 100755 --- a/t/auto-daemon-tests.pl +++ b/t/auto-daemon-tests.pl @@ -35,6 +35,257 @@ my ($sock_a, $sock_b, $port_a, $port_b, $ssrc, $resp, +# RTP to SRTP switch (and SRTP re-invite) TT#81850 + +new_call; + +(undef, undef, $srtp_key_a) = offer('RTP to SRTP switch (and SRTP re-invite)', + { "transport-protocol" => "RTP/SAVP", "ICE" => "remove", "rtcp-mux" => [ "demux" ], + DTLS => 'off', + "replace" => [ "origin", "session-connection" ], + "via-branch" => "z9hG4bK0ae8.cc3c994fa8d0c0f1f2536bba541306fb.0", + }, < "remove", "rtcp-mux" => [ "demux" ], + DTLS => 'off', + "replace" => [ "origin", "session-connection" ], + "via-branch" => "z9hG4bK0ae8.cc3c994fa8d0c0f1f2536bba541306fb.0", + }, < "RTP/SAVP", "ICE" => "remove", "rtcp-mux" => [ "demux" ], + DTLS => 'off', + "replace" => [ "origin", "session-connection" ], + "via-branch" => "z9hG4bK0ae8.cc3c994fa8d0c0f1f2536bba541306fb.0", + 'to-tag' => tt(), + }, < "remove", "rtcp-mux" => [ "demux" ], + DTLS => 'off', + "replace" => [ "origin", "session-connection" ], + "via-branch" => "z9hG4bK0ae8.cc3c994fa8d0c0f1f2536bba541306fb.0", + }, < "RTP/SAVP", "ICE" => "remove", "rtcp-mux" => [ "demux" ], + DTLS => 'off', + "replace" => [ "origin", "session-connection" ], + "via-branch" => "z9hG4bK0ae8.cc3c994fa8d0c0f1f2536bba541306fb.0", + 'to-tag' => tt(), + }, <