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 on 0.0.0.0 to detect port usage #300

Merged
merged 2 commits into from
Feb 19, 2024

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Feb 14, 2024

What does this PR do?

Updates the function used to detect a free port, to try to listen on the address 0.0.0.0 instead of 127.0.0.1, as on Mac, a port opened on all addresses is detected as non open on 127.0.0.1

We had to fix the problem on odo lately, and we had chosen this solution:

What issues does this PR fix or reference?

Fixes #299
Fixes #297

How to test this PR?

See steps to reproduce in #299

@feloy feloy requested a review from a team as a code owner February 14, 2024 13:29
Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not work for me on Windows 11. I tried to start playround for 2 different models

image

@jeffmaury
Copy link
Contributor

Replacing 0.0.0.0 by 127.0.0.1 fixes the issue for me

@benoitf
Copy link
Collaborator

benoitf commented Feb 14, 2024

before there was no host in podman-desktop
podman-desktop/podman-desktop#4616

I'm wondering if it's like a windows vs mac problem

@feloy
Copy link
Contributor Author

feloy commented Feb 15, 2024

before there was no host in podman-desktop containers/podman-desktop#4616

I'm wondering if it's like a windows vs mac problem

Yes, either we could change the test depending on the platform, or another more conservative solution would be to check on all addresses (127.0.0.1 and 0.0.0.0) and consider the port free if it is in all of them, as we are not sure who is opening the port, and on which address (and the behaviour of systems seems to be different here: when the address is bound to 0.0.0.0, it can be either considered open or not open on 127.0.0.1)

@jeffmaury
Copy link
Contributor

before there was no host in podman-desktop containers/podman-desktop#4616
I'm wondering if it's like a windows vs mac problem

Yes, either we could change the test depending on the platform, or another more conservative solution would be to check on all addresses (127.0.0.1 and 0.0.0.0) and consider the port free if it is in all of them, as we are not sure who is opening the port, and on which address (and the behaviour of systems seems to be different here: when the address is bound to 0.0.0.0, it can be either considered open or not open on 127.0.0.1)

That maye podman machine dependant. On WSL when you ask port binding it uses 127.0.0.1 for the bound port. Let me try with an HyperV machine

@benoitf
Copy link
Collaborator

benoitf commented Feb 15, 2024

side question: should we add the port detection as part of Podman Desktop extension API ?

@feloy
Copy link
Contributor Author

feloy commented Feb 15, 2024

side question: should we add the port detection as part of Podman Desktop extension API ?

Yes (I didn't dare ask ;) )

In the future, it would be helpful to complete the detection by checking the ports that could be used by stopped containers, so having this detection centralized would be nice

@lstocchi
Copy link
Contributor

I guess it really depends on the network used. Just made a test and by using network pasta it is also reachable at 127.0.0.1 on mac qemu. So I guess it really depends on the pod configuration. BTW i wonder why 0.0.0.0 is not working on Windows. Shouldn't it target everything?

@jeffmaury
Copy link
Contributor

Seems node related as Java new ServerSocket(9000) raises an exception if 9000 is bound to 127.0.0.1 (like Podman WSL does)

@feloy
Copy link
Contributor Author

feloy commented Feb 15, 2024

https://stackoverflow.com/questions/60217071/node-js-server-listens-to-the-same-port-on-0-0-0-0-and-localhost-without-error

On my mac with qemu:

% lsof -Pi :9000
COMMAND   PID     USER   FD   TYPE             DEVICE SIZE/OFF NODE NAME
gvproxy 24922 phmartin   32u  IPv6 0x1a628d900fafb071      0t0  TCP *:9000 (LISTEN)

So gvproxy is listening on ANYADDR(*)

% % lsof -a -p 24922 -i 6 -T f
COMMAND   PID     USER   FD   TYPE             DEVICE SIZE/OFF NODE NAME
gvproxy 24922 phmartin   32u  IPv6 0x1a628d900fafb071      0t0  TCP *:cslistener (SO=ACCEPTCONN,PQLEN=0,QLEN=0,QLIM=128,RCVBUF=1048576,REUSEADDR,SNDBUF=1048576 SS=UNKNOWN=0x100 TF=MSS=1024,UNKNOWN=0xa0)

And it has set the REUSEADDR flag.

So the behaviour is what is expected when trying to bind to localhost (resusable) or anyaddr (not reusable) from what I understand from:

Some folks don't like SO_REUSEADDR because it has a security stigma attached to it. On some operating systems it allows the same port to be used with a different address on the same machine by different processes at the same time. This is a problem because most servers bind to the port, but they don't bind to a specific address, instead they use INADDR_ANY (this is why things show up in netstat output as *.8080). So if the server is bound to *.8080, another malicious user on the local machine can bind to local-machine.8080, which will intercept all of your connections since it is more specific.

It seems to me that trying to bind on ANYADDR would be the good approach, as it should cover all cases (REUSEADDR set or not, and socket bound to localhost or ANYADDR). I'm curious to know why it does not work on Windows

@feloy
Copy link
Contributor Author

feloy commented Feb 15, 2024

Another solution, to get rid of these problems or reusability, would be to read the netstat data, for example https://www.npmjs.com/package/netstats

@jeffmaury
Copy link
Contributor

Or try to connect to localhost:port ?

@feloy
Copy link
Contributor Author

feloy commented Feb 15, 2024

Or try to connect to localhost:port ?

This is what is currently done (before this PR), but this doesn't work on Mac, or more generally when the port is opened on ANYADDR with the flag REUSEADDR:

Some folks don't like SO_REUSEADDR because it has a security stigma attached to it. On some operating systems it allows the same port to be used with a different address on the same machine by different processes at the same time. This is a problem because most servers bind to the port, but they don't bind to a specific address, instead they use INADDR_ANY (this is why things show up in netstat output as *.8080). So if the server is bound to *.8080, another malicious user on the local machine can bind to local-machine.8080, which will intercept all of your connections since it is more specific.

@feloy feloy marked this pull request as draft February 15, 2024 17:26
@feloy
Copy link
Contributor Author

feloy commented Feb 15, 2024

Moving to draft as this proposal does not work on Windows

@jeffmaury
Copy link
Contributor

Or try to connect to localhost:port ?

This is what is currently done (before this PR), but this doesn't work on Mac, or more generally when the port is opened on ANYADDR with the flag REUSEADDR:

Some folks don't like SO_REUSEADDR because it has a security stigma attached to it. On some operating systems it allows the same port to be used with a different address on the same machine by different processes at the same time. This is a problem because most servers bind to the port, but they don't bind to a specific address, instead they use INADDR_ANY (this is why things show up in netstat output as *.8080). So if the server is bound to *.8080, another malicious user on the local machine can bind to local-machine.8080, which will intercept all of your connections since it is more specific.

No you don't get me. Before it was trying to LISTEN on localhost:port. I'm proposing that in order to detect if the port is in use or not, we try to CONNECT to that port on localhost (or 0.0.0.0)

@feloy
Copy link
Contributor Author

feloy commented Feb 16, 2024

Or try to connect to localhost:port ?

This is what is currently done (before this PR), but this doesn't work on Mac, or more generally when the port is opened on ANYADDR with the flag REUSEADDR:

Some folks don't like SO_REUSEADDR because it has a security stigma attached to it. On some operating systems it allows the same port to be used with a different address on the same machine by different processes at the same time. This is a problem because most servers bind to the port, but they don't bind to a specific address, instead they use INADDR_ANY (this is why things show up in netstat output as *.8080). So if the server is bound to *.8080, another malicious user on the local machine can bind to local-machine.8080, which will intercept all of your connections since it is more specific.

No you don't get me. Before it was trying to LISTEN on localhost:port. I'm proposing that in order to detect if the port is in use or not, we try to CONNECT to that port on localhost (or 0.0.0.0)

ah, sorry, my bad :(

Yes, but it seems it would be much more intrusive, as it will connect to the services, which could have potential side effects.

I tried the node-netstat library (https://www.npmjs.com/package/node-netstat), but 1. it relies on the command netstat to be installed on the OS and 2. it seems buggy, as it is, for example, not able to parse corretly '*.9000' in the output of netstat in mac.

I pushed a new commit to check on both addresses 127.0.0.1 and 0.0.0.0. IMO, it should cover all cases. Could you please make another test on Windows?

@feloy feloy force-pushed the fix-299/isFreePort branch from 72acc9a to a8198c2 Compare February 16, 2024 12:16
@feloy feloy marked this pull request as ready for review February 16, 2024 15:27
@feloy feloy requested a review from jeffmaury February 19, 2024 10:10
Copy link
Contributor

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine for me on Win 11

@feloy
Copy link
Contributor Author

feloy commented Feb 19, 2024

Did not work for me on Windows 11. I tried to start playround for 2 different models

@jeffmaury Is it working for you too on Windows?

Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on Windows and MacOS

@feloy feloy merged commit 5fda460 into containers:main Feb 19, 2024
3 checks passed
mhdawson pushed a commit to mhdawson/podman-desktop-extension-ai-lab that referenced this pull request Nov 22, 2024
…dels

adding merlinite and granite models
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.

Failed to start a second playground on mac Failed to start a second application
4 participants