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

DTLS handshake packets arrive because of connectivity checks before DTLS transport setup #173

Closed
robin-raymond opened this Issue Feb 6, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@robin-raymond
Copy link
Contributor

robin-raymond commented Feb 6, 2015

Issue:
What do we do about DTLS packets that can arrive before there's a DTLS transport to receive the DTLS packets?

How this can happen:

  1. Alice sends an offer containing ICE candidates (i.e. Alice has an IceGatherer but has not called IceTransport.start() because remote ufrag/password is not yet known)
  2. Bob sends his response ufrag / password via signalling channel and starts ICE connectivity checks (i.e. he has called IceTransport.start() method)
  3. Alice receives connectivity checks and responds (aka as agreed in last ORTC CG meeting re: issue #170 ) and she sends an ICE consent reply back to Bob (but she still has not received ufrag/password via signalling yet so she cannot call IceTransport.start() yet)
  4. Bob receives ICE connectivity consent reply and therefor believes a channel is open to send the DtlsTransport client "hello" packet
  5. Alice receives DtlsTransport client hello packet but does not have any IceTransport or DtlsTransport started to receive the DTLS client "hello" packets.

While eventually Alice will receive the signalling necessary to setup an IceTransport and DtlsTransport, she may receive DTLS packets prematurely.

Possible options:

  1. Drop DTLS packets incoming, force DTLS client re-transmit (i.e. consequence is a slow started or disrupted connections);

  2. Buffer DTLS packets incoming for a period of time and deliver packets to eventual IceTransport.start() which specifies a matching remote ufrag/password (i.e. consequence is insufficient buffering, over-buffering (DOS attack), or timeouts that drop packets pre-maturely before signalling arrives).

  3. Put IceTransport into a .listen() mode of operation with a pre-wired DTLS transport (i.e. consequence is a larger / more complex ORTC API surface and one that must handle forking scenarios). This idea was briefly mentioned in last ORTC CG meeting.

I personally favour option 2 or 3 options but not 1. If we are going to respond to connectivity checks (as per issue #170 ) then we are giving consent to the remote side to send data. To incorrectly drop incoming data despite giving consent seems wrong.

If we do 3, then we need to be concerned with forking. If Alice sends out a set of offer candidates but multiple answers come back then a single IceTransport in .listen() mode will not be sufficient. We may want to consider having the gatherer in a .listen() mode [where an event is fired to tell of an unknown incoming connectivity consent check from an unknown remote ufrag never seen before) and thus having an IceTransport.accept() to specify where the incoming ICE connectivity checks must be processed.

E.g.

partial interface IceGatherer {
void listen();
attribute EventHandler onunhandledice;
}

dictionary IceUnhandledEventInit : EventInit {
DOMString remoteUFrag;
};

partial interface IceTransport {
//...
void accept(DOMString remoteUFrag);
}

This API could handle forked replies too. This could also allow the connectivity checks to be replied from a particular IceTransport rather than from the IceGatherer (so long as there's a short buffering of incoming ICE connectivity checks exists for unhandled remote ufrag packets). This would also allow for wiring of a DTLS transport to handle the transport before the consent was given to the remote side (although replying to the incoming DTLS client "hello" would not be possible since no reverse ice consent would be granted). In other words, incoming connectivity checks are handled but outgoing connectivity checks are still not possible since the remote password would remain unknown until signalling arrived but at least the DTLS transport could be wired up before any DTLS packets would / could arrive.

So IMHO, it's either buffer all packets until signalling arrives, or do a listen()/accept() scenario modelled after TCP listen()/accept().

[will cross post to ortc CG list and please follow up on list]

@robin-raymond robin-raymond added the 1.1 label Feb 6, 2015

@robin-raymond robin-raymond changed the title DTLS handshake packets arrive because of connectivity checks DTLS handshake packets arrive because of connectivity checks before DTLS transport setup Feb 6, 2015

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Apr 16, 2015

As noted in a recent comment in Issue 170, an IceGatherer has no role, so that it cannot generate a role conflict error. This makes it inadvisable for an IceGatherer to respond to an incoming connectivity check - and an IceTransport cannot do so until iceTransport.start() is called.

As a result, an Answerer can call iceTransport.start and dtlsTransport.start, causing ICE connectivity checks to be started, and DTLS negotiation to begin once a response to a connectivity check is received. Since the latter can only occur once the Offerer receives the Answer and calls iceTransport.start() (probably followed soon after by dtlsTransport.start()), option 2 (buffering) might be acceptable.

@aboba aboba added PR exists and removed PR exists labels Apr 19, 2015

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Apr 19, 2015

The DTLS transport can respond to incoming DTLS packets before start() is called, but we do not say this explicitly. Here is a proposed change to Section 2.3.2:

A newly constructed RTCDtlsTransport MUST listen and respond to incoming DTLS packets before start() is called. However, to complete the negotiation it is necessary to verify the remote fingerprint, which is dependent on remoteParameters, passed to start().

Also, since start() can return an error, this needs to be a Promise.

@aboba aboba added the PR exists label Apr 19, 2015

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Apr 21, 2015

Incoming DTLS packets can cause the DTLS transport state to transition to "connecting" or even "connected" before RTCDtlsTransport.start() is called.

@robin-raymond

This comment has been minimized.

Copy link
Contributor

robin-raymond commented Apr 27, 2015

I'm okay with RTCDtlsTransport needing to be in connected state to mean packets negotiated + validated. I would recommend though that we do not send media until in the connected state but incoming media should be sent through the pipeline as soon as it arrives even if not officially connected to avoid the race condition where one side has validated but the other side has not. We don't want to be discarding incoming packets needlessly which can cause disruption in proper media flow-through.

@robin-raymond

This comment has been minimized.

Copy link
Contributor

robin-raymond commented Apr 29, 2015

To address original issue: What do we do about DTLS packets that can arrive before there's a DTLS transport to receive the DTLS packets?

This is dependant upon #170 . If an RTCIceGatherer can respond to incoming connectivity checks prior to an RTCIceTransport being associated to an RTCDtlsTransport then it is possible incoming DTLS packets to arrive. Here's an example of how this could happen.

var aliceGather = new RTCIceGatherer();
aliceSignal(aliceGatherer.getLocalParameters());

var aliceGathererParams = bobReceiveSignal();
var bobGather = new RTCIceGatherer();
var bobTransport = new RTCIceTransport(bobGatherer);
bobTransport.start(aliceGathererParams);

// alice receives incoming check
aliceGatherer.onremotetransport = function(remoteUsernameFragment) {
  // note: incoming check has arrived, alice associated transport since
  // she doesn't have one yet

  // by associating an IceTransport the incoming connectivity will respond..
  var aliceTransport = new RTCIceTransport(aliceGather);
  // in race possibly before this dtls tranport is setup
  var dtlsTranposrt = new RTCDtlsTransport(aliceTransport);
}

Thus if it's possible to setup a RTCIceTransport AFTER having received the connectivity check will may have to buffer incoming connectivity checks AND DTLS packets.

robin-raymond pushed a commit that referenced this issue May 7, 2015

Robin Raymond
- sender.setTrack() updated to return a Promise, as noted in:
#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

@aboba aboba closed this Oct 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment