From ee7e80d615a718fd9c37be06b26cc3b7a43c2997 Mon Sep 17 00:00:00 2001 From: Mikhail Fludkov Date: Mon, 25 Jan 2016 16:25:51 +0100 Subject: [PATCH] rtpsession: don't act on suspicious BYE RTCP Some endpoints (like Tandberg E20) can send BYE packet containing our internal SSRC. I this case we would detect SSRC collision and get rid of the source at some point. But because we are still sending packets with that SSRC the source will be recreated immediately. This brand new internal source will not have some variables incorrectly set in its state. For example 'seqnum-base` and `clock-rate` values will be -1. The fix is not to act on BYE RTCP if it contains internal or unknown SSRC. https://bugzilla.gnome.org/show_bug.cgi?id=762219 --- gst/rtpmanager/rtpsession.c | 21 ++---- tests/check/Makefile.am | 2 +- tests/check/elements/rtpsession.c | 108 +++++++++++++++++++++++++++++- 3 files changed, 115 insertions(+), 16 deletions(-) diff --git a/gst/rtpmanager/rtpsession.c b/gst/rtpmanager/rtpsession.c index ae34d67c1a..620d623d23 100644 --- a/gst/rtpmanager/rtpsession.c +++ b/gst/rtpmanager/rtpsession.c @@ -2443,21 +2443,18 @@ rtp_session_process_bye (RTPSession * sess, GstRTCPPacket * packet, for (i = 0; i < count; i++) { guint32 ssrc; RTPSource *source; - gboolean created, prevactive, prevsender; + gboolean prevactive, prevsender; guint pmembers, members; ssrc = gst_rtcp_packet_bye_get_nth_ssrc (packet, i); GST_DEBUG ("SSRC: %08x", ssrc); /* find src and mark bye, no probation when dealing with RTCP */ - source = obtain_source (sess, ssrc, &created, pinfo, FALSE); - if (!source) - return; - - if (source->internal) { - /* our own source, something weird with this packet */ - g_object_unref (source); - continue; + source = find_source (sess, ssrc); + if (!source || source->internal) { + GST_DEBUG ("Ignoring suspicious BYE packet (reason: %s)", + !source ? "can't find source" : "has internal source SSRC"); + break; } /* store time for when we need to time out this source */ @@ -2515,12 +2512,7 @@ rtp_session_process_bye (RTPSession * sess, GstRTCPPacket * packet, } } - if (created) - on_new_ssrc (sess, source); - on_bye_ssrc (sess, source); - - g_object_unref (source); } if (reconsider) { RTP_SESSION_UNLOCK (sess); @@ -2529,6 +2521,7 @@ rtp_session_process_bye (RTPSession * sess, GstRTCPPacket * packet, sess->callbacks.reconsider (sess, sess->reconsider_user_data); RTP_SESSION_LOCK (sess); } + g_free (reason); } diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am index bc78f9fc5d..3aec98ce91 100644 --- a/tests/check/Makefile.am +++ b/tests/check/Makefile.am @@ -561,7 +561,7 @@ elements_rtprtx_CFLAGS = $(GST_PLUGINS_BASE_CFLAGS) $(CFLAGS) $(AM_CFLAGS) elements_rtprtx_LDADD = $(GST_PLUGINS_BASE_LIBS) -lgstrtp-$(GST_API_VERSION) $(LDADD) elements_rtpsession_CFLAGS = $(GST_PLUGINS_BASE_CFLAGS) $(CFLAGS) $(AM_CFLAGS) -elements_rtpsession_LDADD = $(GST_PLUGINS_BASE_LIBS) -lgstrtp-$(GST_API_VERSION) $(LDADD) +elements_rtpsession_LDADD = $(GST_PLUGINS_BASE_LIBS) -lgstnet-$(GST_API_VERSION) -lgstrtp-$(GST_API_VERSION) $(GIO_LIBS) $(LDADD) elements_rtpcollision_CFLAGS = $(GST_PLUGINS_BASE_CFLAGS) $(CFLAGS) $(AM_CFLAGS) elements_rtpcollision_LDADD = $(GST_PLUGINS_BASE_LIBS) -lgstnet-$(GST_API_VERSION) -lgstrtp-$(GST_API_VERSION) $(GIO_LIBS) $(LDADD) diff --git a/tests/check/elements/rtpsession.c b/tests/check/elements/rtpsession.c index 2f19554374..faeb653cc3 100644 --- a/tests/check/elements/rtpsession.c +++ b/tests/check/elements/rtpsession.c @@ -20,6 +20,7 @@ * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, * Boston, MA 02110-1301, USA. */ +#define GLIB_DISABLE_DEPRECATION_WARNINGS #include #include @@ -27,6 +28,7 @@ #include #include +#include static const guint payload_size = 160; static const guint clock_rate = 8000; @@ -702,6 +704,109 @@ GST_START_TEST (test_dont_lock_on_stats) } GST_END_TEST; +static void +suspicious_bye_cb (GObject * object, GParamSpec * spec, gpointer data) +{ + GValueArray *stats_arr; + GstStructure *stats, *internal_stats; + gboolean *cb_called = data; + gboolean internal = FALSE, sent_bye = TRUE; + guint ssrc = 0; + guint i; + + g_assert (*cb_called == FALSE); + *cb_called = TRUE; + + g_object_get (object, "stats", &stats, NULL); + stats_arr = + g_value_get_boxed (gst_structure_get_value (stats, "source-stats")); + g_assert (stats_arr != NULL); + fail_unless (stats_arr->n_values >= 1); + + for (i = 0; i < stats_arr->n_values; i++) { + internal_stats = g_value_get_boxed (g_value_array_get_nth (stats_arr, i)); + g_assert (internal_stats != NULL); + + gst_structure_get (internal_stats, + "ssrc", G_TYPE_UINT, &ssrc, + "internal", G_TYPE_BOOLEAN, &internal, + "received-bye", G_TYPE_BOOLEAN, &sent_bye, NULL); + + if (ssrc == 0xDEADBEEF) { + fail_unless (internal); + fail_unless (!sent_bye); + break; + } + } + fail_unless_equals_int (ssrc, 0xDEADBEEF); + + gst_structure_free (stats); +} + +static GstBuffer * +create_bye_rtcp (guint32 ssrc) +{ + GstRTCPPacket packet; + GstRTCPBuffer rtcp = GST_RTCP_BUFFER_INIT; + GSocketAddress *saddr; + GstBuffer *buffer = gst_rtcp_buffer_new (1000); + + fail_unless (gst_rtcp_buffer_map (buffer, GST_MAP_READWRITE, &rtcp)); + fail_unless (gst_rtcp_buffer_add_packet (&rtcp, GST_RTCP_TYPE_BYE, &packet)); + gst_rtcp_packet_bye_add_ssrc (&packet, ssrc); + gst_rtcp_buffer_unmap (&rtcp); + + /* Need to add meta to trigger collision detection */ + saddr = g_inet_socket_address_new_from_string ("127.0.0.1", 3490); + gst_buffer_add_net_address_meta (buffer, saddr); + g_object_unref (saddr); + return buffer; +} + +GST_START_TEST (test_ignore_suspicious_bye) +{ + GstHarness *h_rtcp = NULL; + GstHarness *h_send = NULL; + gboolean cb_called = FALSE; + GstTestClock *testclock = GST_TEST_CLOCK (gst_test_clock_new ()); + + /* use testclock as the systemclock to capture the rtcp thread waits */ + gst_system_clock_set_default (GST_CLOCK (testclock)); + + h_rtcp = + gst_harness_new_with_padnames ("rtpsession", "recv_rtcp_sink", + "send_rtcp_src"); + h_send = + gst_harness_new_with_element (h_rtcp->element, "send_rtp_sink", + "send_rtp_src"); + + /* connect to the stats-reporting */ + g_signal_connect (h_rtcp->element, "notify::stats", + G_CALLBACK (suspicious_bye_cb), &cb_called); + + /* Push RTP buffer making our internal SSRC=0xDEADBEEF */ + gst_harness_set_src_caps_str (h_send, + "application/x-rtp,ssrc=(uint)0xDEADBEEF," + "clock-rate=90000,seqnum-offset=(uint)12345"); + gst_harness_push (h_send, + generate_test_buffer (0, FALSE, 12345, 0, 0xDEADBEEF)); + + /* Push BYE RTCP with internal SSRC (0xDEADBEEF) */ + gst_harness_set_src_caps_str (h_rtcp, "application/x-rtcp"); + gst_harness_push (h_rtcp, create_bye_rtcp (0xDEADBEEF)); + + /* "crank" and check the stats */ + g_assert (gst_test_clock_crank (testclock)); + gst_buffer_unref (gst_harness_pull (h_rtcp)); + fail_unless (cb_called); + + gst_harness_teardown (h_send); + gst_harness_teardown (h_rtcp); + gst_object_unref (testclock); +} + +GST_END_TEST; + static Suite * rtpsession_suite (void) { @@ -713,8 +818,9 @@ rtpsession_suite (void) tcase_add_test (tc_chain, test_multiple_senders_roundrobin_rbs); tcase_add_test (tc_chain, test_internal_sources_timeout); tcase_add_test (tc_chain, test_receive_rtcp_app_packet); - tcase_add_test (tc_chain, test_dont_lock_on_stats); + tcase_add_test (tc_chain, test_ignore_suspicious_bye); + return s; }