cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.24k stars 476 forks source link

Consider making some of the private headers public #721

Open sorenstoutner opened 4 months ago

sorenstoutner commented 4 months ago

I work on packaging Chromium (and Qt WebEngine) in Debian. One of my projects is to get to the stage where Chromium can be built using a system copy of libsrtp instead of the version that is included in the Chromium codebase. Doing so has positive security implications because an update to the system copy of libsrtp can fix a security problem in all instances of the Chromium and derivitive packages in Debian.

The blocker for doing so is that Chromium utilizes some of the private headers in the libsrtp package. This is discussed in the following Chromium issue:

https://issues.chromium.org/issues/40272799

Unfortunately, this issue is currently marked private. I have made a request for the visibility to be opened up, but in the meantime I don't think anyone would mind me quoting this section:

The include from webrtc/pc/srtp_session.c seems harder to remove. It's used in GetRtpAuthParams and GetSendStreamPacketIndex with calls originating from sending packets with external auth. I haven't followed this codepath, but I believe the external auth is used for things like the abs-send-time header extension where chrome needs to update the header extensions close to the socket and then reapply the authentication.

The purpose of this feature request is to see how willing the libsrtp developers would be to make the necessary parts of this private header public. Additionally, to understand if there are any negative implications to doing so.

For a little bit of background, see the following bug reports:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=866784

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1038240

paulej commented 4 months ago

One of the concerns with making private parts public is that there is no guarantee that the private parts will not change. That said, with the latest version of srtp (still under development), some of the public APIs have also changed, though those will be documented.

IMO, it would be wiser to provide APIs to allow access to read-only information, as opposed to exposing more of the internals.

sorenstoutner commented 4 months ago

Both of those are good points. I have requested that one of the Google developers comment here explaining exactly what they need. Perhaps it is already handled by your new public APIs.

paulej commented 4 months ago

Given the code I saw in the links you shared, it doesn't appear that level of information is exposed in the new APIs. It might be undesirable to publish such APIs. I suggest we consider it once we understand exactly what information is needed. If you can collect the list of data that's needed, then everyone can evaluate the best approach to take.

pabuhler commented 4 months ago

@sorenstoutner, if they are indeed updating parts of the authenticated packet and need to "re authenticate" then I would also suggest that they look at restructuring the code so it does not protect the packet until the last possible moment. How would this re auth even work with AEAD / GCM ciphers. But like Paul said if we understand the requirements then we can look at changing the API to accommodate.

sorenstoutner commented 4 months ago

I agree with you. I hope that someone from Google will engage in this conversation. I will politely remind them again in about a week. Otherwise, there isn't much we can do without their input.

fippo commented 3 months ago

tagging @alvestrand (and I wonder whether the "external auth" stuff actually stamps abs-send-time in the network process)