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

Listen to '::' instead of '0.0.0.0' #4693

Closed
wants to merge 1 commit into from
Closed

Conversation

UnderEu
Copy link

@UnderEu UnderEu commented May 12, 2024

Ensure invidious process listens to '::' (IPv6+IPv4) instead of '0.0.0.0' (IPv4-only), allowing every host on every Internet protocol (current or jurassic a.k.a IPv6 or IPv4, respectively) to reach the service.

Documentation still needs to be updated but function-wise this is enough to make it work correctly.

Ensure invidious process listens to '::' (IPv6+IPv4) instead of '0.0.0.0' (IPv4-only), allowing every host on every Internet protocol (current or jurassic a.k.a IPv6 or IPv4, respectively) to reach the service.
@UnderEu UnderEu requested a review from a team as a code owner May 12, 2024 15:38
@UnderEu UnderEu requested review from unixfox and removed request for a team May 12, 2024 15:38
@unixfox
Copy link
Member

unixfox commented May 12, 2024

I think it's better to change it here than through this parameter: https://github.com/iv-org/invidious/blob/master/src/invidious.cr#L228

@GlowingUmbreon
Copy link

:: does not necessarily mean listen to both IPv6 and IPv4.

On FreeBSD this will always listen to IPv6, to accept IPv6 & IPv4 traffic you would need to run two bind commands, which it appears Kemal is not currently setup to handle this as it only allows you to bind to 1 address. You could also set the ipv6_ipv4mapping system option but this is not the default.

On Linux it is similar :: can listen to both IPv6 and IPv4 traffic but only if the IPV6_V6ONLY parameter is unset. this could be set or unset depending on the distribution or the user's configuration since it appears that Crystal's HTTP server does not provide this option in the listen command.

At the moment it looks like to have listening to IPv6 also consistently listen to IPv4 would require a update from either Kemal or Crystal.

@unixfox
Copy link
Member

unixfox commented May 15, 2024

@UnderEu could you please test to confirm what @GlowingUmbreon said? Thank you

@UnderEu
Copy link
Author

UnderEu commented May 18, 2024

@UnderEu could you please test to confirm what @GlowingUmbreon said? Thank you

In fact, my particular system (Ubuntu 24.04) does both protocols when binding "::" by default.

Untitled2

Untitled

@GlowingUmbreon
Copy link

GlowingUmbreon commented May 18, 2024

I have looked into this a bit further since I was not entirely happy with how I worded my initial comment on this issue, the below response should hopefully be clearer and have some sources.

To allow for IPv6 binding to also bind to IPv4 two features must be enabled:

  1. IPv6 and IPv4 address mapping1 must be enabled by the Operating system. This converts IPv4 addresses to suitable IPv6 addresses.
  2. The IPV6_V6ONLY23 option must be unset by the Operating System or by the Program. This allows for a IPv6 listen to also listen to IPv4 clients.

These above options are not entirely predictable as they will vary based on OS/Distro. Some programs (such as nginx) handle this by having 1 listen for IPv6 and 1 listen for IPv4 which are handled separately4.

Admittedly I am not entirely sure what the defaults for these options are between different OS/Distro's is and if this change will cause issues among most invidious hosters, there does not appear to be a good resource to find these defaults online.

If updating the default address is to be merged it may be worth first testing the most common OS/Distro's to see if these two requirements are met and adding a note to the documentation on how to handle this if instance managers are having troubles with networking caused by this change.

Footnotes

  1. https://docs.freebsd.org/en/books/handbook/network/#_ipv6_and_ipv4_address_mapping

  2. https://www.man7.org/linux/man-pages/man7/ipv6.7.html under IPV6_V6ONLY

  3. https://man.freebsd.org/cgi/man.cgi?query=ip6&sektion=4&format=html under IPV6_V6ONLY

  4. https://serverfault.com/questions/638367/do-you-need-separate-ipv4-and-ipv6-listen-directives-in-nginx under the first accepted answer

@unixfox
Copy link
Member

unixfox commented May 18, 2024

Ok so this won't break on most Linux distribution Debian, CentOS, AlmaLinux and ArchLinux:

[root@rockylinux-ipv6only ~]# cat /proc/sys/net/ipv6/bindv6only
0
archlinux$ cat /proc/sys/net/ipv6/bindv6only
0
root@debiantestipv6only:~# cat /proc/sys/net/ipv6/bindv6only 
0
[root@centostestipv6only ~]# cat /proc/sys/net/ipv6/bindv6only 
0

About freebsd I did some testing and setting to :: will break IPv4 connectivity on invidious. Invidious will not listen on IPv4 anymore.

I'm not sure how to tackle this improvement. Maybe we should have an if case for BSD distributions in order to not listen on ::.

But the initial proposal is wrong though, we want to make this "binding" change into the code not in the systemd service.

EDIT: I'm closing this issue for discussing of a better solution: #4705

@UnderEu
Copy link
Author

UnderEu commented May 19, 2024

[...]

I'm not sure how to tackle this improvement. Maybe we should have an if case for BSD distributions in order to not listen on ::.

But the initial proposal is wrong though, we want to make this "binding" change into the code not in the systemd service.

EDIT: I'm closing this issue for discussing of a better solution: #4705

Sure, I wasn't expecting this PR to be merged like this but the development to happen on the gist of it.
Let me know if you need any further testing from my end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants