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

connect: improve binding to local ports on Windows #8528

Closed
wants to merge 4 commits into from

Conversation

inPhraZ
Copy link
Contributor

@inPhraZ inPhraZ commented Mar 1, 2022

There is a configuration parameter in windows registry
called TcpTimedWaitDelay that determines the time that a
connection stays in the TIME_WAIT state when it is closing.
It's minimum and default value is 30 seconds.
As long as a connection is in the TIME_WAIT state, the socket pair
cannot be re-used, BUT bind() still returns 0 in this case.
curl now can detect the state of a given port using
GetTcpTable2() for IPv4 connections and avoids to bind that port
if the state is TIME_WAIT

ref: https://docs.microsoft.com/en-us/troubleshoot/windows-client/networking/tcpip-and-nbt-configuration-parameters#optional-tcpip-parameters-that-you-can-configure-by-using-registry-editor

Fixes: #8112
Reported-by: gclinch on github

to use GetTcpTable2() and GetTcp6Table2()
There is a configuration parameter in windows registry
called TcpTimedWaitDelay that determines the time that a
connection stays in the TIME_WAIT state when it is closing.
It's minimum and default value is 30 seconds.
As long as a connection is in the TIME_WAIT state, the socket pair
cannot be re-used, BUT bind() still returns 0 in this case.
curl now can detect the state of a given port using
GetTcpTable2() for IPv4 connections and avoids to bind that port
if the state is TIME_WAIT

Fixes: curl#8112
Bug: curl#8112
Reported-by: gclinch on github
@jay
Copy link
Member

jay commented Mar 1, 2022

This is an interesting solution and I am very interested in what others will think of it. My initial thoughts are:

I think it would probably have some cost to enumerate. The bind address would have to be checked as well as the port. IPv4 / IPv6 (af == AF_INET6) differences, the latter needs IPv6 enumeration check. Also how reliable is it if the port is in some other state and the check passes because it's not timewait and then on connect same problem?

Also I wonder how malware detection software is going to react to enumerating the entire table of tcp.

@jay jay added the Windows Windows-specific label Mar 1, 2022
@inPhraZ
Copy link
Contributor Author

inPhraZ commented Mar 2, 2022

The bind address would have to be checked as well as the port. IPv4 / IPv6 (af == AF_INET6) differences, the latter needs IPv6 enumeration check.

GetTcp6Table2 can be used for IPv6 connections.

Also how reliable is it if the port is in some other state and the check passes because it's not timewait and then on connect same problem?

I did some tests and yes... if the port is in some other state we may face the same problem (E.g. FIN_WAIT1). so basically bind should only be called if the port is not in any state defined in the MIB_TCP_STATE.
of course there's a very low possibility for the port state to change just before calling the bind.

@bagder bagder changed the title KNOWN_BUGS: fix 13.2 (trying local ports on Windows) connect: improve binding to local ports on Windows Mar 7, 2022
@bagder
Copy link
Member

bagder commented May 23, 2022

I need more guidance and feedback on this proposal from people using and knowing Windows.

@bagder bagder added the needs-votes Pull-request in need of thumbs-ups to make progress label Aug 8, 2022
@bagder
Copy link
Member

bagder commented Sep 25, 2022

This improvement proposal has not received much interest and I cannot deem the necessity myself. As such, I cannot merge this. Sorry.

@bagder bagder closed this Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted needs-votes Pull-request in need of thumbs-ups to make progress Windows Windows-specific
3 participants