Skip to content

Commit

Permalink
Merge pull request #24 from CHERIoT-Platform/hlefeuvre/tcpip-crash-re…
Browse files Browse the repository at this point in the history
…turn-values-in-netapi

Better handle TCP/IP stack crashes in the NETAPI compartment.
  • Loading branch information
hlef authored May 23, 2024
2 parents ef3fe19 + 515e006 commit f7844cd
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 37 deletions.
50 changes: 42 additions & 8 deletions lib/netapi/NetAPI.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,37 @@ SObj network_socket_connect_tcp(Timeout *timeout,
Debug::log("Host capability does not authorise a TCP connection");
return nullptr;
}

NetworkAddress address{NetworkAddress::AddressKindInvalid};
CHERI::Capability addressPtr = &address;
addressPtr.permissions() &= {CHERI::Permission::Store};
firewall_permit_dns();
NetworkAddress address = network_host_resolve(host->hostname, UseIPv6);
int ret = network_host_resolve(host->hostname, UseIPv6, addressPtr);
firewall_permit_dns(false);
if (address.kind == NetworkAddress::AddressKindInvalid)
if ((ret < 0) || (address.kind == NetworkAddress::AddressKindInvalid))
{
Debug::log("Failed to resolve host");
return nullptr;
}
bool isIPv6 = address.kind == NetworkAddress::AddressKindIPv6;
auto sealedSocket = network_socket_create_and_bind(
bool isIPv6 = address.kind == NetworkAddress::AddressKindIPv6;

CHERI::Capability sealedSocket = network_socket_create_and_bind(
timeout, mallocCapability, isIPv6, ConnectionTypeTCP);
auto kind = network_socket_kind(sealedSocket);
if (!sealedSocket.is_valid())
{
Debug::log("Failed to create socket");
return nullptr;
}

SocketKind kind;
CHERI::Capability kindPtr = &kind;
kindPtr.permissions() &= {CHERI::Permission::Store};
if (network_socket_kind(sealedSocket, kindPtr) < 0)
{
Debug::log("Failed to retrieve socket kind");
return nullptr;
}

// FIXME: IPv6
if (isIPv6)
{
Expand All @@ -78,10 +97,13 @@ SObj network_socket_connect_tcp(Timeout *timeout,
firewall_add_tcpipv4_endpoint(
address.ipv4, kind.localPort, ntohs(host->port));
}

if (network_socket_connect_tcp_internal(
timeout, sealedSocket, address, host->port) != 0)
{
Timeout t{UnlimitedTimeout};
// We pass an unlimited timeout, so this cannot fail in any
// actionable manner. Don't check the return value.
network_socket_close(&t, mallocCapability, sealedSocket);
timeout->elapse(t.elapsed);
if (isIPv6)
Expand Down Expand Up @@ -118,7 +140,14 @@ NetworkAddress network_socket_udp_authorise_host(Timeout *timeout,
Debug::log("Host capability does not authorise a UDP connection");
return address;
}
auto kind = network_socket_kind(socket);

SocketKind kind;
CHERI::Capability kindPtr = &kind;
kindPtr.permissions() &= {CHERI::Permission::Store};
// No need to check the return value here, potential errors will be
// detected in the switch.
network_socket_kind(socket, kindPtr);

bool isIPv6 = false;
switch (kind.protocol)
{
Expand All @@ -133,10 +162,13 @@ NetworkAddress network_socket_udp_authorise_host(Timeout *timeout,
isIPv6 = true;
break;
}

CHERI::Capability addressPtr = &address;
addressPtr.permissions() &= {CHERI::Permission::Store};
firewall_permit_dns();
address = network_host_resolve(host->hostname, UseIPv6);
int ret = network_host_resolve(host->hostname, UseIPv6, addressPtr);
firewall_permit_dns(false);
if (address.kind == NetworkAddress::AddressKindInvalid)
if ((ret < 0) || (address.kind == NetworkAddress::AddressKindInvalid))
{
Debug::log("Failed to resolve host");
return address;
Expand All @@ -146,6 +178,7 @@ NetworkAddress network_socket_udp_authorise_host(Timeout *timeout,
Debug::log("Host address does not match socket type");
return address;
}

if (isIPv6)
{
firewall_add_udpipv6_endpoint(
Expand All @@ -161,6 +194,7 @@ NetworkAddress network_socket_udp_authorise_host(Timeout *timeout,
firewall_add_udpipv4_endpoint(
address.ipv4, kind.localPort, ntohs(host->port));
}

return address;
}

Expand Down
17 changes: 13 additions & 4 deletions lib/tcpip/network-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@
* Resolve a host name to an IPv4 or IPv6 address. If `useIPv6` is true, then
* this will first attempt to find a IPv6 address and fall back to IPv4 if none
* is found.
*
* The result of the resolve is stored in `outAddress`.
*
* This returns zero for success, or a negative value on error.
*/
NetworkAddress __cheri_compartment("TCPIP")
network_host_resolve(const char *hostname, bool useIPv6);
__cheri_compartment("TCPIP") int network_host_resolve(
const char *hostname,
bool useIPv6,
NetworkAddress *outAddress);

/**
* Create a socket and bind it to the given address. The socket will be
Expand Down Expand Up @@ -72,6 +78,9 @@ struct SocketKind
};

/**
* Returns information about the given socket.
* Returns information about the given socket in `kind`.
*
* This returns zero for success, or a negative value on error.
*/
SocketKind __cheri_compartment("TCPIP") network_socket_kind(SObj socket);
int __cheri_compartment("TCPIP")
network_socket_kind(SObj socket, SocketKind *kind);
56 changes: 31 additions & 25 deletions lib/tcpip/network_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ namespace
* will favour IPv6 addresses, but can still return IPv4 addresses if no
* IPv6 address is available.
*/
NetworkAddress host_resolve(const char *hostname, bool useIPv6 = UseIPv6)
int
host_resolve(const char *hostname, bool useIPv6, NetworkAddress *address)
{
struct AddrinfoHints hints;
hints.family = useIPv6 ? FREERTOS_AF_INET6 : FREERTOS_AF_INET;
Expand All @@ -229,31 +230,31 @@ namespace
// Try with IPv4 if the lookup failed with IPv6
if (useIPv6)
{
return host_resolve(hostname, false);
return host_resolve(hostname, false, address);
}
Debug::log("DNS request returned: {}", ret);
return {0, NetworkAddress::AddressKindInvalid};
address->kind = NetworkAddress::AddressKindInvalid;
address->ipv4 = 0;
return ret;
}

NetworkAddress address;
bool isIPv6 = false;
bool isIPv6 = false;
for (freertos_addrinfo *r = results; r != nullptr; r = r->ai_next)
{
Debug::log("Canonical name: {}", r->ai_canonname);
if (r->ai_family == FREERTOS_AF_INET6)
{
memcpy(
address.ipv6, r->ai_addr->sin_address.xIP_IPv6.ucBytes, 16);
address.kind = NetworkAddress::AddressKindIPv6;
address->ipv6, r->ai_addr->sin_address.xIP_IPv6.ucBytes, 16);
address->kind = NetworkAddress::AddressKindIPv6;
Debug::log("Got IPv6 address");
}
else
{
address.ipv4 = r->ai_addr->sin_address.ulIP_IPv4;
address.kind = NetworkAddress::AddressKindIPv4;
Debug::log("Got IPv4 address");
address->ipv4 = r->ai_addr->sin_address.ulIP_IPv4;
address->kind = NetworkAddress::AddressKindIPv4;
Debug::log(
"Got address: {}.{}.{}.{}",
"Got IPv4 address: {}.{}.{}.{}",
static_cast<int>(r->ai_addr->sin_address.ulIP_IPv4) & 0xff,
static_cast<int>(r->ai_addr->sin_address.ulIP_IPv4) >> 8 &
0xff,
Expand All @@ -265,7 +266,7 @@ namespace
}

FreeRTOS_freeaddrinfo(results);
return address;
return 0;
}

/**
Expand Down Expand Up @@ -422,9 +423,11 @@ namespace
}
} // namespace

NetworkAddress network_host_resolve(const char *hostname, bool useIPv6)
int network_host_resolve(const char *hostname,
bool useIPv6,
NetworkAddress *address)
{
return host_resolve(hostname, useIPv6);
return host_resolve(hostname, useIPv6, address);
}

SObj network_socket_create_and_bind(Timeout *timeout,
Expand Down Expand Up @@ -959,27 +962,30 @@ ssize_t network_socket_send_to(Timeout *timeout,
socket);
}

SocketKind network_socket_kind(SObj socket)
int network_socket_kind(SObj socket, SocketKind *kind)
{
SocketKind kind = {SocketKind::Invalid, 0};
with_sealed_socket(
kind->protocol = SocketKind::Invalid;
kind->localPort = 0;

int ret = with_sealed_socket(
[&](SealedSocket *socket) {
if (socket->socket->ucProtocol == FREERTOS_IPPROTO_TCP)
{
kind.protocol = socket->socket->bits.bIsIPv6
? SocketKind::TCPIPv6
: SocketKind::TCPIPv4;
kind->protocol = socket->socket->bits.bIsIPv6
? SocketKind::TCPIPv6
: SocketKind::TCPIPv4;
}
else
{
kind.protocol = socket->socket->bits.bIsIPv6
? SocketKind::UDPIPv6
: SocketKind::UDPIPv4;
kind->protocol = socket->socket->bits.bIsIPv6
? SocketKind::UDPIPv6
: SocketKind::UDPIPv4;
}
kind.localPort = listGET_LIST_ITEM_VALUE(
kind->localPort = listGET_LIST_ITEM_VALUE(
(&((socket->socket)->xBoundSocketListItem)));
return 0;
},
socket);
return kind;

return ret;
}

0 comments on commit f7844cd

Please sign in to comment.