Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
60 changes: 0 additions & 60 deletions test/windows/NetworkTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4283,66 +4283,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