Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also listen to IPv6 by default #3347

Closed
zargony opened this issue Oct 16, 2023 · 17 comments · Fixed by #3348 or #3362
Closed

Also listen to IPv6 by default #3347

zargony opened this issue Oct 16, 2023 · 17 comments · Fixed by #3348 or #3362
Assignees
Labels
enhancement New feature or request

Comments

@zargony
Copy link

zargony commented Oct 16, 2023

Is your feature request related to a problem? Please describe.
zwave-js-ui listens to 0.0.0.0:8091 by default, which is IPv4 only, so it doesn't respond to IPv6 requests.

Describe the solution you'd like
It would be nice to listen to IPv6 as well by default since IPv6 is widely used everywhere nowadays. All that's needed is bind to :: instead of 0.0.0.0 (e.g. by setting HOST="::" in the current version). Imho, this should be the default. (Generally, it works well binding to :: as this listens on IPv6 and IPv4 on dual stack systems. Not sure if there are systems out there that would bind to IPv6 only with ::)

Describe alternatives you've considered
The alternative to listen to IPv6 only by default would be possible but maybe a step too far for now ;)

@zargony zargony added the enhancement New feature or request label Oct 16, 2023
robertsLando added a commit that referenced this issue Oct 16, 2023
@robertsLando
Copy link
Member

robertsLando commented Oct 16, 2023

@zargony I'm wondering if this could create any kind of problems in docker container? I mean :: should extend the previous 0.0.0.0 hosts, right?

@zargony
Copy link
Author

zargony commented Oct 16, 2023

@robertsLando Yes, I suppose. I actually discovered it running zwave-js-ui in k8s with IPv6 only while it previously worked in my dual stack k8s. So it definitely works in containers. From what I understand, binding 0.0.0.0 always binds IPv4 only while binding :: usually binds IPv6 and IPv4. Details might depend on the library that parses the address and/or the library that actually binds the socket via libc. Not sure which part exactly is responsible for choosing IPv6 and IPv4 when you give ::, though.

Btw, the same goes for the websocket server on port 3000. It binds to IPv4 only 0.0.0.0 by default until setting "Server Host" under "Home Assistant" in the settings to ::.

@robertsLando
Copy link
Member

Ok let me do some checks

@Jypy
Copy link

Jypy commented Oct 18, 2023

FYI this affect hosts with IPv6 support disabled.
The fix is to set the HOST environment (back) to 0.0.0.0, in case someone else run into the issue :)

@robertsLando
Copy link
Member

@Jypy Do you mean that if ipv6 is disabled this would prevent the UI to be reached?

@Jypy
Copy link

Jypy commented Oct 18, 2023

yep, actually, it crashes because it can't bind to ::

@robertsLando
Copy link
Member

robertsLando commented Oct 18, 2023

Oh that's bad... Let's see if this is a recurring issue I could do a revert in case. I dunno if there are more that benefit from this change then the ones that have problems. Thanks for your report!

@Jypy
Copy link

Jypy commented Oct 18, 2023

I dunno if there are more that benefit from this change then the ones that have problems.

No idea, you'll soon be aware if that's the case.

Thanks for your report!

No problem, happy to help!
Thanks for the app/tool :)

@kpine
Copy link
Contributor

kpine commented Oct 18, 2023

According to server.listen docs (emphasis mine):

If host is omitted, the server will accept connections on the unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise.

Maybe that would support both cases?

@cloud-aware
Copy link

just an FYI - this change broke my docker-compose environment where I have ipv6 disabled. I had to fix by setting an environment variable of HOST and setting it to the value of the container IP

For some reason setting HOST back to 0.0.0.0 did not fix it either

@robertsLando
Copy link
Member

According to server.listen docs (emphasis mine):

If host is omitted, the server will accept connections on the unspecified IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address (0.0.0.0) otherwise.

Maybe that would support both cases?

This maybe a good alternative! I will try, sorry for the issues guys

@Rickerdo
Copy link

My rock solid install would not restart after upgrading to 9.2.1 because of this issue. Even downgrading to 9.2.0 didn't help. Per Skullduggeryism's post, I added "- HOST=0.0.0.0" to my docker-compose.yml file, did an upgrade to 9.2.1, and all is well again.

FWIW - I also have IPv6 disabled through the kernel because I have no need for IPv6 on my LAN.

@robertsLando
Copy link
Member

robertsLando commented Oct 19, 2023

Can someone of you give a try to: #3362 ?

I have triggered this workflow: https://github.com/zwave-js/zwave-js-ui/actions/runs/6571215103

Once it ends, you can use docker test tag

@robertsLando
Copy link
Member

robertsLando commented Oct 19, 2023

@zargony could you confirm me that test tag works for ipv6 too? I got confirmation that it's working for the once that had the problem described above but dunno if still works for you.

I tested locally and seems that I can reach my instance using https://[::1]:8091

@zargony
Copy link
Author

zargony commented Oct 19, 2023

@robertsLando Using image zwavejs/zwave-js-ui:test, without setting HOST, its UI is reachable via IPv6 and it connects to IPv6 mqtt fine. However, websocket (port 3000) doesn't seem to be reachable via IPv6 when you leave "Server Host" empty in "Home Assistant" section of settings.

@robertsLando
Copy link
Member

robertsLando commented Oct 19, 2023

Good for the UI side, for the ZwaveJs server side the problem is that they set defaultHost to 0.0.0.0, see https://github.com/zwave-js/zwave-js-server/blob/master/src/lib/server.ts#L509.

My suggestion is to open an issue there to suggest the change like I did on my side to simply not provide the host as default. I can merge the fix on my side

@zargony
Copy link
Author

zargony commented Oct 19, 2023

Makes sense. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
6 participants