Merge pull request #24 from CHERIoT-Platform/hlefeuvre/tcpip-crash-return-values-in-netapi

Better handle TCP/IP stack crashes in the NETAPI compartment.

Author: hlef

lib/netapi/NetAPI.cc

@@ -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)
 	{
@@ -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)
@@ -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)
 	{
@@ -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;
@@ -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(
@@ -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;
 }
 

lib/tcpip/network-internal.h

@@ -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
@@ -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);

lib/tcpip/network_wrapper.cc

@@ -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;
@@ -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,
@@ -265,7 +266,7 @@ namespace
 		}
 
 		FreeRTOS_freeaddrinfo(results);
-		return address;
+		return 0;
 	}
 
 	/**
@@ -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,
@@ -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;
 }