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

Local network IP not returned #1153

Open
rgaudin opened this issue Oct 15, 2024 · 23 comments
Open

Local network IP not returned #1153

rgaudin opened this issue Oct 15, 2024 · 23 comments

Comments

@rgaudin
Copy link
Member

rgaudin commented Oct 15, 2024

I chose bug and question because this feature is not documented and I thus, as a user, have no idea what I should get.

I tried the Windows nightly of kiwix-serve including #1132 (seems to be the one, as the ipv4 issue is gone) and I got the following:

C:\Mac\Home\Downloads\kiwix-tools_win-x86_64-2024-10-14>kiwix-serve.exe -p 8001 c:\Users\reg\python-libzim\rlz\test.zim
The Kiwix server is running and can be accessed in the local network at:
  - http://127.0.0.1:8001/
  - http://[::1]:8001

Based on the sentence, I guess I should have received local addresses in addition to the loop back ones (the sentence is false otherwise).

The getBestPublicIps function name is not helping I guess and the docstring neither: what's best?

@rgaudin
Copy link
Member Author

rgaudin commented Oct 15, 2024

FYI, here's the IP stack as seen by a regular user

C:\Users\reg>ipconfig /all

Windows IP Configuration

   Host Name . . . . . . . . . . . . : REGA946
   Primary Dns Suffix  . . . . . . . :
   Node Type . . . . . . . . . . . . : Hybrid
   IP Routing Enabled. . . . . . . . : No
   WINS Proxy Enabled. . . . . . . . : No
   DNS Suffix Search List. . . . . . : localdomain

Ethernet adapter Ethernet:

   Connection-specific DNS Suffix  . : localdomain
   Description . . . . . . . . . . . : Parallels VirtIO Ethernet Adapter
   Physical Address. . . . . . . . . : 00-1C-42-48-56-7E
   DHCP Enabled. . . . . . . . . . . : Yes
   Autoconfiguration Enabled . . . . : Yes
   IPv6 Address. . . . . . . . . . . : fdb2:2c26:f4e4:0:94ca:7a72:9711:f4a7(Preferred)
   Temporary IPv6 Address. . . . . . : fdb2:2c26:f4e4:0:744d:d91f:36c6:f004(Preferred)
   Link-local IPv6 Address . . . . . : fe80::6981:4ed:200f:bc94%14(Preferred)
   IPv4 Address. . . . . . . . . . . : 10.211.55.3(Preferred)
   Subnet Mask . . . . . . . . . . . : 255.255.255.0
   Lease Obtained. . . . . . . . . . : Tuesday, October 15, 2024 10:16:51 AM
   Lease Expires . . . . . . . . . . : Tuesday, October 15, 2024 11:48:58 AM
   Default Gateway . . . . . . . . . : 10.211.55.1
   DHCP Server . . . . . . . . . . . : 10.211.55.1
   DHCPv6 IAID . . . . . . . . . . . : 100670530
   DHCPv6 Client DUID. . . . . . . . : 00-01-00-01-2B-08-1A-6B-00-1C-42-48-56-7E
   DNS Servers . . . . . . . . . . . : 10.211.55.1
   NetBIOS over Tcpip. . . . . . . . : Enabled

C:\Users\reg>

@kelson42
Copy link
Collaborator

@rgaudin thx forcreprting. @sgourdas Do you havexall what you need? Do you know why we have a regression on Windows? Or is that a non-windows specific bug?

@veloman-yunkan
Copy link
Collaborator

@kelson42 I don't think that it is a regression. 10.211. is not an IP prefix that we consider belonging to a local network.

@sgourdas
Copy link
Collaborator

@kelson42 @rgaudin this works as expected. The fe80:: address is link local (which is automatically excluded from getNetworkInterfacesWin) and the 10.221 one was never considered a valid ip for this implementation. I think that the latter can be considered being added if we want to reconsider the ranges like for example covering the entire 10.0.0.0 – 10.255.255.255 and 172.16.0.0 – 172.31.255.255 spectrums.

As far as the message, for when specifically showing the loopback addresses, I agree can be slightly adjusted.

@kelson42
Copy link
Collaborator

@veloman-yunkan @sgourdas The hwlpwr shows 127.0.0.1, it can not be.

@sgourdas
Copy link
Collaborator

@veloman-yunkan @sgourdas The hwlpwr shows 127.0.0.1, it can not be.

I am not sure I understood correctly. Should we remove the loopback addresses as the last resort binding?

@kelson42
Copy link
Collaborator

kelson42 commented Oct 16, 2024

Don't show (and use) ipv4 loopback if you have an ipv4 IP available on which the system is reachable from the outside (of the system). This is that simple.

@kelson42
Copy link
Collaborator

@kelson42 I don't think that it is a regression. 10.211. is not an IP prefix that we consider belonging to a local network.

"local network" is a concept which has never been requested to consider regarding that feature. Any IP allowing to reach the computer from an other one is fine.

@sgourdas
Copy link
Collaborator

So, should we:

  • Add the entire 10.0.0.0 – 10.255.255.255 and 172.16.0.0 – 172.31.255.255 spectrums?
  • Remove the loopback addresses from being a final resort (currently they are being used only when no other is found)?

@rgaudin
Copy link
Member Author

rgaudin commented Oct 16, 2024

Why Are we limiting to known prefixes in the first place? Isn't it more logical and reliable to only exclude those that are known to cause issues?

@sgourdas
Copy link
Collaborator

Why Are we limiting to known prefixes in the first place? Isn't it more logical and reliable to only exclude those that are known to cause issues?

I am not really sure exactly why the approach is that TBH, it has been like that since previously and #1132 just built on top. It seems to me as well that it would be best to follow that approach, but it may be the case that we are not really sure what can cause issues as well?

@veloman-yunkan
Copy link
Collaborator

One problematic case that I can think of is virtual network IPs (e.g. created by the docker daemon). On my Ubuntu systems two interfaces with IPv4 addresses 172.17.0.1 and 172.18.0.1 are created when docker service is started.

@rgaudin
Copy link
Member Author

rgaudin commented Oct 16, 2024

One problematic case that I can think of is virtual network IPs (e.g. created by the docker daemon). On my Ubuntu systems two interfaces with IPv4 addresses 172.17.0.1 and 172.18.0.1 are created when docker service is started.

How is that an issue? Is dockerd removing the interfaces on exit and you are afraid that kiwix-serve lifecycle would be tied to the one of dockerd in that case?

@veloman-yunkan
Copy link
Collaborator

One problematic case that I can think of is virtual network IPs (e.g. created by the docker daemon). On my Ubuntu systems two interfaces with IPv4 addresses 172.17.0.1 and 172.18.0.1 are created when docker service is started.

How is that an issue? Is dockerd removing the interfaces on exit and you are afraid that kiwix-serve lifecycle would be tied to the one of dockerd in that case?

I am concerned that kiwix-desktop may offer an IP address that actually belongs to a docker virtual network.

@veloman-yunkan
Copy link
Collaborator

Don't show (and use) ipv4 loopback if you have an ipv4 IP available on which the system is reachable from the outside (of the system). This is that simple.

@kelson42 It's not that simple. How do we distinguish between real networks to which the system is connected and virtual networks created by some service like docker running on our system?

@rgaudin
Copy link
Member Author

rgaudin commented Oct 16, 2024

One problematic case that I can think of is virtual network IPs (e.g. created by the docker daemon). On my Ubuntu systems two interfaces with IPv4 addresses 172.17.0.1 and 172.18.0.1 are created when docker service is started.

How is that an issue? Is dockerd removing the interfaces on exit and you are afraid that kiwix-serve lifecycle would be tied to the one of dockerd in that case?

I am concerned that kiwix-desktop may offer an IP address that actually belongs to a docker virtual network.

@kelson42 It's not that simple. How do we distinguish between real networks to which the system is connected and virtual networks created by some service like docker running on our system?

How is that a concern? Should a user want to bind to a specific interface (for preference, control, security, etc), he can specify the interface to use via -i.

@veloman-yunkan
Copy link
Collaborator

veloman-yunkan commented Oct 16, 2024

@rgaudin Definitely. But the concern is about the IP address shown to the user in the absence of the -i option (or, equivalently, -i all). Currently if there are multiple interfaces only one of them is shown to the user (though all of them should work1). If it happens to be a virtual network instead of a real one it won't behave up to the user's expectations.


1 when connecting from the corresponding network

@rgaudin
Copy link
Member Author

rgaudin commented Oct 16, 2024

Oh I thought all of them were shown.

Is this something that could be rethought? I understand how showing a single one is better in terms of UX… if the one shown is the right one.I think that showing all of them would be simpler and more realistic.

I think it would benefit from a bit of sorting: publicly routable IPs first (actual public IP), then the ones that are most-likely user-managed (192.168.0.0/16), then the rest of the private ranges, then the loopbacks (indicating that those are for same-host only).

I believe most non-tech users would have only one, but indeed, people with complex network stacks would have more… but I think we can consider them able to understand the difference.

@kelson42
Copy link
Collaborator

kelson42 commented Oct 20, 2024

I'm not in favour of showing a helper for all IPs or having listening on all IPs per default because most users will be just lost. This is only a helper.

We should keep relying on the heuristic to choose the best IP. If this is not possible to get it working fine for all scenarios, it should at least work fine for the most common. Here we have a very common - difficult to make more simple IMHO - and it fails! So we have a bug.

Here we face a normalised (by the IETF) IP range for local networks, please handle it as such: https://www.lookip.net/network/10.211.55.

If the way how the code works currently is correct, you should also ensure that you have the full list of the such IP range for local networks.

@kelson42
Copy link
Collaborator

@sgourdas @veloman-yunkan Does the expected behaviour is clear enough?

@sgourdas
Copy link
Collaborator

sgourdas commented Nov 3, 2024

@kelson42 TBH I am not sure I understand how we want this to work.

What do we want displayed in each case?

@kelson42
Copy link
Collaborator

kelson42 commented Nov 3, 2024

@sgourdas I don't want to get127.0.0.1 chosen if there is a clear LAN/routable/public IP. This is the same requirement since months. Or maybe your question is "how it should it be coded?" to deliver this.

@sgourdas
Copy link
Collaborator

sgourdas commented Nov 7, 2024

Sorry for the lack of feedback here. Will try to get back ASAP.

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

No branches or pull requests

4 participants