Skip to content
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

NoSIP plugin: Fixed SRTP-SDES for "process" request and session update #2196

Merged
merged 3 commits into from
Jun 3, 2020
Merged

NoSIP plugin: Fixed SRTP-SDES for "process" request and session update #2196

merged 3 commits into from
Jun 3, 2020

Conversation

ihusejnovic
Copy link
Contributor

Hi,
As I promised in my previous pull request here is the fix for SRTP-SDES in the NoSIP plugin.

Two issues are fixed:

  • SRTP-SDES for session update. It's the same fix applied in the SIP plugin.
  • "srtp" property was ignored for "process" requests. For example, if you send a "process" request with "srtp" set to “sdes_mandatory” and SDP that doesn’t contain any crypto attributes, although it should fail, it would be processed.

Also, I updated the demo page for NoSIP plugin for easier testing session update and SRTP-SDES.

p.s. I hope I will not promise a new fix in this pull request :)

@lminiero
Copy link
Member

lminiero commented Jun 3, 2020

Thanks a lot, this is really appreciated! I'll try to test this later today but from a quick glance this looks great, and much more work than I expected it to be.

p.s. I hope I will not promise a new fix in this pull request :)

Hahaha, hope you didn't really feel compelled to do it! You're doing a lot for the project, and your contributions are always appreciated 🙂

@lminiero
Copy link
Member

lminiero commented Jun 3, 2020

Looks good! I was a bit confused during the demo at first, because disabling video only hid the local one but not the remote one: then I remembered how I wrote my own demo, and that the remove video is actually coming from the callee, that is indeed still sending video... I verified through webrtc-internals that renegotiation seems to be happening correctly ✌️

Merging 👍

@lminiero lminiero merged commit 96929f5 into meetecho:master Jun 3, 2020
@ihusejnovic ihusejnovic deleted the ihusejnovic-nosip-plugin-fix-srtp branch June 20, 2020 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants