TcpClient(hostName, port) constructor not handling IPv4-Mapped Addresses correctly

[agowa]

The System.Net.Sockets.TcpClient constructor fails to handle IPv4-Mapped Addresses correctly.
However the Connect function does.

I've tried this with Powershell 7.0.1 and .Net Core 3.1.4

Not working example:

PS C:\> $tcpConnection = [System.Net.Sockets.TcpClient]::new("::ffff:c163:9050", 443)
MethodInvocationException: Exception calling ".ctor" with "2" argument(s): "The requested address is not valid in its context. [::ffff:193.99.144.80]:443"

Working example:

PS C:\> $tcpConnection = [System.Net.Sockets.TcpClient]::new()
PS C:\> $tcpConnection.Connect("::ffff:c163:9050", 443)
PS C:\> $tcpConnection.Connected
True

Version information:

PS C:\> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.0.1
PSEdition                      Core
GitCommitId                    7.0.1
OS                             Microsoft Windows 10.0.19631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

PS C:\> [Environment]::Version

Major  Minor  Build  Revision
-----  -----  -----  --------
3      1      4      -1

Related RFC: https://tools.ietf.org/html/rfc4038#section-4.2

References: microsoft/dotnet#1214

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

The System.Net.Sockets.TcpClient constructor fails to handle IPv4-Mapped Addresses correctly.
However the Connect function does.

I've tried this with Powershell 7.0.1 and .Net Core 3.1.4

Not working example:

PS C:\> $tcpConnection = [System.Net.Sockets.TcpClient]::new("::ffff:c163:9050", 443)
MethodInvocationException: Exception calling ".ctor" with "2" argument(s): "The requested address is not valid in its context. [::ffff:193.99.144.80]:443"

Working example:

PS C:\> $tcpConnection = [System.Net.Sockets.TcpClient]::new()
PS C:\> $tcpConnection.Connect("::ffff:c163:9050", 443)
PS C:\> $tcpConnection.Connected
True

Version information:

PS C:\> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.0.1
PSEdition                      Core
GitCommitId                    7.0.1
OS                             Microsoft Windows 10.0.19631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

PS C:\> [Environment]::Version

Major  Minor  Build  Revision
-----  -----  -----  --------
3      1      4      -1

Related RFC: https://tools.ietf.org/html/rfc4038#section-4.2

References: microsoft/dotnet#1214

Author:	agowa338
Assignees:	-
Labels:	area-System.Net.Sockets, untriaged
Milestone:	-

[karelz]

Reproduced on both .NET Core 3.1 and .NET 5:

TcpClient c1 = new TcpClient();
c1.Connect("::ffff:c163:9050", 443);
Console.WriteLine(c1.Connected); // True
TcpClient c2 = new TcpClient("::ffff:c163:9050", 443);
// System.Net.Internals.SocketExceptionFactory+ExtendedSocketException (10049): The requested address is not valid in its context. [::ffff:193.99.144.80]:443

[antonfirsov]

Since dotnet/corefx#30036 the parameterless constructor is triggering InitializeClientSocket() that will pre-create a dual-stack socket for the connection:

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/TCPClient.cs

Line 26 in c2e098b

public TcpClient() : this(AddressFamily.Unknown)

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/TCPClient.cs

Lines 432 to 440 in c2e098b

	if (_family == AddressFamily.Unknown)
	{
	// If AF was not explicitly set try to initialize dual mode socket or fall-back to IPv4.
	_clientSocket = new Socket(SocketType.Stream, ProtocolType.Tcp);
	if (_clientSocket.AddressFamily == AddressFamily.InterNetwork)
	{
	_family = AddressFamily.InterNetwork;
	}
	}

The TcpClient(string, int) constructor does not call that method, and will eventually attempt connecting using a non-DualMode socket:

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/TCPClient.cs

Lines 171 to 174 in c2e098b

	if ((address.AddressFamily == AddressFamily.InterNetwork && Socket.OSSupportsIPv4) || Socket.OSSupportsIPv6)
	{
	var socket = new Socket(address.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
	// Use of Interlocked.Exchanged ensures _clientSocket is written before Disposed is read.

I think this is a legacy logic inherited from the times before dual-stack sockets existed in .NET. Replacing it with a call to InitializeClientSocket() seems to be a trivial fix.

@agowa338 are you interested in a PR?

[jaymin93]

@antonfirsov just to understand your last comment better , InitializeClientSocket() method call addition will fix out the issue correct ?

[antonfirsov]

Setting _family = AddressFamily.Unknown, and calling InitializeClientSocket() in the constructor should fix the bug.
After that, the following branch might become dead, if yes it should be removed:

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/TCPClient.cs

Line 166 in c2e098b

if (_clientSocket == null)

We also need a new test case to validate the fix and establish functional coverage.

---

However:
Taking a second look, I would do a more radical simplification, and drop all the logic from TcpClient.Connect(string, int), so it delegates into Socket.Connect(string, int) instead. @wfurt is there any reason you didn't do it in dotnet/corefx#30036? Do you see any chance we'll break something with such a change?

[wfurt]

I think we can look at it. I'm not sure what the behavior would be is OS does not support DualMode sockets. It seems like the current logic would still work but I'm not sure what the behavior would be with Socket.Connect(). It seems like we would always connect to only one address family.
On broader thought, I'm wondering if Pv4-Mapped should be really treated as IPv4 e.g. use IPv4 socket instead of DualMode.

[agowa]

@wfurt: I'd expect behavior according to the RFC https://tools.ietf.org/html/rfc4038#section-4.2

I'm interested in this feature as I do not want to handle dual stack and just use the mapped addresses instead of the IPv4 ones, so that my applications can be fully IPv6 only while still being able to reach github and twitter...

[antonfirsov]

@agowa338 some context on @wfurt's concerns:
While .NET socket behavior tends to (silently) shift towards dual-stack everywhere, recently we discovered that it may lead to unexpected issues when used together with certain VPN-like solutions, which do not support it properly (see #44686).

Regardless, we need to pick a preference regarding the type of the sockets we use in AddressFamily-agnostic API-s, and stick to it for consistency. Currently our most visible Socket constructor is creating Dual Stack sockets in most environments, so we should prefer it everywhere IMO. If we are concerned about potential issues and side effects of using dual-stack, we should shift our preference globally, not just in one particular method.

[agowa]

@antonfirsov Well, these workaround than provide issues to others. I'd suggest to follow the RFC.
We frequently encounter issues where applications try to be smart instead of just leaving that task to the admin. The administrator can than decide to disable IPv4 completely, to add a firewall rule, or to do whatever else. But what is needed for that to work is software that follows the RFC.

[karelz]

@agowa338 I am not sure which part of RFC you have in mind. Can you please be specific? Thanks!

[wfurt]

The RFC really talks what should happen on the wire. There is no doubt that it should go out as IPv4. The trickery in in TcpClient it self. I was wondering if we should do something similar to https://github.com/dotnet/corefx/pull/37194/files e.g. treat v4 mapped address as IPv4 instead of using dual-mode socket.

Since that (and any other clean-up) is unrelated to the reported issue I put up PR to fix this issue.

[agowa]

@wfurt: I thought the RFC is clear on that, look at the "stack" in section 4.2, the application (and the libraries should be IPv6).
E.g. The application doesn't know that IPv4 exists nor that it is a special address. The "magic" happens when the TCP layer (aka. the library that crafts the tcp package) adds the IP layer. It will pick the IPv4 layer when sending the package. But from the application side it will be IPv6 (with the mapped address). Allowing application developers to disregard the existence of IPv4.

Therefore the application should think that it has a IPv6 only socket. But when the application is going to access a mapped address it will be translated transparently onto the wire while still looking like "normal" IPv6 traffic to the application.

The beauty of IPv4 Mapped addresses is that it will (if implemented properly) free Layer 4 to 7 from checking if the Layer 3 connection is IPv4 or IPv6 allowing for a uniform IPv6 only code path while still being able to connect to IPv4 only endpoints (like github and twitter).
I imagine a call stack going through the IPv6 Layer 3 function to craft the package, but than being forwarded from there towards the IPv4 function instead. And "shiming" the function calls, e.g. appearing as IPv6 if the application asks, but being put onto the wire as IPv4. If the package sizes are respected this could be as simple as opening two sockets and transparently "copying" the upper layer data.

Also just a quick reminder (even though windows doesn't support them itself), for historic reasons there are not just only IPv4 Mapped addresses, but also IPX and NSAP (https://tools.ietf.org/html/rfc1884#page-10 2.4.5-2.4.6), if you now consider an application attempting to make a connection to such addresses windows would pass it onto the wire according to the routing table. Than another device (a router) than could do the address conversion and connect the IPv6 network to an IPX or NSAP network. Therefore looking into the future if windows is going to drop the IPv4 stack, the same will happen here, therefore the IPv4Mapped addresses will no longer be considered special (as the stack got removed) and another device (router) will provide backwards compatibility and free developers and admins from dealing with legacy stuff and allowing to innovate.
And another final thought, it is most likely that the same approach will be taken if at any time another protocol will supersede IPv6...

Tl;Dr: Applications should be able to be written in IPv6 only fashion utilizing the Mapped addresses to tell Layer 3 or a router (if the device doesn't have an IPv4 stack anymore and the special case gets removed) to perform the address translation and connect the two protocols with each other for compatibility reasons.