Closed Bug 1436555 Opened 8 years ago Closed 7 years ago

Webrtc - Firefox sends stun binding with incorrect use_candidate flag

Categories

(Core :: WebRTC: Signaling, defect, P3)

58 Branch
defect

Tracking

()

RESOLVED DUPLICATE of bug 1238249

People

(Reporter: nzli, Unassigned)

References

Details

(Whiteboard: [need info reporter 2018-02-09][platform-rel-Facebook])

Attachments

(3 files)

Attached file firefox_log.rtf
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:58.0) Gecko/20100101 Firefox/58.0 Build ID: 20180128191252 Steps to reproduce: Make a Facebook Video call from Firefox to Facebook Messenger mobile on Android on a limited network. Firefox 58 on Mac. Actual results: It's declared successful on Firefox side, but remain not connecting on Facebook Messenger side. Eventually the call drops. On Facebook Messenger side, we found out that use_candidate were not set on the selected pair. Logs from Messenger side. Please note that all pairs except the relay-relay one works; others are one-way due to the network constraints. Received stun binding has use_candidate Conn[audio:en0|Q7YZkKkm:1:0:local:udp:172.22.9.146:59280->zc8X80vA:1:0:local:udp:172.27.180.164:50980|CR-I|3|3000] Received stun binding has use_candidate Conn[audio:en0|i+Cv1o43:1:0:relay:udp:157.240.22.48:47491->zc8X80vA:1:0:local:udp:172.27.180.164:50980|CR-I|4|3000] Received stun binding has use_candidate Conn[audio:en0|Q7YZkKkm:1:0:local:udp:172.22.9.146:59280->zc8X80vA:1:0:local:udp:172.27.180.164:50980|CR-I|4|3000] Received stun binding no use_candidate Conn[audio:en0|i+Cv1o43:1:0:relay:udp:157.240.22.48:47491->NnmHEjaa:1:0:relay:udp:157.240.22.48:46453|CRWS|2|47] Received stun binding has use_candidate Conn[audio:en0|i+Cv1o43:1:0:relay:udp:157.240.22.48:47491->zc8X80vA:1:0:local:udp:172.27.180.164:50980|CR-I|7|3000] Received stun binding no use_candidate Conn[audio:en0|i+Cv1o43:1:0:relay:udp:157.240.22.48:47491->NnmHEjaa:1:0:relay:udp:157.240.22.48:46453|CRWS|2|47] Firefox's connection log is attached. Expected results: Call connects, and the selected pairs Stun binding has use_candidate flag set.
Correction: "all pairs except the relay-relay one works" should be "only the relay-relay pair works, all others are one-way due to network constraints".
Component: Untriaged → WebRTC: Networking
Product: Firefox → Core
Nils, can you help me triage this one?
Component: WebRTC: Networking → WebRTC: Signaling
Flags: needinfo?(drno)
Whiteboard: [need info reporter 2018-02-08]
(In reply to Naizhi from comment #0) > Actual results: > > It's declared successful on Firefox side, but remain not connecting on > Facebook Messenger side. Eventually the call drops. > > On Facebook Messenger side, we found out that use_candidate were not set on > the selected pair. Logs from Messenger side. Please note that all pairs > except the relay-relay one works; others are one-way due to the network > constraints. > > Received stun binding has use_candidate > Conn[audio:en0|Q7YZkKkm:1:0:local:udp:172.22.9.146:59280->zc8X80vA:1:0:local: > udp:172.27.180.164:50980|CR-I|3|3000] > Received stun binding has use_candidate > Conn[audio:en0|i+Cv1o43:1:0:relay:udp:157.240.22.48:47491->zc8X80vA:1:0: > local:udp:172.27.180.164:50980|CR-I|4|3000] > Received stun binding has use_candidate > Conn[audio:en0|Q7YZkKkm:1:0:local:udp:172.22.9.146:59280->zc8X80vA:1:0:local: > udp:172.27.180.164:50980|CR-I|4|3000] > Received stun binding no use_candidate > Conn[audio:en0|i+Cv1o43:1:0:relay:udp:157.240.22.48:47491->NnmHEjaa:1:0: > relay:udp:157.240.22.48:46453|CRWS|2|47] > Received stun binding has use_candidate > Conn[audio:en0|i+Cv1o43:1:0:relay:udp:157.240.22.48:47491->zc8X80vA:1:0: > local:udp:172.27.180.164:50980|CR-I|7|3000] > Received stun binding no use_candidate > Conn[audio:en0|i+Cv1o43:1:0:relay:udp:157.240.22.48:47491->NnmHEjaa:1:0: > relay:udp:157.240.22.48:46453|CRWS|2|47] Sorry but I'm confused what do you think the problem is here. In the text you say "... use_candidate were not set on the selected pair." But this log indicates that every single received binding request has the use_candidate flag set. Which is the expected behavior from Firefox, because it implements the agressive nomination from the ICE RFC https://tools.ietf.org/html/rfc5245#section-8.1.1.2 The other important information missing here is: who is the ICE controller and who is the controlled in these calls? In other words who gets to set the use-candidate flag?
Flags: needinfo?(drno) → needinfo?(nzli)
Whiteboard: [need info reporter 2018-02-08] → [need info reporter 2018-02-09]
Rank: 25
Priority: -- → P3
The last line of the log: Received stun binding no use_candidate Conn[audio:en0|i+Cv1o43:1:0:relay:udp:157.240.22.48:47491->NnmHEjaa:1:0:relay:udp:157.240.22.48:46453|CRWS|2|47] It says "Received stun binding no use_candidate", so no use_candidate flag set. Firefox is the controller. And some stun bindings from Firefox came with use_candidate flag, but on the wrong connection.
Flags: needinfo?(nzli)
Please note that Firefox 58 has a significantly lower connect success rate than 57 for Facebook video calls. So please use this information when determining priorities.
Facebook(we) is rolling out a new client code on web browsers. The major difference between the two clients is that we will make video stream on demand, which means the audio call needs to go through sdp update to add video when user request for video. Now we have difficulties to make Firefox works (we cleared out all issues on Chrome), the issues in high level come in two parts: 1. video call from Firefox to our mobile client breaks on video (And it is what the previous conversation in this bug report trying to address) 2. cannot update by adding video stream, the call will fail. (We thought this was associated to Bug 1425618. (but it didn't fix this issue) TBH, there could be something off on our side, but we need your help to follow up and work together with us to bring in the new product.
We're probably going to need a packet capture to sort out what might be going wrong here.
Whiteboard: [need info reporter 2018-02-09] → [need info reporter 2018-02-09][platform-rel-Facebook]
(In reply to Byron Campen [:bwc] from comment #7) > We're probably going to need a packet capture to sort out what might be > going wrong here. Assuming I know nothing, how can I capture the packet you need. Naizhi has already submitted a file includes the log from about:webrtc, is that helpful?
(In reply to agener917 from comment #8) > (In reply to Byron Campen [:bwc] from comment #7) > > We're probably going to need a packet capture to sort out what might be > > going wrong here. > > Assuming I know nothing, how can I capture the packet you need. Naizhi has > already submitted a file includes the log from about:webrtc, is that helpful? Wireshark is the usual tool for capturing packets from the network. I would need a full capture of the call to know exactly what is going on here. Naizhi, can you supply this?
Flags: needinfo?(nzli)
Would it be possible to debug the issue by looking at the webrtc logs attached? I have got rid of my environment already and it will take us a lot of time to set things up to repro the issue and do the capturing. I really appreciate it.
Flags: needinfo?(nzli)
(In reply to Naizhi from comment #10) > Would it be possible to debug the issue by looking at the webrtc logs > attached? I have got rid of my environment already and it will take us a lot > of time to set things up to repro the issue and do the capturing. I really > appreciate it. The logs are not telling me enough to diagnose the issue. According to your logs, the STUN checks that do not have USE-CANDIDATE are the ones that involve relay candidates at both ends. I suspect that something in the relays is confusing things, so a packet capture is necessary to diagnose.
unzip and there is a readme for some context.
So I've looked at repro-case2-audio-only, and we are sending USE-CANDIDATE on all of our ICE checks, including the ones in our send indications (ie; the ICE checks that are being relayed through the TURN server). The relay/relay candidate pair succeeded (there is a successful STUN check in both directions), and it is nominated also, so everything looks fine as far as ICE is concerned. It is possible that the problem isn't ICE, but rather the DTLS handshake. I don't see either side sending a client hello, and the answer does not have an a=setup attribute. The answerer is supposed to send the client hello in webrtc, and our code seems to assume this when the a=setup attribute is missing (which doesn't seem to agree with https://tools.ietf.org/html/rfc4145#section-4.1). Maybe that's the source of the problem?
@drno, this code assumes that the absence of an a=setup attribute in an answer means the answerer is active: https://searchfox.org/mozilla-central/source/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp#654 However, https://tools.ietf.org/html/rfc4145#section-4.1 says this means the answerer is passive instead. This is a bug, right?
Flags: needinfo?(drno)
I haven't looked at the logs, but yes we have bug 1329028 regarding improving Firefox handling of the a=setup attribute. The specs say pretty clearly that by default the answers assume the DTLS active role to save time, and that is what our current code assume is absence of a=setup. To avoid confusion like this here I think it might actually be safer to reject any SDP w/o a=setup in it. Typical signs of DTLS problems is that ICE on about:webrtc will show a successful connection. But no RTP what so ever gets exchanged.
Flags: needinfo?(drno)
See Also: → 1329028
(In reply to Nils Ohlmeier [:drno] from comment #15) > I haven't looked at the logs, but yes we have bug 1329028 regarding > improving Firefox handling of the a=setup attribute. > > The specs say pretty clearly that by default the answers assume the DTLS > active role to save time, and that is what our current code assume is > absence of a=setup. Where does it say that? The base spec says the opposite, so even if the JSEP spec said it was active I don't think that would be valid protocol design.
From https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-24#section-5.3.1 An "a=setup" line, as specified in [RFC4145], Section 4, and clarified for use in DTLS-SRTP scenarios in [RFC5763], Section 5. The role value in the answer MUST be "active" or "passive". When the offer contains the "actpass" value, as will always be the case with JSEP endpoints, the answerer SHOULD use the "active" role. Offers from non-JSEP endpoints MAY send other values for "a=setup", in which case the answer MUST use a value consistent with the value in the offer. With "base spec" you are referring to the RFC5763? But even in https://tools.ietf.org/html/rfc5763#section-5 I see the recommendation for the answerer to use 'active': The endpoint MUST use the setup attribute defined in [RFC4145]. The endpoint that is the offerer MUST use the setup attribute value of setup:actpass and be prepared to receive a client_hello before it receives the answer. The answerer MUST use either a setup attribute value of setup:active or setup:passive. Note that if the answerer uses setup:passive, then the DTLS handshake will not begin until the answerer is received, which adds additional latency. setup:active allows the answer and the DTLS handshake to occur in parallel. Thus, setup:active is RECOMMENDED. Whichever party is active MUST initiate a DTLS handshake by sending a ClientHello over each flow (host/port quartet).
I'm referring to this text from RFC 4145: The default value of the setup attribute in an offer/answer exchange is 'active' in the offer and 'passive' in the answer.
Has the issue been identified? Any suggestions for the next steps?
(In reply to agener917 from comment #19) > Has the issue been identified? Any suggestions for the next steps? You need to make sure your endpoint is putting an a=setup attribute in its SDP. Ideally, you would want to use a=setup:active if you are the answerer, provided your endpoint can handle being the client in the DTLS handshake.
Attached file package-debug-fb.zip
Update: We tried to set the DTSL roles, but the call still fails. Looking further into the problem, the failure happens in earlier point. What Naizhi has mentioned before is about 'use_candidate' attribute missing in the STUN message, and we would like to get us together solve this issue first before we move forward to other issues. Basically, once Firefox selects the ICE candidate, our mobile client expect the 'use_candidate' attribute is set in the STUN package via the selected ICE. However, we found this attribute is missing, i.e. the last package sent from Firefox. Attached has a PDF demonstrate our debugging: 'use_candidate' [00 25] is missing in the last package.
I believe what you're seeing there is consent freshness STUN requests, not ICE STUN requests (https://tools.ietf.org/html/rfc7675). Consent freshness checks do not carry a USE-CANDIDATE attribute. The nomination comes earlier on.
^
Flags: needinfo?(agener917)
Firefox is using aggressive nomination outlined here https://tools.ietf.org/html/rfc5245#page-52 I believe that using aggressive nomination, the consent freshness checks also should have USE-CANDIDATE set. This gives the controlling side full control in cases like wifi/cell considerations. For this purpose, Messenger follows the latest Stun binding requests with USE-CANDIDATE flag set. In our repro, Firefox and Messenger has one way communication except relay to relay. Using ConnectionR to denote this selected pair, and ConnectionF as the another one way pair, this is what happened: Messenger received Stun binding from ConnectionR with USE-CANDIDATE, but the pair is in-progress. Messenger received Stun binding from ConnectionF with USE-CANDIDATE, but the pair is in-progress. Messenger received Stun binding response from ConnectionR, the pair is successful, but the latest connection selected is ConnectionF. Messenger received Stun binding from ConnectionR, this time without USE-CANDIDATE flag, so Messenger does not follow. PS I found that WebRTC in Chrome solved the problem by adding another attribute: https://tools.ietf.org/html/draft-thatcher-ice-renomination-00 Is Firefox planning to support that?
(In reply to Naizhi from comment #24) > Firefox is using aggressive nomination outlined here > https://tools.ietf.org/html/rfc5245#page-52 > > I believe that using aggressive nomination, the consent freshness checks > also should have USE-CANDIDATE set. This gives the controlling side full > control in cases like wifi/cell considerations. For this purpose, Messenger > follows the latest Stun binding requests with USE-CANDIDATE flag set. That's not what the spec says. Also, if Messenger is treating STUN requests without USE-CANDIDATE as un-nominating previously nominated candidates, that would be a bug in Messenger, because that's not how ICE nomination works in general.
(In reply to Naizhi from comment #24) > Messenger received Stun binding from ConnectionR with USE-CANDIDATE, but the > pair is in-progress. > > Messenger received Stun binding from ConnectionF with USE-CANDIDATE, but the > pair is in-progress. > > Messenger received Stun binding response from ConnectionR, the pair is > successful, but the latest connection selected is ConnectionF. USE-CANDIDATE just nominates candidate pairs; both sides are supposed to use the highest priority nominated (and successful) pair, not the pair that was nominated most recently.
How is Firefox planning to solve wifi/cell situation? The scenario is that a controlling side switched to Wifi connection, and both wifi and cell are now available. How can the controlled side switch to use the wifi connection?
PS we'll fix it on our side to allow a pair that's nominated only when in-progress.
(In reply to Naizhi from comment #27) > How is Firefox planning to solve wifi/cell situation? The scenario is that a > controlling side switched to Wifi connection, and both wifi and cell are now > available. How can the controlled side switch to use the wifi connection? That's over at bug 1238249. (In reply to Naizhi from comment #28) > PS we'll fix it on our side to allow a pair that's nominated only when > in-progress. The spec requires logic that remembers whether each pair has been nominated, and use the highest priority pair that is both succeeded and nominated. I'm not exactly sure what you're describing here, but it does not sound like that.
I looked at bug 1238249. It's awesome that you guys are considering this new continuous ICE which is very important for mobile and interop with mobile clients. A quote from that document: "If Binding Requests need to be sent for any other reason, such as consent checks [I-D.ietf-rtcweb-stun-consent-freshness], any checks sent on the selected pair MUST include a USE-CANDIDATE attribute." Would you consider front-loading this part sooner? This will enable better interop with our existing clients. This will be a mutual win for both of us, so I sincerely hope you consider it.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Flags: needinfo?(agener917)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: