1526253 - Datachannel.id uses an invalid id when you create a new one and there is a negotiation

Status: NEW

(Core :: WebRTC: Networking, defect, P2)

Description

[Raul]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36

Steps to reproduce:

I've created a jsfiddle with the steps.

1. Create 2 peerConnections
2. Negotiate both
3. Create a datachannel
4. Renegotiate again
5. Repeat 3, 4 several times

https://jsfiddle.net/rbenitez/xp6vdzmb/9/

Actual results:

When you reproduce those steps, datachannel.id uses an invalid id, 65535, and it's incorrect.

Looks like when the peerConnection creates a data channel using the id 65535 which is invalid and it is reserved by SCTP for the SCTP INIT message, that's why I think that it is screwing up the whole SCTP stack.

Basically, those stream ids are picked by client and server based on the SCTP role as defined here https://tools.ietf.org/html/draft-ietf-rtcweb-data-protocol-09] for in-band negotiation (Section 4):

...
To avoid collisions where both sides try to open a Data Channel with
the same Stream Identifiers, each side MUST use Streams with either
even or odd Stream Identifiers when sending a DATA_CHANNEL_OPEN
message. When using SCTP over DTLS
[I-D.ietf-tsvwg-sctp-dtls-encaps], the method used to determine which
side uses odd or even is based on the underlying DTLS connection
role: the side acting as the DTLS client MUST use Streams with even
Stream Identifiers, the side acting as the DTLS server MUST use
Streams with odd Stream Identifiers.
...
Available range for stream ids are defined here:

https://tools.ietf.org/html/draft-ietf-rtcweb-data-protocol-09 [3]:

Stream: A unidirectional stream of an SCTP association. It is
uniquely identified by an SCTP stream identifier (0-65534). Note:
the SCTP stream identifier 65535 is reserved due to SCTP INIT and
INIT-ACK chunks only allowing a maximum of 65535 Streams to be
negotiated (0-65534).
Some aditional references regarding the stream id are included in the PeerConnection API as well:

https://www.w3.org/TR/webrtc/#peer-to-peer-data-api
createDataChannel [13]:

If [[DataChannelId]] is equal to 65535, which is greater than the maximum allowed ID of 65534 but still qualifies as an unsigned short, throw a TypeError.NOTE
If the [[DataChannelId]] slot is null after this step, it will be populated once the DTLS role is determined during the process of setting an RTCSessionDescription.

Expected results:

All datachannels should have id's valid.

Comment 1

[Raul]

And here another jsfiddle without renegotiations:
https://jsfiddle.net/sancane/v1uzo2fy/134/

And here, the datachannels's id grows as expected.

Comment 2

[Nils Ohlmeier [:drno]]

Thank you for the really good fiddles. That helps a lot.

So it looks like if the data channel gets created with the initial offer/answer everything works fine. But if the data channel only gets added through a sub-sequent renegotiation then the ID gets into this bad state after which no further data channels can be added any more.

Dan could you please have a look at this?

Comment 3

[Dan Minor [:dminor]]

Sure thing.

Comment 4

[Byron Campen [:bwc]]

It looks like this bug is at least partly responsible for this wpt failure:

https://searchfox.org/mozilla-central/rev/0d9b215518e2004038524fafef06c8522f9823d4/testing/web-platform/meta/webrtc/RTCDataChannel-id.html.ini#3

When I run that test, I see the error "assert_not_equals: got disallowed value 65535".

I think those tests get the even/odd requirement backwards too, FWIW.