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

Consider making some of the private headers public #721

Open
sorenstoutner opened this issue Jul 26, 2024 · 8 comments
Open

Consider making some of the private headers public #721

sorenstoutner opened this issue Jul 26, 2024 · 8 comments

Comments

@sorenstoutner
Copy link

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
Copy link
Contributor

paulej commented Jul 26, 2024

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
Copy link
Author

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
Copy link
Contributor

paulej commented Jul 27, 2024

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
Copy link
Member

@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
Copy link
Author

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
Copy link
Contributor

fippo commented Aug 13, 2024

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

@pabuhler
Copy link
Member

Will close this issue soon if there is no follow up.

@fippo
Copy link
Contributor

fippo commented Dec 21, 2024

Oh this is basically the same as #738 -- I have a libWebRTC change to remove that usage but it might take a while to land. No libsrtp action needed I think

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

4 participants