-
Notifications
You must be signed in to change notification settings - Fork 2.3k
container: add default support for IPv6 #4448
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
Conversation
|
How does it listen to ipv4 as well? You've only made it bind to the v6 wildcard address. This will probably confuse people when debugging they can't reach anything over v4. |
https://www.rfc-editor.org/rfc/rfc3493#section-3.7 ..basically, every operating system or thing that supports IPv6 will automatically convert from IPv4 to IPv6 space for you. And if someone explicitly defines the BIND_ADDRESS and the dual stack screwed it is not our problem, all applications go and will go through this transition process someday and users need to learn how to deal with it |
|
In most programming language like python, [::] means that the app will open a socket on both IPv4 and IPv6. |
|
Ah interesting - in golang it would just listen to v6. Learned something new about python today :) Ok, so after reading the RFC, IPv4 clients should appear as ::ffff:192.0.2.1 in the logs for example? I'll take a in depth look later tonight. |
|
Ok well maybe not in python about the sockets listening on both ipv4/ipv6. But indeed it's going to do what the RFC say. If do I: Then I get: Now I that I think about it, I wonder if this could mess up with the rate limiter that doesn't expect IPv4 IP addresses into an IPv6 address. According to stackoverflow, if we want to have a dual stack socket then we need to use In Docker I thought it would "remove" the |
In fact, this is expected to identify that an IPv6 address comes from IPv4 (RFC3493; section 3.7), although now that you mention about ratelimit this may be an issue |
|
@return42 would you be ok to add a specific case about I guess we would only need to catch this specific |
By default IPv4Address.is_link_local are not filtered: searxng/searx/botdetection/ip_limit.py Lines 102 to 104 in 67a8b66
Or with other words, by default botdetection.ip_limit.filter_link_local is set to |
|
@return42 if we merge this PR, all the IPv4 address space will be like this for Docker. So my question is not only about the localhost IP addresses but all the IPv4 address space. |
Python has an abstraction named A >>> from ipaddress import ip_address
>>> ip_address("::ffff:192.0.2.1")
IPv6Address('::ffff:192.0.2.1')while this is a IPv4 address .. >>> ip_address("192.0.2.1")
IPv4Address('192.0.2.1') |
Keep in mind that IPv4-mapped addresses aren't link local, they are from the block Either way the limiter must be able to differentiate between v4 and v6 so that IP ranges can be properly blocked. Because this range is treated like any other v6 address, blocking a /48 (default) would ban all of IPv4 address space. Line 11 in 67a8b66
|
|
The limiter uses this function to get the IP address .. searxng/searx/botdetection/_helpers.py Lines 71 to 74 in 67a8b66
May we can (as @unixfox suggested) remove a searxng/searx/botdetection/_helpers.py Lines 128 to 130 in 67a8b66
|
Yes, this will be the best place to do it (I recommend using ipv6_mapped though). We should also consider where else we're using remote_addr - such as in the Tor Check plugin - which this PR would break for v4. Actually after looking at this, I think the tor check plugin should use get_real_ip() instead - would you be happy if I send through a PR for this done? searxng/searx/plugins/tor_check.py Lines 75 to 79 in 67a8b66
|
FYI I incorporated the patch in PR: |
|
I vouch on changing the bind address for the settings.yml too: searxng/docs/admin/settings/settings.rst Line 62 in 02b76c8
Is everyone ok? |
|
@inetol could you add that too as a modification in this PR :)? you can change the title like "add default support for IPv6" |
@return42 May I understand that what I have asked you is resolved in the PR above?
Thank you in advance :) |
yes / sorry if it was not clear :-) |
Related #4378
Closes #965
To be merged after #4424