search

cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.19k stars 470 forks source link

When is safe to use srtp_remove_stream? #656

Open fippo opened 8 months ago

fippo commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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.