diff --git a/daemon/call.c b/daemon/call.c index 999483316..ea1315b05 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -1426,6 +1426,14 @@ static void __ice_offer(const sdp_ng_flags *flags, struct call_media *this, if (flags->ice_reject) MEDIA_CLEAR(other, ICE); + // An answer must not introduce ICE where the original offer did not + // advertise it. A locally created ICE agent means we intentionally + // offered ICE on this leg, in which case keep the negotiated state. + // In other cases ignore ICE=force on the answer (see issue #2161). + if (flags->opmode == OP_ANSWER && flags->ice_option == ICE_FORCE && + !MEDIA_ISSET(other, ICE) && !this->ice_agent) + MEDIA_CLEAR(this, ICE); + if (flags->passthrough_on) { ilog(LOG_DEBUG, "enabling passthrough mode"); MEDIA_SET(this, PASSTHRU); diff --git a/docs/ng_control_protocol.md b/docs/ng_control_protocol.md index ea100121f..91f607037 100644 --- a/docs/ng_control_protocol.md +++ b/docs/ng_control_protocol.md @@ -439,6 +439,11 @@ Optionally included keys are: had ICE attributes listed. If the incoming SDP did not contain ICE attributes, then no ICE attributes are added. + For `answer` messages, an endpoint must not negotiate ICE that was absent from + the corresponding offer. Prefer `ICE=default` when handling an answer from + a peer which might not support ICE. `ICE=force` on an answer cannot invent + ICE support when no ICE was offered on that leg. + With `force-relay`, existing ICE candidates are left in place except `relay` type candidates, and *rtpengine* inserts itself as a `relay` candidate. It will also leave SDP c= and m= lines unchanged. diff --git a/t/Makefile b/t/Makefile index db85b9593..db600b870 100644 --- a/t/Makefile +++ b/t/Makefile @@ -87,7 +87,7 @@ include ../lib/common.Makefile daemon-tests-templ-def daemon-tests-templ-def-offer daemon-tests-t38 daemon-tests-evs-dtx \ daemon-tests-transform daemon-tests-http daemon-tests-heuristic daemon-tests-asymmetric \ daemon-tests-dtx-no-shift daemon-tests-rtcp daemon-tests-redis-subscribe daemon-tests-rtp-ext daemon-tests-ssrc \ - daemon-tests-bundle daemon-tests-dtls daemon-tests-rollback \ + daemon-tests-bundle daemon-tests-dtls daemon-tests-ice-force-answer daemon-tests-rollback \ daemon-tests-rollback-redis daemon-tests-redis-srtp-index \ daemon-tests-recording daemon-tests-create daemon-tests-created-subscriptions daemon-tests-alias \ daemon-tests-kernel-api \ @@ -140,7 +140,7 @@ daemon-tests: daemon-tests-main daemon-tests-jb daemon-tests-pubsub daemon-tests daemon-tests-sdp-orig-replacements daemon-tests-moh daemon-tests-evs-dtx daemon-tests-transform \ daemon-tests-transcode-config daemon-tests-codec-prefs daemon-tests-http daemon-tests-heuristic \ daemon-tests-asymmetric daemon-tests-rtcp daemon-tests-redis-subscribe daemon-tests-rtp-ext daemon-tests-ssrc \ - daemon-tests-bundle daemon-tests-dtls daemon-tests-rollback daemon-tests-rollback-redis \ + daemon-tests-bundle daemon-tests-dtls daemon-tests-ice-force-answer daemon-tests-rollback daemon-tests-rollback-redis \ daemon-tests-redis-srtp-index \ daemon-tests-recording daemon-tests-create daemon-tests-created-subscriptions \ daemon-tests-dtx daemon-tests-dtx-cn daemon-tests-dtx-no-shift daemon-tests-dtx-dtmf \ @@ -320,6 +320,9 @@ daemon-tests-bundle: daemon-test-deps daemon-tests-dtls: daemon-test-deps ./auto-test-helper "$@" perl -I../perl auto-daemon-tests-dtls.pl +daemon-tests-ice-force-answer: daemon-test-deps + ./auto-test-helper "$@" perl -I../perl auto-daemon-tests-ice-force-answer.pl + daemon-tests-recording: daemon-test-deps ./auto-test-helper "$@" perl -I../perl auto-daemon-tests-recording.pl diff --git a/t/auto-daemon-tests-ice-force-answer.pl b/t/auto-daemon-tests-ice-force-answer.pl new file mode 100644 index 000000000..9196115de --- /dev/null +++ b/t/auto-daemon-tests-ice-force-answer.pl @@ -0,0 +1,77 @@ +#!/usr/bin/perl + +use strict; +use warnings; +use NGCP::Rtpengine::Test; +use NGCP::Rtpclient::SRTP; +use NGCP::Rtpengine::AutoTest; +use Test::More; + +# Regression coverage for issue #2161: forced ICE towards a plain-RTP peer +# that explicitly rejects ICE in its answer. The answer towards the +# originating plain-RTP peer must not invent ICE, even when the answer +# control request specifies ICE=force. +autotest_start(qw(--config-file=none -t -1 -i 203.0.113.1 -i 2001:db8:4321::1 + -n 2223 -f -L 7 -E)) or die; + +my ($sock_a, $sock_b, $port_a, $port_b); +($sock_a, $sock_b) = new_call( + [qw(198.51.100.1 7320)], + [qw(198.51.100.3 7322)], +); + +($port_a) = offer('forced ICE offer, plain RTP peer', { + ICE => 'force', +}, < 'force', +}, <