Open Bug 1940001 Opened 1 months ago Updated 29 days ago

WebRTC role conflict error response

Categories

(Core :: WebRTC, defect)

Firefox 135
defect

Tracking

()

People

(Reporter: brassevan, Assigned: bwc)

References

Details

Attachments

(3 files)

Attached file WebRTC code

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:135.0) Gecko/20100101 Firefox/135.0

Steps to reproduce:

Create a webrtc connection to a server where both sides are ice-controlled. Firefox starts sending ice connection tests and the server responds with role-conflict error responses.

Actual results:

Firefox doesn't seem to switch roles to controlling. The logs show: received response -> error processing response, retry may be possible 487 multiple times until ice fails.

Expected results:

Chrome switches roles and becomes the controlling peer, which is the correct behavior if I'm understanding the spec: https://datatracker.ietf.org/doc/html/rfc8445#section-7.2.5.1

I've created firefox->firefox webrtc connections with conflicting roles and one peer will switch, but my guess is that this logic is only performed when receiving connection test requests and not when receiving connection test responses. That's my theory.

Lastly, adding an "a=ice-lite" line to the offer causes Firefox to start as the controlling peer and the ice-connection succeeds.

Attached file WebRTC connection logs
Attached file Example Chrome logs

I know the ice role in these logs shows controlling from the beginning, but I believe this is because it switches before the first report or generated.

Byron, would you mind taking a look?

Flags: needinfo?(docfaraday)

I think I see what must be happening here. We have part of nICEr that believes that error responses on ICE checks don't count as putting the STUN context in a failed state:

https://searchfox.org/mozilla-central/source/dom/media/webrtc/transport/third_party/nICEr/src/stun/stun_client_ctx.c#495-504
https://searchfox.org/mozilla-central/source/dom/media/webrtc/transport/third_party/nICEr/src/stun/stun_client_ctx.c#613-615

But the part of nICEr that handles 487 is only called when the STUN context is in some completed state:

https://searchfox.org/mozilla-central/source/dom/media/webrtc/transport/third_party/nICEr/src/stun/stun_client_ctx.c#821-822,829
https://searchfox.org/mozilla-central/source/dom/media/webrtc/transport/third_party/nICEr/src/ice/ice_candidate_pair.c#225-236

It seems that this has been broken since the beginning, and that the only way we ever resolve role conflicts is when we receive a request with the same role as we have. I guess that either this isn't happening here, or the peer is getting the tiebreaker calculation wrong?

Flags: needinfo?(docfaraday)
Assignee: nobody → docfaraday
Status: UNCONFIRMED → NEW
Ever confirmed: true

Deciding "Is this error fatal or not?" solely at the level of the STUN client context is wrong. The STUN client context has to be able to handle a small number of errors (like auth challenges) on its own, but for the overwhelming majority of failures this determination needs to be made by whatever is using the STUN ctx. We must refactor a little here.

The STUN client context should transition to a failed state when receives an error it does not know how to handle, and that subsequent error response handling should be handled by something else (eg; the ICE candidate pair). That means most of this block of code will need to go away (either to other places, or gone entirely):

https://searchfox.org/mozilla-central/source/dom/media/webrtc/transport/third_party/nICEr/src/stun/stun_client_ctx.c#471-540

I imagine that the refactoring here will address bug 1023619.

See Also: → 1023619

The severity field is not set for this bug.
:mjf, could you have a look please?

For more information, please visit BugBot documentation.

Flags: needinfo?(mfroman)

Marking this an S3, but feel free to change.

Severity: -- → S3
Flags: needinfo?(mfroman)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: