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

When is safe to use srtp_remove_stream? #656

Open
fippo opened this issue Oct 25, 2023 · 2 comments
Open

When is safe to use srtp_remove_stream? #656

fippo opened this issue Oct 25, 2023 · 2 comments

Comments

@fippo
Copy link
Contributor

fippo commented Oct 25, 2023

srtp_remove_stream removes the context associated with a particular SSRC.
Which seems like a good thing memory wise (and for faster lookups?) to do for long-running sessions with a lot of SSRCs added and removed (even though I have not heard many complaints yet).

Given to the considerations in
https://www.rfc-editor.org/rfc/rfc3711#section-8
and
https://www.rfc-editor.org/rfc/rfc7714#section-8.4
is this "safe" to call srtp_remove_stream as a receiver?
If a remote side chooses to reuse a SSRC with a new ROC that is a problem of that side and if that side reuses the ROC it should fail to decrypt?

And while at it, removing a sending SSRC does not prevent later reuse? Which seems ok but documentation handrails might be good!

See also #68 which is somewhat related

@pabuhler
Copy link
Member

Yeah this is not a clean cut thing. At the moment I think it is not libSRTP's jobs to prevent "you" or the far end from misusing SSRC's, nor is it responsible for managing the life time of SSRC's. In general I have followed the rules of RFC 3550 when it comes to SSRC lifetime, ie either receiving an RTCP BYE or the SSRC timeout, along with some extra RTP level book keeping to prevent old SSRC's. But I can see how this alone might not address all the security concerns.
You could argue that libSRTP could do more to help and we could change the api to support marking a SSRC as inactive/old and release the resources associated with it, but maintain a list of these "old" SSRC's and prevent them from being reused, could still be a resource problem in cases where there are a lot of SSRC changes.

@fippo
Copy link
Contributor Author

fippo commented Nov 1, 2023

Thank you, it is complicated indeed. I think it is ok for me to land the libWebRTC change to use srtp_remove_stream which should address the resource usage woes. It isn't perfect, SSRCs might still get re-added via the ssrc_any_inbound policy if they are in-flight but should lead hopefully lead to lower resource usage and maybe even slightly faster lookup during unprotect.

In general I like the approach libsrtp takes, very "lib" style which makes it useful and easy to upgrade.
If libsrtp were to automatically timeout that would wreak havoc on webrtc's approach for using replaceTrack(null) which should never cause renegotiations (and it does not switch ssrcs in that case; one might argue that is permited though with the mid extension) or disabling e.g. a simulcast layer which for ages incorrectly caused a RTCP BYE.

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

No branches or pull requests

2 participants