-
Notifications
You must be signed in to change notification settings - Fork 496
New DTLS-SRTP implementation based upon SharpSRTP #1486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…'t match the certificate type
|
Fair dues for taking this on! The AGPL code does cause a few people problems. I'm doing some testing with the various WebRTC examples and will add comments for any issues I find (first one immediately below). What's your plan for the SharpSRTP package? It would seem more efficient for this project to use a separate nuget package rather than mirroring the code. If you did want to stop maintaining the package at some point it would be easy enough to bring the code into this repo as this PR does. |
|
Using this PR with the WebRTC Get Started example results in an exception almost immediately after the media streams are started. My guess would be it's caused by two media stream (audio and video) attempting to use the same encryption context. |
|
"What's your plan for the SharpSRTP package?" "My guess would be it's caused by two media stream (audio and video) attempting to use the same encryption context." As for the locking, the original implementation had locking implemented inside SRTP contexts. I wanted to avoid locking entirely, which means now the caller is responsible for locking. Ideally, you'd have 1:1 mapping in between contexts/threads, avoiding locking entirely. Edit: The issue should be fixed. I added locking when RTP/RTCP multiplexing is being used and the rtpChannel is shared among multiple media streams. |
Ok, if the public API is likely to be evolving quickly it may be better to use the original approach in this PR and duplicate the code in this repo.
It's working for me now. I do always get a single message of a decrypt failure as per below with teh WebRTCGetStarted exanple. It only seems to happen once, It doesn't occur on the master branch, |
|
-4 is the replay protection failing, so it is likely the MediaTrack trying to decrypt the same RTCP message for the second time. I presume this is the result of multiplexing re-using the same channel for multiple tracks. Yes, in this case it seems to be a problem of my current implementation as per RFC 3711:
RTCP indexes for both video/audio start from 1 and they only differ in SSRC, yet in this case they are sharing the same SrtpContext. Edit: Should be fixed now. |
To remove the AGPL-derived code, I implemented a new DTLS-SRTP layer based upon BouncyCastle DTLS samples and the RFCs. More info here: https://github.com/jimm98y/SharpSRTP
This PR wires up the new implementation into the current code base. The only modification when compared to SharpSRTP are the namespaces which have been adjusted to match sipsorcery and the logging was wired up to sipsorcery's logging mechanism.
I have unit tests for all the implemented RFCs in my repo and it should be easy to sync any changes/bugfixes in the future. I tested some of the WebRTC samples that I was able to run on ARM64 and I also tested the new implementation on https://github.com/jimm98y/SharpRTSPtoWebRTC.
This new DTLS-SRTP implementation also fixes the RSA TLS certificate sipsorcery was using in WebRTC, now it's using ECDsa by default. By default I also disabled DTLS 1.0, leaving only DTLS 1.2 available. DTLS 1.0 can be enabled using overrides. DTLS 1.3 currently cannot be supported because BouncyCastle does not support it yet.
This PR should also resolve AES-GCM AEAD support #871.