From 12ac8846de488356c8f649f825b505d34744f09d Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Thu, 20 Aug 2020 13:35:41 -0400 Subject: [PATCH] TT#90101 don't blindly accept a protocol switch in an answer An endpoint switching protocols is normally passed through to the peer, but in an answer that is usually not desirable. Change the default behaviour to stick to the original protocol that was offered even if the answerer changes protocols. Change-Id: Ib288549f4b1c9ab57a6333c6b7dd511537af96f7 --- README.md | 8 ++ daemon/call.c | 28 ++++- daemon/call_interfaces.c | 9 +- include/call_interfaces.h | 2 +- t/auto-daemon-tests.pl | 232 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 270 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index f958583e8..9c5b37cd9 100644 --- a/README.md +++ b/README.md @@ -845,6 +845,14 @@ Optionally included keys are: Valid values are: `RTP/AVP`, `RTP/AVPF`, `RTP/SAVP`, `RTP/SAVPF`. + Additionally the string `accept` can be given in `answer` messages to allow a special case: By + default (when no `transport-protocol` override is given) in answer messages, *rtpengine* will + use the transport protocol that was originally offered. However, an answering client may answer + with a different protocol than what was offered (e.g. offer was for `RTP/AVP` and answer comes + with `RTP/AVPF`). The default behaviour for *rtpengine* is to ignore this protocol change and + still proceed with the protocol that was originally offered. Using the `accept` option here + tells *rtpengine* to go along with this protocol change and pass it to the original offerer. + * `media address` This can be used to override both the addresses present in the SDP body diff --git a/daemon/call.c b/daemon/call.c index b42fe5d49..52a16405e 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -1909,14 +1909,30 @@ static void __update_media_protocol(struct call_media *media, struct call_media /* deduct protocol from stream parameters received */ if (other_media->protocol != sp->protocol) { other_media->protocol = sp->protocol; - /* if the endpoint changes the protocol, we reset the other side's + /* If the endpoint changes the protocol, we reset the other side's * protocol as well. this lets us remember our previous overrides, * but also lets endpoints re-negotiate. - * Exception: OSRTP answer/accept. */ - if (flags && flags->opmode == OP_ANSWER && other_media->protocol && other_media->protocol->rtp - && !other_media->protocol->srtp - && media->protocol && media->protocol->osrtp && flags->osrtp_accept) - ; + * Answers are a special case: handle OSRTP answer/accept, but otherwise + * answer with the same protocol that was offered, unless we're instructed + * not to. */ + if (flags && flags->opmode == OP_ANSWER) { + // OSRTP? + if (other_media->protocol && other_media->protocol->rtp + && !other_media->protocol->srtp + && media->protocol && media->protocol->osrtp) + { + // accept it? + if (flags->osrtp_accept) + ; + else + media->protocol = NULL; // reject + } + // pass through any other protocol change? + else if (!flags->protocol_accept) + ; + else + media->protocol = NULL; + } else media->protocol = NULL; } diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index 17bcd1ce1..242b4ee0b 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -963,8 +963,13 @@ static void call_ng_process_flags(struct sdp_ng_flags *out, bencode_item_t *inpu call_ng_flags_list(out, input, "T.38", ng_t38_option, NULL); #endif - bencode_get_alt(input, "transport-protocol", "transport protocol", &out->transport_protocol_str); - out->transport_protocol = transport_protocol(&out->transport_protocol_str); + str transport_protocol_str; + bencode_get_alt(input, "transport-protocol", "transport protocol", &transport_protocol_str); + if (!str_cmp(&transport_protocol_str, "accept")) + out->protocol_accept = 1; + else + out->transport_protocol = transport_protocol(&transport_protocol_str); + bencode_get_alt(input, "media-address", "media address", &out->media_address); if (bencode_get_alt(input, "address-family", "address family", &out->address_family_str)) out->address_family = get_socket_family_rfc(&out->address_family_str); diff --git a/include/call_interfaces.h b/include/call_interfaces.h index 5507a2e4d..dc8161679 100644 --- a/include/call_interfaces.h +++ b/include/call_interfaces.h @@ -25,7 +25,6 @@ struct sdp_ng_flags { str received_from_family; str received_from_address; str media_address; - str transport_protocol_str; str address_family_str; const struct transport_protocol *transport_protocol; sockaddr_t parsed_received_from; @@ -47,6 +46,7 @@ struct sdp_ng_flags { rev_ptime; GHashTable *sdes_no; int asymmetric:1, + protocol_accept:1, no_redis_update:1, unidirectional:1, trust_address:1, diff --git a/t/auto-daemon-tests.pl b/t/auto-daemon-tests.pl index a13941293..a8cbaed19 100755 --- a/t/auto-daemon-tests.pl +++ b/t/auto-daemon-tests.pl @@ -36,6 +36,238 @@ my ($sock_a, $sock_b, $sock_c, $sock_d, $port_a, $port_b, $ssrc, $resp, +# stray answer protocol changes + +new_call; + +offer('stray answer protocol changes, default', { + ICE => 'remove', + flags => [], + 'transport-protocol' => 'RTP/SAVP', + DTLS => 'off', + }, < 'remove', + flags => [], + DTLS => 'off', + }, < 'remove', + flags => [], + 'transport-protocol' => 'RTP/SAVP', + DTLS => 'off', + }, < 'remove', + flags => [], + DTLS => 'off', + 'transport-protocol' => 'accept', + }, < 'remove', + flags => [], + 'transport-protocol' => 'RTP/SAVP', + DTLS => 'off', + }, < 'remove', + flags => [], + DTLS => 'off', + 'transport-protocol' => 'RTP/AVPF', + }, <