Potential interop issue: Constructing a DtlsTransport from an existing IceTransport #168

Closed
aboba opened this Issue Dec 11, 2014 · 9 comments

Projects

None yet

5 participants

@aboba
Contributor
aboba commented Dec 11, 2014

From Roman Shpount (see http://lists.w3.org/Archives/Public/public-ortc/2014Dec/0028.html):

All the tools needed to shoot oneself in the foot with the current WebRTC implementation are ready and provided to the ORTI API user.

A developer could construct a new RTCDtlsTransport based on an existing RTCIceTransport and then call RTCDtlsTransport.start(). This would cause a new DTLS negotiation to occur and the DTLS role, fingerprints, etc. could change with a new DTLS/SRTP key being derived.

Existing implementations will only expect a new DTLS session negotiation if transport parameters have changed. They will not handle a new DTLS session over the existing ICE transport connection and they have no way to demultiplex two DTLS sessions over the same ICE connection.

I do not think this needs to be specifically disabled since there are valid use cases when such functionality is needed to interwork with valid DTLS and DTLS-SRTP implementations.

@aboba aboba added the 1.0 label Dec 11, 2014
@aboba
Contributor
aboba commented Dec 12, 2014

An onerror event is needed to deal with the case where two DTLS connections are attempting to be multiplexed over a single ICE connection.

@juberti
juberti commented Dec 12, 2014

Also, if DTLSTransport A is talking to DTLSTransport B, and then A goes away, this should tear down the DTLS association, so that a new DTLSTransport C can reconnect to B.

@robin-raymond
Contributor

Just to clarify from B's perspective, since the DTLS associated with B is torn down it's possible to initiate a new DTLS session for B. However, no new DTLS session can be initiated so long as the current DTLS transport is still active. Or is this a brand new IceTransport for B for the connection to C thus the new DTLS exchange is not replacing a previous torn down DTLS exchange from A to B?

If the former, what happens to the RTP sender / receivers still using the current SRTP keys derived from the existing DTLS exchange when the DTLS session gets torn down? Does the SRTP keying material change for B only when the new DTLS session is established?

@rshpount
rshpount commented Jan 6, 2015

A new DTLS session would need to be negotiated for each new ICE ufrag. SRTP
keys should be negotiated via each new DTLS session and continue to be used
while this session is running. The place where things get tricky is when
ICE parameters stay the same, but new DTLS session is negotiated for some
reason.

I am not sure what exactly is meant by "A goes away". Does it mean DTLS
transport change with the same ICE transport or entirely new ICE and DTLS
transports.


Roman Shpount

On Tue, Jan 6, 2015 at 1:01 PM, Robin Raymond notifications@github.com
wrote:

Just to clarify from B's perspective, since the DTLS associated with B is
torn down it's possible to initiate a new DTLS session for B. However, no
new DTLS session can be initiated so long as the current DTLS transport is
still active. Or is this a brand new IceTransport for B for the connection
to C thus the new DTLS exchange is not replacing a previous torn down DTLS
exchange from A to B?

If the former, what happens to the RTP sender / receivers still using the
current SRTP keys derived from the existing DTLS exchange when the DTLS
session gets torn down? Does the SRTP keying material change for B only
when the new DTLS session is established?


Reply to this email directly or view it on GitHub
#168 (comment).

@aboba
Contributor
aboba commented Jan 13, 2015

[BA] We can indicate that only one DtlsTransport can run over an IceTransport. So DtlsTransport.stop() MUST be called prior to calling DtlsTransport.start() on an DtlsTransport constructed from the same iceTransport. However, this does not cover all corner cases.

Justin said: "Also, if DTLSTransport A is talking to DTLSTransport B, and then A goes away, this should tear down the DTLS association"

[BA] If A calls DtlsTransport.stop(), then the DTLS association will be torn down on both A and B. However, if there is a connectivity failure, there may be a period before B tears down the association. Although there has been previous discussion of DTLS Heartbeat [RFC6520](see http://www.ietf.org/mail-archive/web/rtcweb/current/msg10224.html), this document is not referenced in any RTCWEB drafts, including draft-ietf-rtcweb-security, draft-ietf-rtcweb-security-arch, data channel drafts, etc. So if there is no data channel in use, B may not detect that DtlsTransport A has "gone away" until STUN consent freshness fails.

At that point, doesn't B need to construct a new ICE transport, rather than just calling iceTransport.start() with a newly constructed iceGatherer?

@aboba
Contributor
aboba commented Jan 13, 2015

Correction: RFC 6520 is mandated in draft-ietf-tsvwg-sctp-dtls-encaps, in connection with PMTU discovery and probe packets. If this is extended to general use, then connectivity loss could be detected at the DTLS layer.

If not, then connectivity could be restored in ICE either via re-start which would change ufrag/password, or by continuous gathering, which would not.

@robin-raymond
Contributor

My $0.02 - if .start() is called on a new DTLS transport where there was a previous DTLS transport associated to the same underlying ICE transport, if .stop() was not called on the previous DTLS transport an implicit .stop() should be called rather than throwing an error. There's no way to demux two DTLS transports so let's not pester with needless errors where there's no recovery and we can understand what is the intention anyway...

@robin-raymond robin-raymond pushed a commit that referenced this issue Jan 22, 2015
Robin Raymond Updated Section 8.3 to reflect proposed matching rules, reflecting:
#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
a927ecf
@pthatcherg

I disagree. If the JS does something that won't work, we should throw an
error and let the developer fix it. Guessing what the JS intended is too
risky.

On Tue, Jan 13, 2015 at 9:11 AM, Robin Raymond notifications@github.com
wrote:

My $0.02 - if .start() is called on a new DTLS transport where there was a
previous DTLS transport associated to the same underlying ICE transport, if
.stop() was not called on the previous DTLS transport an implicit .stop()
should be called rather than throwing an error. There's no way to demux two
DTLS transports so let's not pester with needless errors where there's no
recovery and we can understand what is the intention anyway...


Reply to this email directly or view it on GitHub
#168 (comment).

@aboba
Contributor
aboba commented Jan 29, 2015

Based on the discussion at the ORTC CG meeting on January 27, the consensus was to throw an InvalidState exception.

@robin-raymond robin-raymond pushed a commit that referenced this issue Mar 25, 2015
Robin Raymond - Added Section 8.3.1 on RTP matching rules, as noted in: Issue #48
- 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
b03c129
@aboba aboba closed this Apr 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment