Unencrypted SRTP fails with gstreamer gstsrtp

[dwmw2]

Since commit f1d1e57 using gstrtsp (in Pidgin, with the Pidgin-chime audio stream) fails.

(02:21:06) backend-fs2: farstream-send-codec-changed: codec: 97: audio CHIME clock:16000 channels:1
(02:21:06) mediamanager: gst pipeline error: Could not initialize SRTP encoder
(02:21:06) mediamanager: Debug details: ../ext/srtp/gstsrtpenc.c(1090): gst_srtp_enc_check_set_caps (): /GstPipeline:pipeline0/GstBin:conf_0x5c662637cc50/FsRtpConference:fsrtpconference0/GstRtpBin:rtpbin0/GstSrtpEnc:srtpenc_1:
Failed to add stream to SRTP encoder (err: 2)
(02:21:06) backend-fs2: gst error Could not initialize SRTP encoder

https://github.com/GStreamer/gst-plugins-bad/blob/master/ext/srtp/gstsrtp.c

[pabuhler]

@dwmw2, thanks for reporting, I will do some investigation and get back to you.

[pabuhler]

I have tried to look at both bits of code, and I see there is a miss match. The origin code in libSRTP was in correct as the cipher_key_len is also the size of the mast key and salt which is used to derive authentications keys, so the previous value of 16 is just incorrect. Exposing the NULL cipher and srtp_sec_serv_t in the public API is a bit of a double up and slightly confusing.
I am not sure what todo here, it is possible to revert the change and go back refactor why it was done in the first place. Alternatively we can make a patch on GStreamer / gstsrtp.c to set the right key size. @bifurcation do you remember the reason for this change?

[freemangordon]

I am facing the same/similar issue on maemo leste (which is devuan daedalus with packages on top) using telepathy-farstream. After some debugging I came to a conclusion that libsrtp does not like null crypto/null auth after 2542d68: basically it ignores policy being set to null auth and proceeds as if sha1-80 is used.

I made a patch against 2.5.0 which makes it work, like, I am able to do SIP calls with android and with nokia n900 (running fremantle): https://git.maemo.org/leste-upstream-forks/libsrtp2/commit/cd285c8f4daa13ee4d2f5d131c6aacf18c8e11dc

@pabuhler: Is that the proper way of fixing the issue? Shall I make a PR with the patch re-based to current master?

[pabuhler]

@freemangordon , I looked quickly at your patch and I think it looks ok, so a PR is probably the best next step.
If I understood it correctly then the change is to require the key size to be the max of either the chiper key or the auth key, and in the case both are null then no key is required?
If you make a pr it will be easier to review and verify. Great that you added a test!

[freemangordon]

@freemangordon , I looked quickly at your patch and I think it looks ok, so a PR is probably the best next step.<

#757

If I understood it correctly then the change is to require the key size to be the max of either the chiper key or the auth key, and in the case both are null then no key is required?<

yes, it does not make sense to set auth key len to anything but zero if there is no auth