Closed Bug 1338384 Opened 5 years ago Closed 5 years ago

TURN responses with bandwidth attr result in failure


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




Tracking Status
firefox51 --- wontfix
firefox52 --- wontfix
firefox53 --- fixed
firefox54 --- fixed
Blocking Flags:


(Reporter: drno, Assigned: drno)



(1 file)

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:
On the other hand the server sends us a "reserved" attribute (0x10) according to IANA which falls into the category o comprehension required.

Byron, Ekr: what are your thoughts on this?
Flags: needinfo?(ekr)
Flags: needinfo?(docfaraday)
backlog: --- → webrtc/webaudio+
Rank: 19
Is the attribute advisory or mandatory. If the former, let's just add it to our table of known attributes, parse it, and skip.
Flags: needinfo?(ekr)
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

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 coturn installation from the stackoverflow question.
Comment on attachment 8836210 [details]
Bug 1338384: added STUN bandwidth attribute.
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.
Flags: needinfo?(martin.thomson)
Pushed by
added STUN bandwidth attribute. r=bwc
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Too late for 51. Mark 51 won't fix.
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.