Closed Bug 1338384 Opened 5 years ago Closed 5 years ago
TURN responses with bandwidth attr result in failure
No description provided.
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?
Is the attribute advisory or mandatory. If the former, let's just add it to our table of known attributes, parse it, and skip.
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?
Flags: needinfo?(docfaraday) → needinfo?(martin.thomson)
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.
That's probably reasonable.
Just manually confirmed that adding the attribute with the patch makes Firefox work with the viagenie.ca coturn installation from the stackoverflow question.
Comment on attachment 8836210 [details] Bug 1338384: added STUN bandwidth attribute. https://reviewboard.mozilla.org/r/111666/#review112962
Attachment #8836210 - Flags: review?(docfaraday) → review+
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.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/18699bcc1c2c added STUN bandwidth attribute. r=bwc
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
Attachment #8836210 - Flags: approval-mozilla-aurora?
Comment on attachment 8836210 [details] Bug 1338384: added STUN bandwidth attribute. Fix a WebRTC issue. Aurora53+.
Attachment #8836210 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.