w3c / webrtc-pc

WebRTC 1.0 API
https://w3c.github.io/webrtc-pc/
Other
437 stars 115 forks source link

RTCPeerConnection lacks identity marker beyond current process #1775

Closed eladalon1983 closed 6 years ago

eladalon1983 commented 6 years ago

An object of RTCPeerConnection can be identified within a given process using a reference to it, or even decorated with an identity label. However, neither of these is meaningful if the object is to be referenced by another process, or even on a different machine. Attaching a label on the process itself in an ad-hoc manner would probably be less maintainable than adding an ID attribute to RTCPeerConnection in a standardized way, which would be unique within that process, but visible outside the process. It would be preferable to set the identity to be a string of a specific format, and chosen in such a way that name collisions with peer connections created elsewhere would be unlikely (though such collisions should not truly be problematic for a good implementation).

eladalon1983 commented 6 years ago

I propose a 32 characters hexadecimal string, all characters uppercase ASCII.

  1. When a browser creates a new peer connection, it would theoretically need to ensure that it did not previously randomly choose the same string for a previously created peer connection; in practice, this would be so unlikely, that almost anyone would be able to get away with skipping this check.
  2. Following the usual principle, a good implementation would only assign uppercase characters to the ID, but would be lenient in accepting lowercase IDs from other implementations. Or maybe not...?
benjamingr commented 6 years ago

I'm not familiar with any other DOM API that does this (keep a hard coded guid for reference between processes) ?

alvestrand commented 6 years ago

MediaStreamTracks and MediaStreams have their "id" attribute (not for exactly that purpose, but initially added for a very similar reason)

fippo commented 6 years ago

Following the usual principle, a good implementation would only assign uppercase characters to the ID, but would be lenient in accepting lowercase IDs from other implementations

This would imply it is signaled by SDP? Incidentally it could be put into the o= line... but I think this is a can of worms you do not want to open

eladalon1983 commented 6 years ago

Philipp, my intention was that a browser where the peer connection lives would normally follow the spec to the letter and only provide uppercase hexadecimal strings as IDs, but if ever given an ID from a JS application, would transform lowercase->uppercase before looking up the associated peer connection (*). I do not think the SDP is concerned, but maybe I am missing something?


(*) If this attribute is added to the API, one could have later APIs depend on this identifier.

ibc commented 6 years ago

What does "object to be referenced by another process, or even on a different machine" exactly mean?

And why do we need a peerconnection.id at all? There is no window.getRTCPeerConnectionById().

alvestrand commented 6 years ago

SDP is, happily, not the only way processes talk to each other :-)

eladalon1983 commented 6 years ago

I have a use case where peer connections are produced in one process (namely Chrome's renderer process), but I then need to refer to them on another process ("perform this and that on peer connection this and that"). The latter instruction should originate from the renderer process (where JS code lives). It seems to me that making up a single-use, Chrome-only way of attaching an identifier to the peer connection, so that I might refer to it later when talking about the PC to the other process, would be inadvisable. Therefore this suggested standardized identifier.

ibc commented 6 years ago

If you need to access a specific RTCPeerConnection by id or name, you first need to have a map/object to index them. Again, there is no window.getRTCPeerConnectionById(id) method in the WebRTC API.

So, you need a map/object to index them. And if you have that you can index them in any way:

var peerconnection1 = new RTCPeerConnection(...);

myPeerConnectionsMap = {
   'pc1' : peerconnection1
};

This is something you can do in your own application without the need of polluting the whole WebRTC spec with not needed ids.

murillo128 commented 6 years ago

How "processes" relates at all with javascript apis?

eladalon1983 commented 6 years ago

How would the other process have this map, when the reference for the peer connection is only meaningful in the original process?

eladalon1983 commented 6 years ago

murillo128, the renderer process of Chrome is where the JS code lives. But we need to perform some peer-connection-related work on a different process. So we need a way to tell another process "perform X on peer connection Y." The part where we'd be able to says "on peer connection Y" is the problematic part.

ibc commented 6 years ago

The other process may just be aware of the ids you assign to each peerconnection.

So we need a way to tell another process "perform X on peer connection Y."

What is "another process"?

fippo commented 6 years ago

chrome uses pid and rid internally in webrtc-internals, see here

I would like to have a pc identifier so that in createOffer or similar I can do a console.log(this.id, 'createOffer'). However, standardising this when it is trivial to achieve in JS...

(and I'm glad we agree to keep SDP out of this :-))

eladalon1983 commented 6 years ago

ibc, if I am going to assign IDs to peer connections, and expose those IDs from Chrome to arbitrary JS code, it would probably benefit us to have a standardized way of assigning and exposing those IDs, I think.

murillo128 commented 6 years ago

@eladalon1983 i assume you are talking about native libwertc code, right?

eladalon1983 commented 6 years ago

murillo128, not exactly, afaiu.

murillo128 commented 6 years ago

I am not aware of what a process is in javascript land nor how can you share a pc between two "processes" (javascript contexts?).

ibc commented 6 years ago

it would probably benefit us to have a standardized way of assigning and exposing those IDs, I think.

I'm against standardizing stuff that can be easily implemented in JavaScript.

eladalon1983 commented 6 years ago

ibc, that's very reasonable. However, I've consulted a few people with significantly more experience than me, and they were under the impression that the JS-based approach is not viable in this case (though it took some time to shed light on the nature of the problem). They also thought that adding this attribute would be of general benefit, not just for Chrome, but in general. I will leave the stage to Harald, who is one of those people.

eladalon1983 commented 6 years ago

murillo128 - Chrome is multi-process. Tabs usually have their own process. Some sensitive, private APIs (which are open source, btw - see https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/webrtc_logging_private/) are accessible, but live on their own process, for added security. So the JS code lives in one process, but is trying to communicate "I mean this PC" to another.

ibc commented 6 years ago

Again, a peerconnection.id is useless if there is not a window.getRTCPeerConnectionById(id) method or if your app doesn't provide its own map/object to reference them by some id. And if your apps has its own map/object, then you don't need RTCPeerConnection.id at all.

murillo128 commented 6 years ago

You are referring me to c++ code, so libwertc code, not w3c apis. Seems to me this is something that should be handled internally in libwebrtc/chrome without any need of changing the w3c apis for that.

eladalon1983 commented 6 years ago

I think I am referring to the intersection of C++ and JS code. Please pardon my unfamiliarity with the established terminology, making me invent my own. Let me call an application such as a browser the "hosting application", and a JS-based application the "hosted application". Different hosting applications might define their own new functionality. It may be specific to that hosting application. To give some made-up examples, it could be that one browser wants to allow an API that would rate-limit a given peer connection, and another browser wants to add an API that limits the time that a peer connection may remain open. Normally, this would be achievable with just giving a reference to the RTCPeerConnection object, so no problem. But this assumes a specific structure of the hosting application. Namely, it assumes that the application is single-process, and that the API called for "please rate limit X" is the same process where X lives. Normally, this assumption is true. But not always. When it doesn't hold true, the hosting application would need to invent its own naming scheme that would allow one to say "please do Z on peer connection X." That is possible. But it would be easier if there were a standardized way of saying "peer connection X", which would work even when a reference to the actual RTCPeerConnection is out of the question.

ibc commented 6 years ago

So just have a map of peerconnections indexed by custom ids (made by your app) and propagate them to the other process/app by any means.

eladalon1983 commented 6 years ago

Communicating the custom ID from the hosting application (browser) to the hosted application (JS code) would require a new JS API. It's preferable to only use standards-complying APIs, isn't it?

murillo128 commented 6 years ago

The problem is that in your case, without a shared pc map it is useless to add an id, as you will not be able to retrieve a PC by it's id in the second process/extension/whatever.

ibc commented 6 years ago

The problem is that in your case, without a shared pc map it is useless to add an id, as you will not be able to retrieve a PC by it's id in the second process/extension/whatever.

Because, again, there is no window.getRTCPeerConnectionById(id) method :)

ibc commented 6 years ago

Communicating the custom ID from the hosting application (browser) to the hosted application (JS code) would require a new JS API. It's preferable to only use standards-complying APIs, isn't it?

Nothing that can be done via custom app and custom JavaScript should need a specific W3 standard. You have an specific use case, I may have N other different use cases (in which I also communicate different apps/processes). We cannot request a "standard" for all of them.

fippo commented 6 years ago

@eladalon1983 so a use-case of this would be to allow a webextension to enforce a policy on a peerconnection?

Why would the peerconnection in the renderer process expose its internal id to its own JS? At the same time, the extension would probably not use the RTCPeerConnection object as an interface. See here for the messy way we solved this in Firefox a while back.

A general way to address a peerconnection may be useful but that doesn't imply exposing the ID to javascript in RTCPeerConnection.

eladalon1983 commented 6 years ago

@fippo, the identifier exposed by the hosting application need not be the internal ID that the application regularly uses. If we give the Chrome implementation as an example, you might see that an int is used for identifying peer connections internally, whereas the ID I've proposed to be used externally would be a randomly selected hexadecimal string.

About the general reservations to adding this attribute, I am waiting for some of my more experienced colleagues to come forward with other justifications for this proposal. Thank you all for your time and attention!

alvestrand commented 6 years ago

@fippo, I think the Firefox use case is simpler because they build so much inside a Javascript context. Part of the issue here is that one of the interfaces elad is looking at has access to the PeerConnection object (so anything built into its IDL can be exposed to it), but NOT its Javascript context (so maps and decorations added by Javascript is NOT visible to it), while the other interface that has to talk about the same peerconnections has the opposite access: Full access to Javascript and no access to C++ internals.

alvestrand commented 6 years ago

@ibc the point of having an API is to make things possible. If you have use cases that are presently impossible, but would be possible if you had more API surfaces, it's appropriate to propose them. If it's only your specific problem that gets solved, or if your problem can be solved in other ways, it may not be appropriate to add them; if it solves others' problems too, it may be. In all cases, it's appropriate to raise the question.

ibc commented 6 years ago

Agreed @alvestrand. However, regarding the current API addition proposal, my opinion is that it's not needed and that the requested API is incomplete (in the sense that not just RTCPeerConnection.id would be needed, but also a global getRTCPeerConnectionById() method).

Now, let's go to ORTC spec in which there is no RTCPeerConnection. Following the rationale of this API proposal, we'd also need more and more methods such as getRTCIceTransportById(), getRTCDtlsTransportById(), getRTCRtpSenderById(), etc.

Even more, we would also need WebSocket.id plus getWebSocketById(), and AudioContext.id plus getAudioContextById() and so on.

taylor-b commented 6 years ago

I have a use case where peer connections are produced in one process (namely Chrome's renderer process), but I then need to refer to them on another process ("perform this and that on peer connection this and that"). The latter instruction should originate from the renderer process (where JS code lives).

This seems like a Chrome implementation detail that could be handled by adding an ID to the internal (C++) PeerConnection object. But why would it need to be exposed to JS?

To give some made-up examples, it could be that one browser wants to allow an API that would rate-limit a given peer connection, and another browser wants to add an API that limits the time that a peer connection may remain open. Normally, this would be achievable with just giving a reference to the RTCPeerConnection object, so no problem. But this assumes a specific structure of the hosting application. Namely, it assumes that the application is single-process, and that the API called for "please rate limit X" is the same process where X lives. Normally, this assumption is true. But not always.

I'm confused. Are you saying you want the ability for one tab to rate-limit a PeerConnection owned by another tab, using an API like:

void setPeerConnectionRateLimit(DOMString id, int kbps);

But you'd still need some inter-tab communication mechanism to communicate the ID from the tab that owns the RTCPeerConnection to the one controlling the rate-limiting. And if such a communication mechanism exists, why couldn't it be used in the reverse direction, to send a message saying "rate-limit your PC". In which case the API could go directly on RTCPeerConnection:

partial interface RTCPeerConnection {
    void setRateLimit(int kbps);
};

Sorry if I'm completely misunderstanding your use case. Maybe you could give a more specific example, with some pseudocode?

eladalon1983 commented 6 years ago

Taylor,

fippo commented 6 years ago

that example helps :+1:

so the problem you're trying to solve is that you can not say "please start logging for this peerconnection" because the js needs to communicate with the extension via postMessage and you can't do that with the peerconnection object.

Would it be enough here to say the ID should show up in the RTCPeerConnections toJSON() output?

murillo128 commented 6 years ago

Not really sure why are we discussing about private native apis at w3c. IMHO this is a Chrome implementation specific detail that has no use at all for any web developer.

fippo commented 6 years ago

this is a use-case for webextensions which are a CG so its perfectly fine to discuss here.

murillo128 commented 6 years ago

@fippo how would an extension get a PC from an id?

henbos commented 6 years ago

I'm not sure what I think but I want to raise the question: Is this really Chrome-only? The extension we're talking about is a Chrome extension, but other browsers could also have extensions. This may be an example for a bigger need. And while MediaStream[Track].id are justified by very different use-cases, the concept of an id fits neatly into a well established model for WebRTC-related interfaces.

stefhak commented 6 years ago

@eladalon1983 can you show how the application JS calls the webrtcLoggingPrivate API (using the peerconnection ID)? Would be nice to see the actual code.

Also, would this not apply to e.g. a video element (which would need an ID)? What I'm thinking of is a situation where the source of the video element is a manifest file, and you would want to see (real time) logging of what resolution that is used for each segment and also how the amount of pre-buffered data changes etc.

icydragons commented 6 years ago

So I have another use case for this. On the application side we report a decent amount of telemetry tied to our own concepts of session identifiers. On the webrtc side (and I presume any webrtc implementation), they also have a decent amount of internal telemetry that the js doesn't (and likely shouldn't know). However, after the fact, we get reports with issues that are tied to our identifiers. It would be really nice to have a short succinct way of reporting this in a way that is understandable to both sides. We've actually gotten around this by adding a "setMetadata" call to our private api that allows us to pass our session identifier, but this way would make a lot more sense.

stefhak commented 6 years ago

It seems the uses discussed are related to debugging/monitoring. I'm personally hesitant to add an Id for that. Still, it would be good to see a code example using the Id.

eladalon1983 commented 6 years ago

stefhak@, I'm afraid my employer might not look kindly upon me sharing private code, but given the public code that enables it, I think your imagination could probably fill in the gaps. The public code is here:

benjamingr commented 6 years ago

Could not load src/chrome/browser/extensions/api/webrtc_logging_private/: Request to Git-on-Borg failed.

eladalon1983 commented 6 years ago

Yeah, I've been receiving similar messages today. The link is not broken, it's just that some back-end service is acting up. You could either retry later, or you could extract the file path from those URLs and use that to check on a locally stored checkout of chromium.

stefhak commented 6 years ago

We should probably talk about this at the next VI. I don't see a good reason for the normal user for adding this, it seems related to certain browser implementations and to debugging (of the implementation). OTOH, the cost for adding the Id seems very low.

murillo128 commented 6 years ago

I might have a side-use case that may require a one time nonce that may be used as PC identity:

https://mailarchive.ietf.org/arch/search/?email_list=rtcweb

Long history short, a unique session id would be generated internally and used for idp assertion and exchanged on the dtls setup as per https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-sdp-uks

This session id could be exposed read-only at the pc and used as id.

aboba commented 6 years ago

Consensus from the April VI does not appear to support adding this to the standard: https://www.w3.org/2018/04/12-webrtc-minutes