From 8f1ef404c990c847748377c6cc218298dd9b7310 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Wed, 28 Aug 2019 15:31:18 -0400 Subject: [PATCH] TT#65800 fix SDES negotiation bug If B accepts a crypto suite that was not listed as the first, in order to support SRTP passthrough, we correctly answer to A with only that one crypto suite. But we must also remove all other crypto suites from our list of supported crypto suites internally, because we use the first one to init our crypto contexts. fixes #829 Change-Id: Id07343d7b24648208e3a4b4e0b246949dce0385e (cherry picked from commit 346670014972f68a0c0ce92514ba3c37df643c95) --- daemon/call.c | 15 ++++ t/auto-daemon-tests.pl | 178 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 188 insertions(+), 5 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index c27f763fe..b9aa327b8 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -1373,11 +1373,15 @@ static void __generate_crypto(const struct sdp_ng_flags *flags, struct call_medi struct crypto_params_sdes *cps_in = cpq_in->head ? cpq_in->head->data : NULL; struct crypto_params_sdes *offered_cps = offered_cpq->head ? offered_cpq->head->data : NULL; if (offered_cps) { + ilog(LOG_DEBUG, "Looking for matching crypto suite to offered %u:%s", offered_cps->tag, + offered_cps->params.crypto_suite->name); // check if we can do SRTP<>SRTP passthrough. the crypto suite that was accepted // must have been present in what was offered to us for (GList *l = cpq_in->head; l; l = l->next) { struct crypto_params_sdes *check_cps = l->data; if (check_cps->params.crypto_suite == offered_cps->params.crypto_suite) { + ilog(LOG_DEBUG, "Found matching crypto suite %u:%s", check_cps->tag, + check_cps->params.crypto_suite->name); cps_in = check_cps; break; } @@ -1394,6 +1398,8 @@ static void __generate_crypto(const struct sdp_ng_flags *flags, struct call_medi // SRTP<>SRTP passthrough cps->params.session_params = cps_in->params.session_params; // XXX verify crypto_params_copy(&cps->params, &offered_cps->params, 1); + ilog(LOG_DEBUG, "Copied crypto params from %i:%s for SRTP passthrough", + cps_in->tag, cps_in->params.crypto_suite->name); } else { random_string((unsigned char *) cps->params.master_key, @@ -1402,7 +1408,16 @@ static void __generate_crypto(const struct sdp_ng_flags *flags, struct call_medi cps->params.crypto_suite->master_salt_len); /* mki = mki_len = 0 */ cps->params.session_params = cps_in->params.session_params; + ilog(LOG_DEBUG, "Creating new SRTP crypto params for %i:%s", + cps->tag, cps->params.crypto_suite->name); } + + // flush out crypto suites we ended up not using - leave only one + if (!g_queue_remove(cpq_in, cps_in)) + ilog(LOG_ERR, "BUG: incoming crypto suite not found in queue"); + crypto_params_sdes_queue_clear(cpq_in); + g_queue_push_tail(cpq_in, cps_in); + __sdes_flags(cps, flags); __sdes_flags(cps_in, flags); } diff --git a/t/auto-daemon-tests.pl b/t/auto-daemon-tests.pl index e8e71d276..d9215e2f3 100755 --- a/t/auto-daemon-tests.pl +++ b/t/auto-daemon-tests.pl @@ -112,6 +112,17 @@ sub snd { my ($sock, $dest, $packet) = @_; $sock->send($packet, 0, pack_sockaddr_in($dest, inet_aton('203.0.113.1'))) or die; } +sub srtp_snd { + my ($sock, $dest, $packet, $srtp_ctx) = @_; + if (!$srtp_ctx->{skey}) { + my ($key, $salt) = NGCP::Rtpclient::SRTP::decode_inline_base64($srtp_ctx->{key}, $srtp_ctx->{cs}); + @$srtp_ctx{qw(skey sauth ssalt)} = NGCP::Rtpclient::SRTP::gen_rtp_session_keys($key, $salt); + } + my ($enc, $out_roc) = NGCP::Rtpclient::SRTP::encrypt_rtp(@$srtp_ctx{qw(cs skey ssalt sauth roc)}, + '', 0, 0, 0, $packet); + $srtp_ctx->{roc} = $out_roc; + $sock->send($enc, 0, pack_sockaddr_in($dest, inet_aton('203.0.113.1'))) or die; +} sub rtp { my ($pt, $seq, $ts, $ssrc, $payload) = @_; print("rtp in $pt $seq $ts $ssrc\n"); @@ -178,6 +189,163 @@ sub rtpm { ok $r->{result} eq 'pong', 'ping works, daemon operational'; } + + +my ($sock_a, $sock_b, $port_a, $port_b, $ssrc, $resp, $srtp_ctx_a, $srtp_ctx_b); + + + + +# github issue 829 + +($sock_a, $sock_b) = new_call([qw(198.51.100.1 7316)], [qw(198.51.100.3 7318)]); + +($port_a) = offer('gh829 control', + { ICE => 'remove', replace => ['origin'], flags => ['pad crypto'], DTLS => 'off' }, < 'remove', replace => ['origin'], flags => ['pad crypto'], DTLS => 'off' }, < $NGCP::Rtpclient::SRTP::crypto_suites{AES_CM_128_HMAC_SHA1_80}, + key => 'IDdiM2QzOWYzMjA2YzkwZWIxY2NmOWVhOTc4MjE1', +}; +$srtp_ctx_b = { + cs => $NGCP::Rtpclient::SRTP::crypto_suites{AES_CM_128_HMAC_SHA1_80}, + key => 'Qk0TvVeyfqfjFd/YebnyyklqSEhJntpVKV1KAhHa', +}; + +srtp_snd($sock_a, $port_b, rtp(0, 1000, 3000, 0x1234, "\x00" x 160), $srtp_ctx_a); +srtp_rcv($sock_b, $port_a, rtpm(0, 1000, 3000, 0x1234, "\x00" x 160), $srtp_ctx_a); +srtp_snd($sock_b, $port_a, rtp(0, 2000, 4000, 0x3456, "\x00" x 160), $srtp_ctx_b); +srtp_rcv($sock_a, $port_b, rtpm(0, 2000, 4000, 0x3456, "\x00" x 160), $srtp_ctx_b); + + +($sock_a, $sock_b) = new_call([qw(198.51.100.1 7310)], [qw(198.51.100.3 7312)]); + +($port_a) = offer('gh829', + { ICE => 'remove', replace => ['origin'], flags => ['pad crypto'], DTLS => 'off' }, < 'remove', replace => ['origin'], flags => ['pad crypto'], DTLS => 'off' }, < $NGCP::Rtpclient::SRTP::crypto_suites{AES_CM_128_HMAC_SHA1_80}, + key => 'IDdiM2QzOWYzMjA2YzkwZWIxY2NmOWVhOTc4MjE1', +}; +$srtp_ctx_b = { + cs => $NGCP::Rtpclient::SRTP::crypto_suites{AES_CM_128_HMAC_SHA1_80}, + key => 'Qk0TvVeyfqfjFd/YebnyyklqSEhJntpVKV1KAhHa', +}; + +srtp_snd($sock_a, $port_b, rtp(0, 1000, 3000, 0x1234, "\x00" x 160), $srtp_ctx_a); +srtp_rcv($sock_b, $port_a, rtpm(0, 1000, 3000, 0x1234, "\x00" x 160), $srtp_ctx_a); +srtp_snd($sock_b, $port_a, rtp(0, 2000, 4000, 0x3456, "\x00" x 160), $srtp_ctx_b); +srtp_rcv($sock_a, $port_b, rtpm(0, 2000, 4000, 0x3456, "\x00" x 160), $srtp_ctx_b); + + # SDP in/out tests, various ICE options new_call; @@ -1252,9 +1420,9 @@ SDP # RTP sequencing tests -my ($sock_a, $sock_b) = new_call([qw(198.51.100.1 2010)], [qw(198.51.100.3 2012)]); +($sock_a, $sock_b) = new_call([qw(198.51.100.1 2010)], [qw(198.51.100.3 2012)]); -my ($port_a) = offer('two codecs, no transcoding', { ICE => 'remove', replace => ['origin'] }, < 'remove', replace => ['origin'] }, < 'remove', replace => ['origin'] }, < 'remove', replace => ['origin'] }, < $ft, blob => $wav_file }); +$resp = rtpe_req('play media', 'media playback, offer only', { 'from-tag' => $ft, blob => $wav_file }); is $resp->{duration}, 100, 'media duration'; my ($ts, $seq);