Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 72 additions & 15 deletions src/windows/service/exe/WslCoreGuestNetworkService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ void wsl::core::networking::GuestNetworkService::CreateGuestNetworkService(
TraceLoggingValue(m_hostUdpEphemeralPortRange.first, "udpStartPort"),
TraceLoggingValue(m_hostUdpEphemeralPortRange.second, "udpEndPort"));

m_hostTcpEphemeralPortCap = ComputeHostEphemeralPortCap(IPPROTO_TCP);
m_hostUdpEphemeralPortCap = ComputeHostEphemeralPortCap(IPPROTO_UDP);

hns::GuestNetworkService request{};
request.VirtualMachineId = VmId;
request.MirrorHostNetworking = true;
Expand Down Expand Up @@ -127,6 +130,11 @@ std::pair<uint16_t, uint16_t> wsl::core::networking::GuestNetworkService::Alloca

WI_ASSERT(m_reservedPortRange.endingPort - m_reservedPortRange.startingPort == c_ephemeralPortRangeSize);

// Count the overlap of the guest's reserved ephemeral range with the host ephemeral range
// and seed the in-use counters accordingly.
m_hostTcpEphemeralPortsInUse = ComputeHostEphemeralOverlap(IPPROTO_TCP);
m_hostUdpEphemeralPortsInUse = ComputeHostEphemeralOverlap(IPPROTO_UDP);

// setting the port to zero as we do not expect any bind requests to be sent to wslcore for ports in this range
m_reservedPorts.emplace(std::make_pair(HCN_PORT_PROTOCOL_TCP, static_cast<uint16_t>(0)), HcnPortReservation{port, 1});

Expand Down Expand Up @@ -225,6 +233,32 @@ bool wsl::core::networking::GuestNetworkService::IsPortInGuestEphemeralRange(uin
return PortNumber >= m_reservedPortRange.startingPort && PortNumber <= m_reservedPortRange.endingPort;
}

uint32_t wsl::core::networking::GuestNetworkService::ComputeHostEphemeralPortCap(int Protocol) const noexcept
{
const auto& range = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortRange : m_hostTcpEphemeralPortRange;
if (range.second < range.first)
{
return 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we ASSERT this an have tests on CHK builds?
this should never happen - it's a terrible code bug when it does

}

// Cap the guest at (at most) half of the host ephemeral range so it can't exhaust the host's ports.
return (static_cast<uint32_t>(range.second) - range.first + 1) / 2;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why does this need a static_cast<uint32_t>? what type is range.second?

}
Comment on lines +236 to +246

uint32_t wsl::core::networking::GuestNetworkService::ComputeHostEphemeralOverlap(int Protocol) const noexcept
{
const auto& range = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortRange : m_hostTcpEphemeralPortRange;
if (range.second < range.first)
{
return 0;
}

// Number of guest reserved ports that fall within the host ephemeral range.
const uint32_t overlapStart = std::max<uint32_t>(m_reservedPortRange.startingPort, range.first);
const uint32_t overlapEnd = std::min<uint32_t>(m_reservedPortRange.endingPort, range.second);
return (overlapStart <= overlapEnd) ? (overlapEnd - overlapStart + 1) : 0;
}

int wsl::core::networking::GuestNetworkService::OnPortAllocationRequest(const SOCKADDR_INET& Address, _In_ int Protocol, _In_ bool Allocate) noexcept
try
{
Expand Down Expand Up @@ -263,21 +297,6 @@ try

const auto lock = m_dataLock.lock_exclusive();

// The guest's reserved ephemeral range can overlap with the host range. Ports in
// the guest range are safe for the guest to use even if they fall within the host range.
if (IsPortInHostEphemeralRange(PortNumber, Protocol) && !IsPortInGuestEphemeralRange(PortNumber))
{
const auto& range = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortRange : m_hostTcpEphemeralPortRange;
WSL_LOG(
"GuestNetworkService::OnPortAllocationRequest - denying port in host ephemeral range",
TraceLoggingValue(StringAddress.c_str(), "IP address"),
TraceLoggingValue(Protocol == IPPROTO_TCP ? "TCP" : "UDP", "protocol"),
TraceLoggingValue(PortNumber, "portNumber"),
TraceLoggingValue(range.first, "hostEphemeralStart"),
TraceLoggingValue(range.second, "hostEphemeralEnd"));
return -LX_EADDRINUSE;
}

if (IsPortInGuestEphemeralRange(PortNumber))
{
WSL_LOG(
Expand All @@ -290,6 +309,8 @@ try
return 0;
}

const bool isHostEphemeralPort = IsPortInHostEphemeralRange(PortNumber, Protocol);

HRESULT result = E_UNEXPECTED;
const auto it = m_reservedPorts.find(std::make_pair(HnsProtocol, PortNumber));
if (Allocate)
Expand All @@ -307,6 +328,24 @@ try
return 0;
}

// New reservation for a port in the host ephemeral range: enforce the cap.
if (isHostEphemeralPort)
{
const auto cap = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortCap : m_hostTcpEphemeralPortCap;
auto& portsInUse = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortsInUse : m_hostTcpEphemeralPortsInUse;
if (portsInUse >= cap)
{
WSL_LOG(
"GuestNetworkService::OnPortAllocationRequest - denying port in host ephemeral range, cap reached",
TraceLoggingValue(StringAddress.c_str(), "IP address"),
TraceLoggingValue(Protocol == IPPROTO_TCP ? "TCP" : "UDP", "protocol"),
TraceLoggingValue(PortNumber, "portNumber"),
TraceLoggingValue(portsInUse, "hostEphemeralPortsInUse"),
TraceLoggingValue(cap, "hostEphemeralPortCap"));
return -LX_EADDRINUSE;
}
}

HANDLE port{nullptr};
auto releasePortOnError = wil::scope_exit([&] {
if (port)
Expand All @@ -324,6 +363,11 @@ try
if (SUCCEEDED(result))
{
m_reservedPorts.emplace(std::make_pair(HnsProtocol, PortNumber), HcnPortReservation{port, 1});

if (isHostEphemeralPort)
{
((Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortsInUse : m_hostTcpEphemeralPortsInUse)++;
}
}
// if the port was reserved, we successfully handed over ownership
releasePortOnError.release();
Expand All @@ -347,6 +391,16 @@ try
{
result = m_releasePort.value()(it->second.Handle);
m_reservedPorts.erase(it);

if (isHostEphemeralPort)
{
auto& portsInUse = (Protocol == IPPROTO_UDP) ? m_hostUdpEphemeralPortsInUse : m_hostTcpEphemeralPortsInUse;
if (portsInUse > 0)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

when would this be == 0 if isHostEphemeralPort is true?
should we add an assert here too and go through testing?

{
portsInUse--;
}
}
Comment on lines 392 to +402

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please strongly consider this change


WSL_LOG(
"GuestNetworkService::OnPortAllocationRequest - released port",
TraceLoggingValue(PortNumber, "Port"),
Expand Down Expand Up @@ -387,6 +441,9 @@ void wsl::core::networking::GuestNetworkService::Stop() noexcept
m_releasePort.value()(reservedPort.second.Handle);
}
m_reservedPorts.clear();

m_hostTcpEphemeralPortsInUse = 0;
m_hostUdpEphemeralPortsInUse = 0;
}

m_guestNetworkServiceCallback.reset();
Expand Down
11 changes: 11 additions & 0 deletions src/windows/service/exe/WslCoreGuestNetworkService.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ class GuestNetworkService
_Requires_lock_held_(m_dataLock)
bool IsPortInGuestEphemeralRange(uint16_t PortNumber) const noexcept;

uint32_t ComputeHostEphemeralPortCap(int Protocol) const noexcept;

_Requires_lock_held_(m_dataLock)
uint32_t ComputeHostEphemeralOverlap(int Protocol) const noexcept;

static std::pair<uint16_t, uint16_t> QueryHostEphemeralPortRange(LPCWSTR WmiClassName) noexcept;

static std::optional<LxssDynamicFunction<decltype(HcnReserveGuestNetworkServicePortRange)>> m_allocatePortRange;
Expand All @@ -81,5 +86,11 @@ class GuestNetworkService
// Host ephemeral port ranges can change. They are queried once at startup, if a change occurs, the service will need to be restarted.
std::pair<uint16_t, uint16_t> m_hostTcpEphemeralPortRange{};
std::pair<uint16_t, uint16_t> m_hostUdpEphemeralPortRange{};

uint32_t m_hostTcpEphemeralPortCap{};
uint32_t m_hostUdpEphemeralPortCap{};

_Guarded_by_(m_dataLock) uint32_t m_hostTcpEphemeralPortsInUse {};
_Guarded_by_(m_dataLock) uint32_t m_hostUdpEphemeralPortsInUse {};
};
} // namespace wsl::core::networking
152 changes: 51 additions & 101 deletions test/windows/NetworkTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2132,18 +2132,19 @@ class NetworkTests

// The port-0 resolution is asynchronous (deferred to a background thread).
// Retry until the host port tracker registers the port, blocking the host bind.
VERIFY_NO_THROW(wsl::shared::retry::RetryWithTimeout<void>(
[&assignedPort]() {
wil::unique_socket sock(socket(AF_INET, SOCK_STREAM, IPPROTO_TCP));
THROW_LAST_ERROR_IF(!sock);

SOCKADDR_IN addr{};
addr.sin_family = AF_INET;
addr.sin_port = htons(assignedPort);
THROW_HR_IF(E_FAIL, bind(sock.get(), reinterpret_cast<SOCKADDR*>(&addr), sizeof(addr)) != SOCKET_ERROR);
},
std::chrono::seconds(1),
std::chrono::seconds(30)));
VERIFY_NO_THROW(
wsl::shared::retry::RetryWithTimeout<void>(
[&assignedPort]() {
wil::unique_socket sock(socket(AF_INET, SOCK_STREAM, IPPROTO_TCP));
THROW_LAST_ERROR_IF(!sock);

SOCKADDR_IN addr{};
addr.sin_family = AF_INET;
addr.sin_port = htons(assignedPort);
THROW_HR_IF(E_FAIL, bind(sock.get(), reinterpret_cast<SOCKADDR*>(&addr), sizeof(addr)) != SOCKET_ERROR);
},
std::chrono::seconds(1),
std::chrono::seconds(30)));

if (!verifyRelease)
{
Expand All @@ -2154,18 +2155,19 @@ class NetworkTests
guestProcess.reset();

// Retry until the host can bind the port again, confirming it was released.
VERIFY_NO_THROW(wsl::shared::retry::RetryWithTimeout<void>(
[&assignedPort]() {
wil::unique_socket sock(socket(AF_INET, SOCK_STREAM, IPPROTO_TCP));
THROW_LAST_ERROR_IF(!sock);
VERIFY_NO_THROW(
wsl::shared::retry::RetryWithTimeout<void>(
[&assignedPort]() {
wil::unique_socket sock(socket(AF_INET, SOCK_STREAM, IPPROTO_TCP));
THROW_LAST_ERROR_IF(!sock);

SOCKADDR_IN addr{};
addr.sin_family = AF_INET;
addr.sin_port = htons(assignedPort);
THROW_HR_IF(E_FAIL, bind(sock.get(), reinterpret_cast<SOCKADDR*>(&addr), sizeof(addr)) == SOCKET_ERROR);
},
std::chrono::seconds(1),
std::chrono::minutes(2)));
SOCKADDR_IN addr{};
addr.sin_family = AF_INET;
addr.sin_port = htons(assignedPort);
THROW_HR_IF(E_FAIL, bind(sock.get(), reinterpret_cast<SOCKADDR*>(&addr), sizeof(addr)) == SOCKET_ERROR);
},
std::chrono::seconds(1),
std::chrono::minutes(2)));
}

static void VerifyPortZeroRebindSucceeds()
Expand All @@ -2174,15 +2176,16 @@ class NetworkTests
// Uses a perl one-liner to perform the entire sequence in a single process,
// matching the semantics of a native C test (no SO_REUSEADDR, same-process rebind).
VERIFY_ARE_EQUAL(
LxsstuLaunchWsl(L"perl -MSocket -e '"
L"socket(S1,AF_INET,SOCK_STREAM,0) or die;"
L"bind(S1,sockaddr_in(0,INADDR_ANY)) or die;"
L"my $port=(sockaddr_in(getsockname(S1)))[0];"
L"close(S1);"
L"socket(S2,AF_INET,SOCK_STREAM,0) or die;"
L"bind(S2,sockaddr_in($port,INADDR_ANY)) or die;"
L"close(S2)"
L"'"),
LxsstuLaunchWsl(
L"perl -MSocket -e '"
L"socket(S1,AF_INET,SOCK_STREAM,0) or die;"
L"bind(S1,sockaddr_in(0,INADDR_ANY)) or die;"
L"my $port=(sockaddr_in(getsockname(S1)))[0];"
L"close(S1);"
L"socket(S2,AF_INET,SOCK_STREAM,0) or die;"
L"bind(S2,sockaddr_in($port,INADDR_ANY)) or die;"
L"close(S2)"
L"'"),
0L);
}

Expand Down Expand Up @@ -2481,8 +2484,10 @@ class NetworkTests
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"ip addr add 192.168.15.1/24 dev testbridge"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"ip -n testns route add default via 192.168.15.1 dev veth-test"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft add table nat"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft \"add chain nat POSTROUTING { type nat hook postrouting priority srcnat; }\""), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft add rule nat POSTROUTING ip saddr 192.168.15.0/24 oif != testbridge masquerade"), 0);
VERIFY_ARE_EQUAL(
LxsstuLaunchWsl(L"--system --user root nft \"add chain nat POSTROUTING { type nat hook postrouting priority srcnat; }\""), 0);
VERIFY_ARE_EQUAL(
LxsstuLaunchWsl(L"--system --user root nft add rule nat POSTROUTING ip saddr 192.168.15.0/24 oif != testbridge masquerade"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"sysctl -w net.ipv4.ip_forward=1"), 0);

// Verify we have connectivity from the networking namespace when using ephemeral port selection.
Expand Down Expand Up @@ -2968,8 +2973,8 @@ class NetworkTests
}
else if (std::regex_search(line, match, routePattern) && match.size() >= 4)
{
state.Routes.emplace_back(Route{
match.str(2), match.str(3), {match.str(1)}, match.size() > 5 && match[5].matched ? std::stoi(match.str(5)) : 0});
state.Routes.emplace_back(
Route{match.str(2), match.str(3), {match.str(1)}, match.size() > 5 && match[5].matched ? std::stoi(match.str(5)) : 0});
}
}

Expand Down Expand Up @@ -4283,66 +4288,6 @@ class MirroredTests
VERIFY_IS_TRUE(canBindUdp);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we should add tests for this behavior

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

definitely need to validate that we can hit our limit, then they fail, then once sockets are closed and it falls below our limit, we can allocate again. then repeat that pattern (hit limit -> fail, fall below limit->succeed, repeat) in the same test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was hesitant to do this as even for the smallest host range (which seems to be enforced to 255), we would need to bind 128 times and I was not sure if that's too heavy for an automated test

I can write it and see if it takes too long to run. I can set the host range temporarily to 255 and then revert it to the original host range

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

calling bind 128 times is not taxing on anything - it should be just fine

}

void VerifyGuestBindToHostEphemeralRangeDenied(LPCWSTR ProtocolSettingClass, LPCWSTR SocatProtocolPrefix)
{
MIRRORED_NETWORKING_TEST_ONLY();

m_config->Update(LxssGenerateTestConfig({.networkingMode = wsl::core::NetworkingMode::Mirrored}));
WaitForMirroredStateInLinux();

// Query the host ephemeral port range via PowerShell.
auto startQuery =
std::wstring(L"(") + ProtocolSettingClass +
L" | Where-Object { $_.DynamicPortRangeStartPort -gt 0 } | Select-Object -First 1).DynamicPortRangeStartPort";
auto [startStr, _1] = LxsstuLaunchPowershellAndCaptureOutput(startQuery.c_str(), 0);
const auto hostEphemeralStart = std::stoi(startStr);

auto countQuery =
std::wstring(L"(") + ProtocolSettingClass +
L" | Where-Object { $_.DynamicPortRangeNumberOfPorts -gt 0 } | Select-Object -First 1).DynamicPortRangeNumberOfPorts";
auto [countStr, _2] = LxsstuLaunchPowershellAndCaptureOutput(countQuery.c_str(), 0);
const auto hostEphemeralEnd = hostEphemeralStart + std::stoi(countStr) - 1;

// Get the guest ephemeral port range.
auto [start, err1] = LxsstuLaunchWslAndCaptureOutput(L"cat /proc/sys/net/ipv4/ip_local_port_range | cut -f1", 0);
start.pop_back();
const auto guestEphemeralRangeStart = std::stoi(start);

auto [end, err2] = LxsstuLaunchWslAndCaptureOutput(L"cat /proc/sys/net/ipv4/ip_local_port_range | cut -f2", 0);
end.pop_back();
const auto guestEphemeralRangeEnd = std::stoi(end);

// Pick a port in the host ephemeral range but not in the guest's assigned range.
// The ranges may overlap
int testPort = 0;
if (hostEphemeralStart < guestEphemeralRangeStart || hostEphemeralStart > guestEphemeralRangeEnd)
{
testPort = hostEphemeralStart;
}
else if (guestEphemeralRangeEnd < hostEphemeralEnd)
{
testPort = guestEphemeralRangeEnd + 1;
}
else
{
VERIFY_FAIL(L"Guest ephemeral range fully covers the host ephemeral range, cannot find a test port");
}

auto socatArg = std::wstring(SocatProtocolPrefix) + std::to_wstring(testPort);
auto [listener, success, read] = NetworkTests::BindGuestPortHelper(socatArg);
VERIFY_IS_FALSE(success);
}

WSL2_TEST_METHOD(GuestTcpBindToHostEphemeralRangeDenied)
{
VerifyGuestBindToHostEphemeralRangeDenied(L"Get-NetTCPSetting", L"TCP4-LISTEN:");
}

WSL2_TEST_METHOD(GuestUdpBindToHostEphemeralRangeDenied)
{
VerifyGuestBindToHostEphemeralRangeDenied(L"Get-NetUDPSetting", L"UDP4-LISTEN:");
}

WSL2_TEST_METHOD(NonRootNamespaceEphemeralBind)
{
MIRRORED_NETWORKING_TEST_ONLY();
Comment on lines 4286 to 4288
Expand Down Expand Up @@ -4412,12 +4357,15 @@ class MirroredTests
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"ip addr add 192.168.15.1/24 dev testbridge"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"ip -n testns route add default via 192.168.15.1 dev veth-test"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft add table nat"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft \"add chain nat POSTROUTING { type nat hook postrouting priority srcnat; }\""), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft add rule nat POSTROUTING ip saddr 192.168.15.0/24 oif != testbridge masquerade"), 0);
VERIFY_ARE_EQUAL(
LxsstuLaunchWsl(L"--system --user root nft \"add chain nat POSTROUTING { type nat hook postrouting priority srcnat; }\""), 0);
VERIFY_ARE_EQUAL(
LxsstuLaunchWsl(L"--system --user root nft add rule nat POSTROUTING ip saddr 192.168.15.0/24 oif != testbridge masquerade"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"sysctl -w net.ipv4.ip_forward=1"), 0);

// Add rule for port forwarding traffic with destination port 8080 to port 80 in the new namespace
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft \"add chain nat PREROUTING { type nat hook prerouting priority dstnat; }\""), 0);
VERIFY_ARE_EQUAL(
LxsstuLaunchWsl(L"--system --user root nft \"add chain nat PREROUTING { type nat hook prerouting priority dstnat; }\""), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft add rule nat PREROUTING tcp dport 8080 dnat to 192.168.15.2:80"), 0);

// Start listeners in root namespace on port 8080 and new namespace on port 80
Expand Down Expand Up @@ -4491,8 +4439,10 @@ class MirroredTests
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"ip addr add 192.168.15.1/24 dev testbridge"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"ip -n testns route add default via 192.168.15.1 dev veth-test"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft add table nat"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft \"add chain nat POSTROUTING { type nat hook postrouting priority srcnat; }\""), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--system --user root nft add rule nat POSTROUTING ip saddr 192.168.15.0/24 oif != testbridge masquerade"), 0);
VERIFY_ARE_EQUAL(
LxsstuLaunchWsl(L"--system --user root nft \"add chain nat POSTROUTING { type nat hook postrouting priority srcnat; }\""), 0);
VERIFY_ARE_EQUAL(
LxsstuLaunchWsl(L"--system --user root nft add rule nat POSTROUTING ip saddr 192.168.15.0/24 oif != testbridge masquerade"), 0);
VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"sysctl -w net.ipv4.ip_forward=1"), 0);

// Create a listener on the Windows host on port 1234
Expand Down
Loading