Skip to content

Firewall: keep the TCP hole open for the final ACK #111

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.

Sign up for GitHub

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

Open
nding0405 wants to merge 3 commits into
base: main
Choose a base branch
from
+288 −26
Open

Firewall: keep the TCP hole open for the final ACK #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
92b2144
Firewall: keep the TCP hole open for the final ACK
nding0405 Aug 7, 2026
c4c4eb5
Firewall: allow TCP hole state query errors
nding0405 Aug 8, 2026
945b9fd
Clarify comments in TCPFirewallState enum
nding0405 Aug 8, 2026
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
191 changes: 169 additions & 22 deletions lib/firewall/firewall.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

#include <atomic>
#include <compartment-macros.h>
#include <cstddef>
#include <debug.hh>
// #include <fail-simulator-on-error.h>
#include <endianness.hh>
#include <errno.h>
#include <locks.hh>
#include <platform-entropy.hh>
#include <platform-ethernet.hh>
Expand Down Expand Up @@ -37,15 +39,16 @@ namespace
__noinline void insert(void *buffer,
size_t bufferSize,
const void *element,
size_t elementSize)
size_t elementSize,
size_t keySize)
{
// This currently does a linear search. This is less code than a
// binary search and we don't insert on a hot path, so this should
// be fine.
for (size_t i = 0; i < bufferSize; i += elementSize)
{
void *current = reinterpret_cast<uint8_t *>(buffer) + i;
if (memcmp(current, element, elementSize) > 0)
if (memcmp(current, element, keySize) > 0)
{
memmove(reinterpret_cast<uint8_t *>(current) + elementSize,
current,
Expand All @@ -66,7 +69,8 @@ namespace
__noinline void *binary_search(void *buffer,
size_t bufferSize,
const void *element,
size_t elementSize)
size_t elementSize,
size_t comparisonSize)
{
if (bufferSize > 0)
{
Expand All @@ -77,7 +81,7 @@ namespace
size_t mid = low + (high - low) / 2;
void *current =
reinterpret_cast<uint8_t *>(buffer) + (mid * elementSize);
int comparison = memcmp(current, element, elementSize);
int comparison = memcmp(current, element, comparisonSize);
if (comparison == 0)
{
return current;
Expand Down Expand Up @@ -107,10 +111,11 @@ namespace
__noinline bool remove(void *buffer,
size_t bufferSize,
const void *element,
size_t elementSize)
size_t elementSize,
size_t keySize)
{
void *found =
binary_search(buffer, bufferSize, element, elementSize);
binary_search(buffer, bufferSize, element, elementSize, keySize);
if (found == nullptr)
{
return false;
Expand Down Expand Up @@ -153,11 +158,11 @@ namespace
/**
* A simple table of `T`s, stored as a sorted array. This uses `memcmp` and
* `memcpy` to compare and copy elements and so requires that `T` is a
* trivial type.
* trivial type. The first `KeySize` bytes form the table key.
*
* This never shrinks and does a full copy if it needs to grow.
*/
template<typename T>
template<typename T, size_t KeySize = sizeof(T)>
class SmallTable : public SmallTableBase
{
static_assert(std::is_trivial_v<T>, "T must be a trivial type");
Expand Down Expand Up @@ -252,8 +257,11 @@ namespace

buffer = CHERI::Capability<T>{static_cast<T *>(currentBase)};

SmallTableBase::insert(
currentBase, currentSize * sizeof(T), &element, sizeof(T));
SmallTableBase::insert(currentBase,
currentSize * sizeof(T),
&element,
sizeof(T),
KeySize);
set_size(currentSize + 1);
}

Expand All @@ -264,7 +272,7 @@ namespace
bool remove(const T &element)
{
if (SmallTableBase::remove(
base(), size() * sizeof(T), &element, sizeof(T)))
base(), size() * sizeof(T), &element, sizeof(T), KeySize))
{
set_size(size() - 1);
return true;
Expand All @@ -286,14 +294,21 @@ namespace
set_size(size() - 1);
}

/**
* Returns the matching element, or `nullptr` if it is not present.
*/
T *find(const T &element)
{
return static_cast<T *>(binary_search(
base(), size() * sizeof(T), &element, sizeof(T), KeySize));
}

/**
* Returns true if the table contains the given element.
*/
bool contains(const T &element)
{
return binary_search(
base(), size() * sizeof(T), &element, sizeof(T)) !=
nullptr;
return find(element) != nullptr;
}

/**
Expand Down Expand Up @@ -523,14 +538,19 @@ namespace
Address remoteAddress;
uint16_t localPort;
uint16_t remotePort;
// Tracks whether this hole is open, closing, or safe to remove.
TCPFirewallState state;
// A clang-tidy bug thinks that this should be = nullptr instead of
// = default.
auto operator<=>(const ConnectionTuple &) const = default; // NOLINT
};
SmallTable<uint16_t> tcpServerPorts;
SmallTable<ConnectionTuple> permittedTCPEndpoints;
SmallTable<ConnectionTuple> permittedUDPEndpoints;
FlagLockPriorityInherited permittedEndpointsLock;
using ConnectionTable =
SmallTable<ConnectionTuple, offsetof(ConnectionTuple, state)>;

SmallTable<uint16_t> tcpServerPorts;
ConnectionTable permittedTCPEndpoints;
ConnectionTable permittedUDPEndpoints;
FlagLockPriorityInherited permittedEndpointsLock;

using GuardedTable =
std::pair<LockGuard<decltype(permittedEndpointsLock)>,
Expand All @@ -548,6 +568,17 @@ namespace
: permittedUDPEndpoints};
}

/**
* Find a hole by endpoint without comparing its state.
*
* Returns the matching hole, or `nullptr` if none is found.
*/
ConnectionTuple *find_endpoint(ConnectionTable &table,
const ConnectionTuple &key)
{
return table.find(key);
}

public:
static EndpointsTable &instance()
{
Expand All @@ -574,10 +605,87 @@ namespace
// auto [g, table] = permitted_endpoints(protocol);
auto guardedTable = permitted_endpoints(protocol);
auto &[g, table] = guardedTable;
ConnectionTuple tuple{endpoint, localPort, remotePort};
ConnectionTuple tuple{
endpoint, localPort, remotePort, TCPFirewallState::InUse};
return table.remove(tuple);
}

/**
* Change a TCP hole from `InUse` to `InTermination`.
*
* Returns:
*
* - 0 on success.
* - `-ENOENT` if no in-use hole matches.
*/
int mark_tcp_endpoint_in_termination(Address remoteAddress,
uint16_t localPort,
uint16_t remotePort)
{
LockGuard g{permittedEndpointsLock};
ConnectionTuple key{
remoteAddress, localPort, remotePort, TCPFirewallState::InUse};
if (ConnectionTuple *tuple =
find_endpoint(permittedTCPEndpoints, key);
(tuple != nullptr) && (tuple->state == TCPFirewallState::InUse))
{
// The final packet still needs this hole.
tuple->state = TCPFirewallState::InTermination;
return 0;
}
return -ENOENT;
}

/**
* Change a TCP hole from `InTermination` to `CanBeRemoved`.
*
* Returns:
*
* - 0 on success.
* - `-ENOENT` if no closing hole matches.
*/
int mark_tcp_endpoint_can_be_removed(Address remoteAddress,
uint16_t localPort,
uint16_t remotePort)
{
LockGuard g{permittedEndpointsLock};
ConnectionTuple key{remoteAddress,
localPort,
remotePort,
TCPFirewallState::InTermination};
if (ConnectionTuple *tuple =
find_endpoint(permittedTCPEndpoints, key);
(tuple != nullptr) &&
(tuple->state == TCPFirewallState::InTermination))
{
// The final packet passed the filter.
tuple->state = TCPFirewallState::CanBeRemoved;
return 0;
}
return -ENOENT;
}

/**
* Get a TCP hole's state without changing it.
*
* Returns the state, or `TCPFirewallState::NotFound` if no hole
* matches.
*/
TCPFirewallState tcp_endpoint_state(Address remoteAddress,
uint16_t localPort,
uint16_t remotePort)
{
LockGuard g{permittedEndpointsLock};
ConnectionTuple key{
remoteAddress, localPort, remotePort, TCPFirewallState::InUse};
if (ConnectionTuple *tuple =
find_endpoint(permittedTCPEndpoints, key))
{
return tuple->state;
}
return TCPFirewallState::NotFound;
}

void add_server_port(uint16_t localPort)
{
LockGuard g{permittedEndpointsLock};
Expand Down Expand Up @@ -607,8 +715,12 @@ namespace
// auto [g, table] = permitted_endpoints(protocol);
auto guardedTable = permitted_endpoints(protocol);
auto &[g, table] = guardedTable;
ConnectionTuple tuple{remoteAddress, localPort, remotePort};
table.insert(tuple);
ConnectionTuple tuple{
remoteAddress, localPort, remotePort, TCPFirewallState::InUse};
if (!table.contains(tuple))
{
table.insert(tuple);
}
}

void remove_endpoint(IPProtocolNumber protocol, uint16_t localPort)
Expand Down Expand Up @@ -643,7 +755,8 @@ namespace
// auto [g, table] = permitted_endpoints(protocol);
auto guardedTable = permitted_endpoints(protocol);
auto &[g, table] = guardedTable;
ConnectionTuple tuple{endpoint, localPort, remotePort};
ConnectionTuple tuple{
endpoint, localPort, remotePort, TCPFirewallState::InUse};
return table.contains(tuple);
}
};
Expand Down Expand Up @@ -881,6 +994,23 @@ namespace
else
{
Debug::log("Permitting outbound IPv4 packet");

const auto *ipv4Header =
reinterpret_cast<const IPv4Header *>(
data + sizeof(EthernetHeader));
if (ipv4Header->protocol == IPProtocolNumber::TCP)
{
const auto *tcpHeader =
reinterpret_cast<const TCPUDPCommonPrefix *>(
reinterpret_cast<const uint8_t *>(ipv4Header) +
ipv4Header->body_offset());
// The packet passed; its hole may now be removed.
(void)EndpointsTable<uint32_t>::instance()
.mark_tcp_endpoint_can_be_removed(
ipv4Header->destinationAddress,
tcpHeader->sourcePort,
tcpHeader->destinationPort);
}
}
return ret;
}
Expand Down Expand Up @@ -1067,6 +1197,23 @@ void firewall_add_tcpipv4_endpoint(uint32_t remoteAddress,
IPProtocolNumber::TCP, remoteAddress, localPort, remotePort);
}

int firewall_mark_tcpipv4_endpoint_in_termination(uint32_t remoteAddress,
uint16_t localPort,
uint16_t remotePort)
{
return EndpointsTable<uint32_t>::instance()
.mark_tcp_endpoint_in_termination(remoteAddress, localPort, remotePort);
}

int firewall_get_tcpipv4_endpoint_state(uint32_t remoteAddress,
uint16_t localPort,
uint16_t remotePort)
{
return static_cast<int>(
EndpointsTable<uint32_t>::instance().tcp_endpoint_state(
remoteAddress, localPort, remotePort));
}

void firewall_add_udpipv4_endpoint(uint32_t remoteAddress,
uint16_t localPort,
uint16_t remotePort)
Expand Down
44 changes: 44 additions & 0 deletions lib/firewall/firewall.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,50 @@ void __cheri_compartment("Firewall")
uint16_t localPort,
uint16_t remotePort);

/**
* State of an IPv4 TCP firewall hole during close.
*/
enum class TCPFirewallState : uint8_t
{
/// The socket is active and packets may pass through this hole.
InUse = 0,
/// The socket is closing and its final ACK has not passed egress.
InTermination = 1,
/// The final ACK passed egress and the hole may be removed.
CanBeRemoved = 2,
/// No hole matches the remote address and local and remote ports.
NotFound = 3,
};

/**
* Change a TCP hole from `InUse` to `InTermination`, keeping it open for the
* final egress packet.
*
* Returns:
*
* - 0 on success.
* - `-ENOENT` if no in-use hole matches.
*/
int __cheri_compartment("Firewall")
firewall_mark_tcpipv4_endpoint_in_termination(uint32_t remoteAddress,
uint16_t localPort,
uint16_t remotePort);

/**
* Get a TCP hole's state without changing it.
*
* Returns:
*
* - A `TCPFirewallState` value on success.
* - `-ENOTENOUGHSTACK` if there is not enough stack to call the compartment.
* - `-ENOTENOUGHTRUSTEDSTACK` if there is not enough trusted stack.
* - `-ECOMPARTMENTFAIL` if the firewall compartment fails.
*/

@davidchisnall davidchisnall Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that this can also fail for normal reasons so the return value might be a negative errno value.

int __cheri_compartment("Firewall")
firewall_get_tcpipv4_endpoint_state(uint32_t remoteAddress,
uint16_t localPort,
uint16_t remotePort);

/**
* Open a hole in the firewall for UDP packets to and from the given endpoint.
* This permits inbound packets to, and outbound packets from, the specified
Expand Down
Loading
Loading