GNOME Bugzilla – Bug 767799
rtspsrc: always fill all srtp encoder properties
Last modified: 2016-06-20 07:58:25 UTC
We need to make all srtp encoder properties explicit, otherwise we might end up with an SRTP encoder with an SRTP cipher with "aes-128-icm" and an SRTCP cipher with "aes-256-icm". Newer versions of libsrtp don't allow this. Well they allow it but, in the example above, for the SRTCP encoder it would use a 128 key instead of a 256 key. This is because SRTP cipher key size is used instead, as it assumes both SRTP and SRTCP uses the same cipher. I fixed this in libsrtp, but it's going to get merged because SDP doesn't even allow having two different cipher for SRTP and SRTCP. Actually, I'm closing the merge request now. https://github.com/cisco/libsrtp/pull/148
Created attachment 329971 [details] [review] make sure to fill all srtp encoder ciphers
Review of attachment 329971 [details] [review]: You probably want the same change in gst-rtsp-server? Also maybe srtpenc should only have a single property for the cipher if it always has the be the same anyway?
(In reply to Sebastian Dröge (slomo) from comment #2) > Review of attachment 329971 [details] [review] [review]: > > You probably want the same change in gst-rtsp-server? Also maybe srtpenc > should only have a single property for the cipher if it always has the be > the same anyway? gst-rtsp-server does the right thing: https://cgit.freedesktop.org/gstreamer/gst-rtsp-server/tree/gst/rtsp-server/rtsp-client.c#n1670 About srtpenc, yes I guess it makes sense, but wouldn't we be breaking a lot of apps? In any case, it is not very urgent. May be we can add a GST_WARNING for now?
A g_warning() sounds better, yes. Not GST_WARNING(), it's a programming error that should be visible :)
Comment on attachment 329971 [details] [review] make sure to fill all srtp encoder ciphers commit 12eb5d6912409cd2ad48985592192ab1c66d8d6d Author: Aleix Conchillo Flaqué <aleix@oblong.com> Date: Fri Jun 17 12:16:32 2016 -0700 rtspsrc: make all srtp encoder properties explicit The Session Data Protocol doesn't allow specifying a cipher for the SRTCP, so it will use the SRTP one. In the "srtpenc" element the cipher "aes-128-icm" is the default for SRTP and SRTCP, but if we want to have an SRTCP with the "aes-256-icm" cipher then we also need to set the SRTP cipher to "aes-256-icm", otherwise "aes-128-icm" will be used instead. https://bugzilla.gnome.org/show_bug.cgi?id=767799