Closed
Bug 1189198
Opened 9 years ago
Closed 9 years ago
UDP stun client sends messages to TCP STUN server
Categories
(Core :: WebRTC: Networking, defect, P1)
Core
WebRTC: Networking
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: drno, Assigned: drno)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
No description provided.
Assignee | ||
Updated•9 years ago
|
Rank: 20
Assignee | ||
Comment 1•9 years ago
|
||
Bug 1189198: don't start STUN transactions with a protocol mis-match
Attachment #8640957 -
Flags: review?(docfaraday)
Comment 2•9 years ago
|
||
https://reviewboard.mozilla.org/r/14405/#review13019 ::: media/mtransport/third_party/nICEr/src/ice/ice_candidate.c:556 (Diff revision 1) > if(cand->stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR) { > if(cand->base.ip_version != cand->stun_server->u.addr.ip_version) { > r_log(LOG_ICE, LOG_INFO, "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate with different IP version (%u) than STUN/TURN server (%u).", cand->label,cand->base.ip_version,cand->stun_server->u.addr.ip_version); > ABORT(R_NOT_FOUND); /* Same error code when DNS lookup fails */ > } > > + if(cand->base.protocol != cand->stun_server->u.addr.protocol) { > + r_log(LOG_ICE, LOG_INFO, "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate with different transport (%u) than STUN/TURN server (%u).", cand->label,cand->base.protocol,cand->stun_server->u.addr.protocol); > + ABORT(R_NOT_FOUND); /* Same error code when DNS lookup fails */ > + } > + Is there a reason not to use nr_transport_addr_cmp() here and above, rather than reaching into the structs? That's kind of the intention.
Comment 3•9 years ago
|
||
Comment on attachment 8640957 [details] MozReview Request: Bug 1189198: don't start STUN transactions with a protocol mis-match https://reviewboard.mozilla.org/r/14405/#review13037 Looks good to me, although using nr_transport_addr_cmp is a good idea.
Attachment #8640957 -
Flags: review?(docfaraday) → review+
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8640957 [details] MozReview Request: Bug 1189198: don't start STUN transactions with a protocol mis-match Bug 1189198: don't start STUN transactions with a protocol mis-match
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Eric Rescorla (:ekr) from comment #2) > Is there a reason not to use nr_transport_addr_cmp() here and above, rather > than reaching into the structs? That's kind of the intention. No other excuse then being lazy and copying the if condition from before. Replaced both with a single call to nr_transport_addr_cmp().
Comment 6•9 years ago
|
||
https://reviewboard.mozilla.org/r/14405/#review13113 ::: media/mtransport/third_party/nICEr/src/ice/ice_candidate.c:557 (Diff revision 2) > - if(cand->base.ip_version != cand->stun_server->u.addr.ip_version) { > - r_log(LOG_ICE, LOG_INFO, "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate with different IP version (%u) than STUN/TURN server (%u).", cand->label,cand->base.ip_version,cand->stun_server->u.addr.ip_version); > + if(nr_transport_addr_cmp(&cand->base,&cand->stun_server->u.addr,NR_TRANSPORT_ADDR_CMP_MODE_PROTOCOL)) { > + r_log(LOG_ICE, LOG_INFO, "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate because of IP version/transport mis-match with STUN/TURN server (%u/%u - %u/%u).", cand->label,cand->base.ip_version,cand->base.protocol,cand->stun_server->u.addr.ip_version,cand->stun_server->u.addr.protocol); It would be nicer if this message indicated the protocols by string rather than by number, but this is acceptable as-is. Also, I wonder if this should be at DEBUG?
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8640957 [details] MozReview Request: Bug 1189198: don't start STUN transactions with a protocol mis-match Bug 1189198: don't start STUN transactions with a protocol mis-match
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/20e92855a2e7
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•