-
Notifications
You must be signed in to change notification settings - Fork 42
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
Response to connectivity checks prior to calling iceTransport.start()? #170
Comments
Here is an update to the proposed text for Section 5.1: An RTCIceGatherer instance is associated to an RTCIceTransport. As noted in [RFC5245] Section 7.1.2.3, an incoming connectivity check utilizes the remote ufrag and the local password, whereas an outgoing connectivity check utilizes the local ufrag and the remote password. Therefore an RTCIceGatherer possesses the information necessary to validate incoming connectivity checks and may respond to them before an RTCIceTransport is constructed or iceTransport.start() is called. Since initiating connectivity checks requires the remote password, an RTCIceTransport can only initiate checks after iceTransport.start() is called. To enable an RTCIceTransport to initiate connectivity checks to peers that had previously sent connectivity checks, or to indicate to an RTCIceTransport that a connectivity check had previously been received (and whether it was responded to), the RTCIceGatherer must store information from incoming connectivity checks (such as the remote ufrag) along with an indication of whether it responded, and provide this to associated RTCIceTransport objects so that they can respond if the RTCIceGatherer did not, and can initiate their own connectivity checks. |
I agree with the idea. The only issue is "may". If we do have some people support pre-transport check answers directly from the gatherer it could cause some implementations to have different behavioural patterns than others. I know it was not desirable to have different levels of API expectations. Is anyone concern about this particular "may" case and the resulting behavioural difference between an engine that does support answering and one which does not. |
#48 Update to the Statistics API, reflecting: #85 Update on 'automatic' use of scalable video coding, as noted in: #156 Update to the H.264 parameters, as noted in: #158 Update to the 'Big Picture', as noted in: #159 Added support for maxptime, as noted in: #160 Changed 'RTCIceTransportEvent' to 'RTCIceGathererEvent' as noted in: #161 Update to RTCRtpUnhandledEvent as noted in: #163 Added support for RTCIceGatherer.state as noted in: #164 Revised the text relating to RTCIceTransport.start() as noted in: #166 Added text relating to DTLS interoperability with WebRTC 1.0, as noted in: #167 Revised the text relating to RTCDtlsTransport.start() as noted in: #168 Clarified handling of incoming connectivity checks by the RTCIceGatherer as noted in: #170 Added a reference to the ICE consent specification, as noted in: #171
At the ORTC CG meeting of January 27, 2015, there appeared to be consensus to change the "MAY" to a "MUST" so as to have consistent behavior. Text in Section 5.1 would now read as follows: "As noted in [RFC5245] Section 7.1.2.3, an incoming connectivity check utilizes the remote ufrag and the local password, whereas an outgoing connectivity check utilizes the local ufrag and the remote password. Therefore an RTCIceGatherer possesses the information necessary to validate incoming connectivity checks and MUST respond to them before an RTCIceTransport is constructed or iceTransport.start() is called. Since initiating connectivity checks requires the remote password, an RTCIceTransport can only initiate checks after iceTransport.start() is called. To enable an RTCIceTransport to initiate connectivity checks to peers that had previously sent connectivity checks, the RTCIceGatherer must store information from incoming connectivity checks (such as the remote ufrag), and provide this to associated RTCIceTransport objects so that they can initiate their own connectivity checks." |
There is a consequence of this, which is that by responding to the connectivity check, the remote peer may believe that it has a functioning ICE transport and can then call DtlsTransport.start(). So DTLS packets can show up before there is even a DtlsTransport to even receive them. |
- Revised the text relating to RTCDtlsTransport.start(), as noted in: Issue #168 - Clarified pruning of local candidates within the RTCIceGatherer, as noted in: Issue #174 - Clarified handling of incoming connectivity checks by the RTCIceGatherer, as noted in: Issue #170 - Added Section 9.3.2.1, defining DTMF capabilities and settings, as noted in: Issue #177 - Clarified pre-requisites for insertDTMF(), based on: Issue #178 - Added Section 8.3.2 and updated Section 9.5.1 to clarify aspects of RTCP sending and receiving, based on: Issue #180
There is a problem with having an IceGatherer respond to connectivity checks. RFC 5245 Section 7.1.2.2 states: 7.1.2.2. ICE-CONTROLLED and ICE-CONTROLLING The agent MUST include the ICE-CONTROLLED attribute in the request if The problem is that an IceGatherer does not know its role, and so it cannot determine if there is a role conflict so as to correctly behave as specified in RFC 5245 Section 7.1.3.1: 7.1.3.1. Failure Cases If the STUN transaction generates a 487 (Role Conflict) error Therefore, I don't believe that an IceGatherer can respond to incoming connectivity checks. |
Correct, the gatherer can pre-screen failed credential because it's using the local password but cannot send a successful reply because it does not know the correct role until IceTransport.start is called. If the gatherer replied earlier it might have incorrectly responded without indicating the role conflict error case. |
Here is a proposed resolution: In Section 5.1, change: "As noted in [RFC5245] Section 7.1.2.3, an incoming connectivity check utilizes the remote ufrag and the local password, whereas an outgoing connectivity check utilizes the local ufrag and the remote password. Therefore an RTCIceGatherer possesses the information necessary to validate incoming connectivity checks and must respond to them before an RTCIceTransport is constructed or iceTransport.start() is called. Since initiating connectivity checks requires the remote password, an RTCIceTransport can only initiate checks after iceTransport.start() is called. To enable an RTCIceTransport to initiate connectivity checks to peers that had previously sent connectivity checks, the RTCIceGatherer must store information from incoming connectivity checks (such as the remote ufrag) and provide this to associated RTCIceTransport objects so that they can initiate their own connectivity checks." To: "As noted in [RFC5245] Section 7.1.2.2, an incoming connectivity check contains an ICE-CONTROLLING or ICE-CONTROLLED attribute, depending on the role of the ICE agent initiating the check. Since an RTCIceGatherer object does not have a role, it cannot determine whether to respond to an incoming connectivity check with a 487 (Role Conflict) error, and therefore it must not respond to incoming connectivity checks. To enable an RTCIceTransport to initiate connectivity checks to peers that had previously sent connectivity checks, the RTCIceGatherer must store information from incoming connectivity checks (such as the remote ufrag) and provide this to associated RTCIceTransport objects so that they can validate the incoming connectivity check in order to decide whether to respond and initiate their own connectivity checks once iceTransport.start() is called." Also, in Section 3.3.2 add: "As noted in [RFC5245] Section 7.1.2.3, an incoming connectivity check utilizes the remote ufrag and the local password, whereas an outgoing connectivity check utilizes the local ufrag and the remote password. Since start() provides role information as well as the local and remote ufrag/password, once start() is called, an RTCIceTransport object can validate incoming connectivity checks, determine whether an incoming check represents a role conflict and initiate connectivity checks." |
If we did allow association a transport prior to calling start, it's possible that an incoming remote ufrag could "latch" onto the transport. But that causes a few race conditions. 1) it's still possible for multiple remote ufrags to come in where there's no local transport setup to attach and it's not always possible to know in advance how many transports would need to be pre-setup to allow latching for all remote ufrags that may arrive (so we are still left with the original problem of what to do). 2) we auto-create transports for non set-up ufrags but 3) there's a race condition where the system auto-latches an incoming ufrag to a transport while the programmer is about to call start() on that transport with a different ufrag/password received via signalling. Even an event saying "transport created/latched to remote ufrag" may not help because of the race between the event and calling start from signalling may be too close in time to prevent mistakes. 4) many scenarios (if not most) will never receive a remote ufrag incoming binding request until start is called due to firewalls anyway. So my recommendation is to either a) ignore binding requests that come in early and allow re-transmission to solve the issue [not ideal] b) buffer incoming requests whose remote ufrag is not associated until a start is called then deliver those packets to the newly started transport to maximize setup speed. I do not see an elegant auto-latching solution for non pre-known ufrags that solves the problem... |
A revision to the proposed text: In Section 3.3.2: As noted in [RFC5245] Section 7.1.2.3, an incoming connectivity check utilizes the local/remote ufrag and the local password, whereas an outgoing connectivity check utilizes the local/remote ufrag and the remote password. Since start() provides role information, an RTCIceTransport object can respond to incoming connectivity checks and initiate connectivity checks. In Section 5.1: As noted in [RFC5245] Section 7.1.2.2, an incoming connectivity check contains an ICE-CONTROLLING or ICE-CONTROLLED attribute, depending on the role of the ICE agent initiating the check. Since an RTCIceGatherer object does not have a role, it cannot determine whether to respond to an incoming connectivity check with a 487 (Role Conflict) error; however, it can validate that incoming connectivity check utilizes the local ufrag/password, and if not, can respond with an 401 (Unauthorized) error, as described in [RFC5389] Section 10.1.2. For incoming connectivity checks that pass validation, the RTCIceGatherer MUST buffer the incoming connectivity checks so as to be able to provide them to associated RTCIceTransport objects so that they can respond and initiate their own connectivity checks once iceTransport.start() is called. |
I do not think that we will see incoming connectivity checks without calling start under most real-world browser to browser (or client to client) scenarios. That's because most times all incoming connectivity checks will be blocked until outgoing connectivity checks are issued too (even when TURN is present). There are a small subset of firewalls where opening the reflexive pinhole will allow incoming connectivity packets but they are the rarity and not the norm. Having said that, if we feel handling this cause is important we could: There are some consequences though of doing this:
The question I ask, is it worth it? Given that firewalls will most certainly block this working for client to client and this is more useful for server scenarios where firewalls are not present. |
The only solution to this is to associate an To know this association has happened, we would need an event on the This allows incoming connectivity checks to be responded to immediate if an If no |
#148 - Clarified handling of incoming connectivity checks prior to calling iceTransport.start(), as noted in: #170 - Clarified handling of incoming DTLS packets, as noted in: #173 - Added RTCIceGatherer as an optional argument to the RTCIceTransport constructor, as noted in: #174 - Clarified handling of contradictory RTP/RTCP multiplexing settings, as noted in: #185 - Clarified error handling relating to RTCIceTransport, RTCDtlsTransport and RTCIceGatherer objects in the "closed" state, as noted in: #186 - Added component method and createAssociatedGatherer() method to the RTCIceGatherer object, as noted in: #188 - Added close() method to the RTCIceGatherer object as noted in: #189 - Clarified behavior of TCP candidate types, as noted in: #190 - Clarified behavior of iceGatherer.onlocalcandidate, as noted in: #191 - Updated terminology in Section 1.1 as noted in: #193 - Updated RTCDtlsTransportState definitions, as noted in: #194 - Updated RTCIceTransportState definitions, as noted in: #197
@pthatcher proposed on the last CG to add a factory method like |
How does this work? For example: // Example to demonstrate forking when RTP and RTCP are not multiplexed,
// so that both RTP and RTCP IceGatherer and IceTransport objects are needed.
// Create ICE gather options
var gatherOptions = new RTCIceGatherOptions();
gatherOptions.gatherPolicy = RTCIceGatherPolicy.relay;
gatherOptions.iceservers = ... ;
// Create ICE gatherer objects
var iceRtpGatherer = new RTCIceGatherer(gatherOptions);
var iceRtcpGatherer = iceRtpGatherer.createAssociatedGatherer();
// Create the ICE RTP and RTCP transports
var iceRtpTransport = new RTCIceTransport(iceRtpGatherer);
var iceRtcpTransport = iceRtpTransport.createAssociatedTransport();
// Prepare to handle incoming connectivity checks
iceRtpGatherer.onunhandledcheck = function (event){iceRtpTransport.respondToIncomingChecks(iceRtpGatherer,event.remoteUfrag,event.remoteRole);};
iceRtcpGatherer.onunhandledcheck = function (event) {iceRtcpTransport.respondToIncomingChecks(iceRtcpGatherer,event.remoteUfrag,event.remoteRole);};
// Prepare to signal local candidates
iceRtpGatherer.onlocalcandidate = function (event) {mySendLocalCandidate(RTCIceComponent.RTP, event.candidate)};
iceRtcpGatherer.onlocalcandidate = function (event) {mySendLocalCandidate(RTCIceComponent.RTCP, event.candidate)};
mySendInitiate({
"icertp": iceRtpGatherer.getLocalParameters(),
"icertcp": iceRtcpGatherer.getLocalParameters()
},
function(response) {
// Assume we are only getting a single response...
// Create the ICE Transport Controller object and add the RTP Ice Transport to it
var controller = new RTCIceTransportController();
controller.addTransport(iceRtpTransport);
// It is possible that the RTP and/or RTCP ICE transports were "early started" so do not call start if
// the role is already set.
if (!iceRtpTransport.role) {iceRtpTransport.start(iceRtpGatherer, response.icertp, RTCIceRole.controlling);};
if (!iceRtcpTransport.role){iceRtcpTransport.start(iceRtcpGatherer, response.icertcp, RTCIceRole.controlling);};
// Prepare to add ICE candidates signalled by the remote peer
mySignaller.onRemoteCandidate = function(remote) {
if (remote.component == RTCIceComponent.RTP){
iceRtpTransport.addRemoteCandidate(remote.candidate);
} else {
iceRtcpTransport.addRemoteCandidate(remote.candidate);
};
}
// ... setup DTLS, RTP, SCTP, etc.
});
function mySendLocalCandidate(component, candidate){
mySignaller.mySendLocalCandidate({
"candidate": candidate,
"component": component
});
} |
I see potential problem with var iceGatherer = new RTCIceGatherer(...);
var iceTransport = new RTCIceTransport(iceGatherer,...);
var dtlsTransport = new RTCDtlsTransport(iceTransport,...)
signalWithBob.sendGathererInfo(aliceGatherer);
signalWithBob.onResponse = function(var data) {
// NOTE: Uncertain if the previous iceTransport auto-latched from an incoming connectivity
// check or if a new transport will be needed.
var useIceTransport = RTCIceTransport.createOrObtainExistingTransport(data.remoteUsernameFrag);
if (useIceTransport != iceTransport) {
// brand new ice transport was created thus need to create new DTLS transport...
// PROBLEM: If an ICE connectivity check arrives during this time for this remote username
// fragment then the ICE gatherer will auto-respond and latch and incoming DTLS packets can
// arrive immediately, i.e. BEFORE the new dtlsTransport is constructed. This is a small race
// window but it exists. Thus the saving of buffering in the IceGatherer is at the expense of
// having to buffer within the IceTransport instead.
var dtlsTransport = new RTCDtlsTransport(useIceTransport);
}
useIceTransport.start(...);
};
|
FYI, I posted the proposal for IceGatherer.getOrCreateTransport to the list, and have gotten no response so far. I think we should proceed with it. I think the issue around receiving a DTLS packet is serious enough to block this. Worst case is that while doing call forking, the DTLS setup takes a little longer, or that we would maybe buffer a few DTLS packets in the ICE transport, neither of which is a very big deal long-term. |
@pthatcher created slides on that for today's CG. I agree with it. |
Here is Peter's proposal to the list: And here are the slides: |
Applying Peter's proposal to the sample code, and found something that bugs me: In the case where an RTCP transport is created, how can getOrCreateTransport know the RTP IceTransport to associate it with? Do we really want getOrCreateAssociatedTransport ?? |
Because of issue #223 , the The problem is that if you pre-created a bunch of RTP and RTCP transports to auto-latch at the start, you'd have to do a look in your own list of pre-created RTP ice transport to find the existing RTP transport that was returned by A better solution would be to change So The same niceness could be done for the ice gatherer by renaming Finally, the RTP gatherer / transport objects should own the RTCP gatherer / transport objects so they would not be garbage collected independent from RTP so your programmer's code would not even have to remember the related RTCP ice gatherer/transport at all (simplifying a lot of code that uses both). For the auto-latch process on the RTCP to work, the |
Alt: add attribute of |
RTP should own RTCP object, not the other way around. So keeping RTP transport would keep RTCP from getting garbage collected. |
The behavior of rtcpIceGatherer.getOrCreateTransport() is under-specified. Here is a proposed clarification: getOrCreateTransport |
Basic connectivity check issue is fixed but the advanced stuff was not completed (yet). |
We can solve the basic connectivity check issue with buffering. |
The question has been asked whether an RTCIceGatherer object can respond to incoming ICE connectivity checks prior to calling iceTransport.start() or even constructing an RTCIceTransport.
It seems to me that the RTCIceGatherer has the information needed to respond to incoming ICE connectivity checks (the local ufrag/password), but does NOT have the information needed to initiate its own connectivity checks (the remote ufrag/password).
However, having an RTCIceGatherer respond to incoming connectivity checks before iceTransport.start() is called is tricky because it means that outgoing connectivity checks will not be triggered until iceTransport.start() is eventually called, and doing so in a timely way may require the RTCIceGatherer to provide information from the incoming checks to the RTCIceTransport once iceTransport.start() is called.
Here is some proposed text for Section 5.1:
An RTCIceGatherer MAY respond to connectivity checks before an RTCIceTransport is constructed or iceTransport.start() is called. However, connectivity checks are only initiated by the RTCIceTransport, after iceTransport.start() is called.
The text was updated successfully, but these errors were encountered: