1338384 - TURN responses with bandwidth attr result in failure

Status: RESOLVED FIXED

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

Comment 1

[Nils Ohlmeier [:drno]]

Apparently the Bandwidth attribute falls into the category of comprehension required attributes. Therefore when we receive that attribute, which we don't support, we mark the transaction as failed and try again until the TURN client transaction times out.

I don't like that this apparently causes interop issues: http://stackoverflow.com/questions/42120698/webrtc-missing-relay-candidates-in-firefox
On the other hand the server sends us a "reserved" attribute (0x10) according to IANA http://www.iana.org/assignments/stun-parameters/stun-parameters.xhtml#stun-parameters-4 which falls into the category o comprehension required.

Byron, Ekr: what are your thoughts on this?

Comment 2

[Eric Rescorla (:ekr)]

Is the attribute advisory or mandatory. If the former, let's just add it to our table of known attributes, parse it, and skip.

Comment 3

[Byron Campen [:bwc]]

So, looking at draft-thomson-mmusic-rtcweb-bw-consent-00 (which is way expired, btw), that attribute is comprehension optional. This sounds like the other agent is just flat out broken in at least two different ways.

Martin, since you worked on the spec on BANDWIDTH, what's your take?

Comment 4

[Nils Ohlmeier [:drno]]

Byron I think you meant draft-thomson-tram-turn-bandwidth-01 which is indeed way expired.
But it is a good point that the draft specifies the attribute to be comprehension-optional. Which appears to be wrong to me, because with type 0x0010 it is in the 0x0000-0x7FFF range for comprehension-required from RFC 5389 https://tools.ietf.org/html/rfc5389#section-15.

IMHO the attribute should have been put in the > 0x8000 range by the draft.
Since the attribute apparently is used in deployed (spec violating?) servers I guess the best we can do is what Ekr suggested to add it to the list of known attributes and skip it.

Comment 5

[Byron Campen [:bwc]]

That's probably reasonable.

Comment 6

[Nils Ohlmeier [:drno]]

Attachment: Bug 1338384: added STUN bandwidth attribute.

Review commit: https://reviewboard.mozilla.org/r/111666/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111666/

Comment 7

[Nils Ohlmeier [:drno]]

Just manually confirmed that adding the attribute with the patch makes Firefox work with the viagenie.ca coturn installation from the stackoverflow question.

Comment 8

[Byron Campen [:bwc]]

Comment on attachment 8836210 [details]
Bug 1338384: added STUN bandwidth attribute.

https://reviewboard.mozilla.org/r/111666/#review112962

Comment 9

[Martin Thomson [:mt:]]

Yep, seems like you have the answer you need.  And damn, I can barely remember that.  The reason I chose that codepoint is that Microsoft were using that specific value in their TURN implementation.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18699bcc1c2c
added STUN bandwidth attribute. r=bwc

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/18699bcc1c2c

Comment 12

[Gerry Chang [:gchang]]

Too late for 51. Mark 51 won't fix.

Comment 13

[Nils Ohlmeier [:drno]]

Comment on attachment 8836210 [details]
Bug 1338384: added STUN bandwidth attribute.

Approval Request Comment
[Feature/Bug causing the regression]: Not a regression. Problem exist since we Firefox supports WebRTC.
[User impact if declined]: Turn allocations with some WebRTC services fails, so calls might not get established.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It is pretty low risk as the patch only whitelists a single attribute ID in our STUN parser.
[String changes made/needed]: N/A

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8836210 [details]
Bug 1338384: added STUN bandwidth attribute.

Fix a WebRTC issue. Aurora53+.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/244f8f9fdc51