1769994 - On systems with IPv6 preferred DNS resolution clients will fail to connect when "localhost" is used as host for the WebSocket server

Status: RESOLVED FIXED

(Remote Protocol :: Agent, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

If on the system there is a IPv6 only stack (or DNS resolves IPv6 first) any WebSocket client would not be able to connect to the Remote Agent. This is because we start the httpd.js server on IPv4 only.

Note that when changing that to start_ipv6() any IPv4 connection attempt is going to be refused.

Ideally when starting on IPv6 there should be a way to unset the IPV6_V6ONLY flag and allow bindings for "any" address. But would that open any risks especially for the "any" addresses part?

As such we might have to figure out which interface resolves first and call start() or start_ipv6() instead when starting the HTTP server.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Note that since node.js v17 the built-in DNS resolver defaults to IPv6 now and that means that no WebSocket client will be able to connect to our Remote Agent socket. A workaround would be to force IPv4 by using the --dns-result-order argument for node, but not sure how easily this can be done via npm.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #0)

As such we might have to figure out which interface resolves first and call start() or start_ipv6() instead when starting the HTTP server.

Whereby this can be application / tool specific so nothing we can build our assumption on. :(

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Bug 1769994 will handle the underlying issue in httpd.js.

So in this bug we should discuss which options we have and how important this issue potentially could be. Not sure on which platforms a dual-stack is actually enabled by default. But as long as a workaround exists it might still be a P3?

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I'll have a look this week if the mentioned workaround would work for now or if we need a ServerSocket change.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I've taken a look and the easiest solution for now might be to use two instances of the HTTPServer for now. By trying to bind to IPv6 first we would get a socket on that interface if available, and if yes maybe for IPv4 as well - depending on the IPV6_V6ONLY flag setting. We could try to bind to IPv4 as well afterward to verify that by checking if it fails.

Important would be that we raise an error if we cannot bind to at least one, or the same port is not available for both interfaces.

That said it would need testing across platforms to see how the default behavior will look like. If it turns out problematic we could have a preference which would allow the selection (IPv4 only, IPv6 only, both).

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I'm going to wait until the network team has discussed the request on bug 1770019 and set their own priority. If a solution could be implemented in the networking layer pretty quickly we wouldn't need the workaround solution as posted in comment 5.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Please note that for Puppeteer the following PR got landed which enabled support for node.js >16 and defaults to IPv4 by default:
https://github.com/puppeteer/puppeteer/pull/8447

As such it's not a deal breaker at least for Puppeteer but we should still fix it for possible BiDi clients, but that's not that urgent now.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

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

For more information, please visit auto_nag documentation.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

A patch has been uploaded to the dependent bug and after some testing today it looks great! I'll do more testing and if it all works great we should be able to get this bug fixed pretty soon. Once done and the code is in a Nightly, we should revert the Firefox specific changes from https://github.com/puppeteer/puppeteer/pull/8447.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Today I stumbled over https://github.com/cyrus-and/chrome-remote-interface/issues/467 and actually saw that we incorrectly print the WebSocket end-points to stderr and to the DevToolsActivePort file. Instead of using the resolved IP address we use localhost, which then leaves it up the client in how to resolve the address. With a dualstack setup this can indeed cause issues if we resolve to IPv4 but the client to IPv6.

I would suggest that we do not take advantage of the dualstack feature of httpd.js but instead print the resolved IP address.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Not sure why but when implementing this patch I noticed bug 1774476, which actually blocks me from running any browser chrome test locally.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1769994 - [wdspec] Re-enable WebDriver BiDi after CDP-only test.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1769994 - [wdspec] Allow to test WebSocket connections for both "127.0.0.1" and "localhost".

Depends on D154148

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1769994 - [remote] Set final host and port for chrome-remote-interface client.

Depends on D154149

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1769994 - [remote] Resolve localhost to an IP before starting httpd.js.

By resolving localhost to an actual IP for the httpd.js server
allows supplying unique URLs for the CDP and WebDriver BiDi
WebSocket connection details and CDP's JSON handler end-points.

Hereby the first resolved IP will be used which in nearly all
the cases will be the local IPv4 address (127.0.0.1).

It will prevent issues especially on dualstack systems where DNS
resolution might prefer IPv6 over IPv4 (like with node.js > 16).

Depends on D154150

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Currently I'm blocked on bug 1783938. Moving bug's status to new for now.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

A solution has finally been found. See bug 1783938 comment 12. This works reliable on my local system for both IPV4 and IPv6 whereby we would only use the IPV6 address if no IPv4 interface would exist. And this should actually never be the case.

As a follow-up and if there is such a request from clients we could introduce a new preference to optionally switch to IPv6 by default. But that's not part of this bug and I wouldn't suggest to do it unless we get such a request.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3897e519d284
[wdspec] Re-enable WebDriver BiDi after CDP-only test. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/752cf2c2dd9a
[wdspec] Allow to test WebSocket connections for both "127.0.0.1" and "localhost". r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/831794113546
[remote] Set final host and port for chrome-remote-interface client. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/dae94303a2df
[remote] Resolve localhost to an IP before starting httpd.js. r=webdriver-reviewers,jdescottes

Comment 19

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/3897e519d284
https://hg.mozilla.org/mozilla-central/rev/752cf2c2dd9a
https://hg.mozilla.org/mozilla-central/rev/831794113546
https://hg.mozilla.org/mozilla-central/rev/dae94303a2df

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

For the removal of the workaround in Puppeteer I've created a PR:
https://github.com/puppeteer/puppeteer/pull/8825

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The upstream PR has been merged! Everything works as expected now.