From 01564818773609ea44ed0bd24ae8d82c2391c27f Mon Sep 17 00:00:00 2001 From: Owen Anderson Date: Wed, 19 Feb 2025 16:35:56 +1300 Subject: [PATCH] Enable -Wcheri-compartment-return-void for lib/dns --- include/NetAPI.h | 4 +- lib/dns/dns.cc | 32 +++++++++------ lib/dns/xmake.lua | 1 + lib/firewall/firewall.cc | 86 ++++++++++++++++++++++++---------------- lib/firewall/firewall.hh | 40 +++++++++---------- 5 files changed, 94 insertions(+), 69 deletions(-) diff --git a/include/NetAPI.h b/include/NetAPI.h index a1f0731..8351ccf 100644 --- a/include/NetAPI.h +++ b/include/NetAPI.h @@ -181,7 +181,7 @@ typedef CHERI_SEALED(struct SealedSocket *) Socket; * Start the network. This is a temporary API. It will eventually be replaced * by a non-blocking version. */ -void __cheri_compartment("TCPIP") network_start(void); +int __cheri_compartment("TCPIP") network_start(void); /** * Create a connected TCP socket. @@ -431,4 +431,4 @@ const char *__cheri_compartment("NetAPI") * * This is disabled unless compiled with the `network-inject-faults` option. */ -void __cheri_compartment("TCPIP") network_inject_fault(void); +int __cheri_compartment("TCPIP") network_inject_fault(void); diff --git a/lib/dns/dns.cc b/lib/dns/dns.cc index 6878222..baf240c 100644 --- a/lib/dns/dns.cc +++ b/lib/dns/dns.cc @@ -201,7 +201,7 @@ namespace * Note: if our own IP address has not yet been determined, this will * send an ARP probe. */ - void send_arp_request(uint32_t ip) + int send_arp_request(uint32_t ip) { struct FullARPPacket *arpPacket = reinterpret_cast(packetBuffer); @@ -221,14 +221,14 @@ namespace arpPacket->arp.spa = deviceIP; arpPacket->arp.tpa = ip; - ethernet_send_frame(packetBuffer, sizeof(FullARPPacket)); + return ethernet_send_frame(packetBuffer, sizeof(FullARPPacket)); } /** * Send a DNS query for passed `hostname` of length `length` (not * including the zero terminator). */ - void send_dns_query(const char *hostname, size_t length, bool askIPv6) + int send_dns_query(const char *hostname, size_t length, bool askIPv6) { Debug::log("Sending a DNS query for {} (IPv6: {})", hostname, askIPv6); @@ -294,7 +294,7 @@ namespace // Request IN (Internet) class information *reinterpret_cast(question + length + 4) = DNSClassIN; - ethernet_send_frame(packetBuffer, packetSize); + return ethernet_send_frame(packetBuffer, packetSize); } /** @@ -365,9 +365,9 @@ namespace * negotiated with the new DHCP server by the network stack. This is an * edge-case which we should be able to safely ignore for now. */ - void process_incoming_dhcp_packet(const uint8_t *dhcpPacket, - size_t length, - EthernetHeader *ethernetHeader) + int process_incoming_dhcp_packet(const uint8_t *dhcpPacket, + size_t length, + EthernetHeader *ethernetHeader) { // DHCP packets may be updating our IP address // or the address of the gateway. @@ -378,7 +378,7 @@ namespace if (sizeof(DHCPHeader) > length) { Debug::log("Ignoring truncated DHCP packet"); - return; + return 0; } auto *dhcpHeader = reinterpret_cast(dhcpPacket); size_t currentOffset = sizeof(DHCPHeader); @@ -387,7 +387,7 @@ namespace { Debug::log("Ignoring DHCP packet with incorrect magic cookie {}", dhcpHeader->cookie); - return; + return 0; } // Go through the options to get the DHCP @@ -497,7 +497,7 @@ namespace { Debug::log("DHCP OFFER does not provide DNS server IP, " "gateway, or mask."); - return; + return 0; } dnsServerIP = extractedDnsServerIP; @@ -507,7 +507,9 @@ namespace static_cast(dnsServerIP >> 8) & 0xff, static_cast(dnsServerIP >> 16) & 0xff, static_cast(dnsServerIP >> 24) & 0xff); - firewall_dns_server_ip_set(dnsServerIP); + int err = firewall_dns_server_ip_set(dnsServerIP); + if (err < 0) + return err; gatewayIP = extractedGateway; Debug::log("The gateway IP is {}.{}.{}.{}", @@ -588,6 +590,8 @@ namespace deviceIP = dhcpHeader->yiaddr; state |= ResolverState::DeviceIPSet; } + + return 0; } /** @@ -902,11 +906,12 @@ namespace * This must be called by the firewall exclusively (checked via rego), before * any other API of the DNS resolver. */ -__cheri_compartment("DNS") void initialize_dns_resolver(uint8_t *macAddress) +__cheri_compartment("DNS") int initialize_dns_resolver(uint8_t *macAddress) { Debug::log("Initializing the DNS resolver."); memcpy(deviceMAC.data(), macAddress, 6); state |= ResolverState::DeviceMACSet; + return 0; } /** @@ -921,7 +926,7 @@ __cheri_compartment("DNS") void initialize_dns_resolver(uint8_t *macAddress) * This does not currently work with DHCP lease renewal if the address of the * gateway changes, but neither does the firewall. */ -void __cheri_compartment("DNS") +int __cheri_compartment("DNS") dns_resolver_receive_frame(uint8_t *packet, size_t length) { on_error( @@ -1017,6 +1022,7 @@ void __cheri_compartment("DNS") "Crashed while the DNS resolver was not yet initialized. This " "may result in a non-functional resolver."); }); + return 0; } /** diff --git a/lib/dns/xmake.lua b/lib/dns/xmake.lua index 5f1e54a..ddf655b 100644 --- a/lib/dns/xmake.lua +++ b/lib/dns/xmake.lua @@ -3,4 +3,5 @@ compartment("DNS") add_includedirs("../../include") add_rules("cheriot.network-stack.ipv6") add_files("dns.cc") + add_cxflags("-Wcheri-compartment-return-void") diff --git a/lib/firewall/firewall.cc b/lib/firewall/firewall.cc index 28fe77c..cd0b3ad 100644 --- a/lib/firewall/firewall.cc +++ b/lib/firewall/firewall.cc @@ -971,7 +971,7 @@ bool ethernet_link_is_up() return ethernet.phy_link_status(); } -void firewall_dns_server_ip_set(uint32_t ip) +int firewall_dns_server_ip_set(uint32_t ip) { // This is potentially racy but, since it's called very early in network // stack initialisation, it's not worth worrying about an attacker being @@ -982,40 +982,46 @@ void firewall_dns_server_ip_set(uint32_t ip) dnsServerAddress = ip; } Debug::log("DNS server address set to {}", ip); + return 0; } -void firewall_permit_dns(bool dnsIsPermitted) +int firewall_permit_dns(bool dnsIsPermitted) { ::dnsIsPermitted += dnsIsPermitted ? 1 : -1; + return 0; } -void firewall_add_tcpipv4_server_port(uint16_t localPort) +int firewall_add_tcpipv4_server_port(uint16_t localPort) { EndpointsTable::instance().add_server_port(localPort); + return 0; } -void firewall_remove_tcpipv4_server_port(uint16_t localPort) +int firewall_remove_tcpipv4_server_port(uint16_t localPort) { EndpointsTable::instance().remove_server_port(localPort); + return 0; } -void firewall_add_tcpipv4_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort) +int firewall_add_tcpipv4_endpoint(uint32_t remoteAddress, + uint16_t localPort, + uint16_t remotePort) { EndpointsTable::instance().add_endpoint( IPProtocolNumber::TCP, remoteAddress, localPort, remotePort); + return 0; } -void firewall_add_udpipv4_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort) +int firewall_add_udpipv4_endpoint(uint32_t remoteAddress, + uint16_t localPort, + uint16_t remotePort) { EndpointsTable::instance().add_endpoint( IPProtocolNumber::UDP, remoteAddress, localPort, remotePort); + return 0; } -void firewall_remove_tcpipv4_local_endpoint(uint16_t localPort) +int firewall_remove_tcpipv4_local_endpoint(uint16_t localPort) { // Server ports are likely to be associated to more than one entry in // the firewall. @@ -1024,11 +1030,12 @@ void firewall_remove_tcpipv4_local_endpoint(uint16_t localPort) "Trying to remove a local endpoint on a server port."); EndpointsTable::instance().remove_endpoint(IPProtocolNumber::TCP, localPort); + return 0; } -void firewall_remove_tcpipv4_remote_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort) +int firewall_remove_tcpipv4_remote_endpoint(uint32_t remoteAddress, + uint16_t localPort, + uint16_t remotePort) { EndpointsTable::instance().remove_endpoint( IPProtocolNumber::TCP, remoteAddress, localPort, remotePort); @@ -1036,20 +1043,23 @@ void firewall_remove_tcpipv4_remote_endpoint(uint32_t remoteAddress, { currentClientCount--; } + return 0; } -void firewall_remove_udpipv4_local_endpoint(uint16_t localPort) +int firewall_remove_udpipv4_local_endpoint(uint16_t localPort) { EndpointsTable::instance().remove_endpoint(IPProtocolNumber::UDP, localPort); + return 0; } -void firewall_remove_udpipv4_remote_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort) +int firewall_remove_udpipv4_remote_endpoint(uint32_t remoteAddress, + uint16_t localPort, + uint16_t remotePort) { EndpointsTable::instance().remove_endpoint( IPProtocolNumber::UDP, remoteAddress, localPort, remotePort); + return 0; } namespace @@ -1073,50 +1083,55 @@ namespace } // namespace #if CHERIOT_RTOS_OPTION_IPv6 -void firewall_add_tcpipv6_server_port(uint16_t localPort) +int firewall_add_tcpipv6_server_port(uint16_t localPort) { EndpointsTable::instance().add_server_port(localPort); + return 0; } -void firewall_remove_tcpipv6_server_port(uint16_t localPort) +int firewall_remove_tcpipv6_server_port(uint16_t localPort) { EndpointsTable::instance().remove_server_port(localPort); + return 0; } -void firewall_add_tcpipv6_endpoint(uint8_t *remoteAddress, - uint16_t localPort, - uint16_t remotePort) +int firewall_add_tcpipv6_endpoint(uint8_t *remoteAddress, + uint16_t localPort, + uint16_t remotePort) { if (auto copy = copy_address(remoteAddress)) { EndpointsTable::instance().add_endpoint( IPProtocolNumber::TCP, *copy, localPort, remotePort); } + return 0; } -void firewall_add_udpipv6_endpoint(uint8_t *remoteAddress, - uint16_t localPort, - uint16_t remotePort) +int firewall_add_udpipv6_endpoint(uint8_t *remoteAddress, + uint16_t localPort, + uint16_t remotePort) { if (auto copy = copy_address(remoteAddress)) { EndpointsTable::instance().add_endpoint( IPProtocolNumber::UDP, *copy, localPort, remotePort); } + return 0; } -void firewall_remove_tcpipv6_local_endpoint(uint16_t localPort) +int firewall_remove_tcpipv6_local_endpoint(uint16_t localPort) { Debug::Assert( !EndpointsTable::instance().is_server_port(localPort), "Trying to remove a local endpoint on a server port."); EndpointsTable::instance().remove_endpoint( IPProtocolNumber::TCP, localPort); + return 0; } -void firewall_remove_tcpipv6_remote_endpoint(uint8_t *remoteAddress, - uint16_t localPort, - uint16_t remotePort) +int firewall_remove_tcpipv6_remote_endpoint(uint8_t *remoteAddress, + uint16_t localPort, + uint16_t remotePort) { if (auto copy = copy_address(remoteAddress)) { @@ -1127,23 +1142,26 @@ void firewall_remove_tcpipv6_remote_endpoint(uint8_t *remoteAddress, currentClientCount--; } } + return 0; } -void firewall_remove_udpipv6_local_endpoint(uint16_t localPort) +int firewall_remove_udpipv6_local_endpoint(uint16_t localPort) { EndpointsTable::instance().remove_endpoint( IPProtocolNumber::UDP, localPort); + return 0; } -void firewall_remove_udpipv6_remote_endpoint(uint8_t *remoteAddress, - uint16_t localPort, - uint16_t remotePort) +int firewall_remove_udpipv6_remote_endpoint(uint8_t *remoteAddress, + uint16_t localPort, + uint16_t remotePort) { if (auto copy = copy_address(remoteAddress)) { EndpointsTable::instance().remove_endpoint( IPProtocolNumber::UDP, *copy, localPort, remotePort); } + return 0; } #endif diff --git a/lib/firewall/firewall.hh b/lib/firewall/firewall.hh index 65b0108..f1c8aed 100644 --- a/lib/firewall/firewall.hh +++ b/lib/firewall/firewall.hh @@ -74,14 +74,14 @@ bool __cheri_compartment("TCPIP") * The DNS resolver expects to be passed all ARP, DHCP, and DNS packets. This * does not support IPv6 for now. */ -void __cheri_compartment("DNS") +int __cheri_compartment("DNS") dns_resolver_receive_frame(uint8_t *packet, size_t length); /** * Initialize the DNS resolver. This must be passed the `macAddress` of the * device. */ -void __cheri_compartment("DNS") initialize_dns_resolver(uint8_t *macAddress); +int __cheri_compartment("DNS") initialize_dns_resolver(uint8_t *macAddress); /** * Set the IP address of the DNS server to use. Packets to and from this @@ -89,7 +89,7 @@ void __cheri_compartment("DNS") initialize_dns_resolver(uint8_t *macAddress); * * This should only be called from the TCP/IP compartment. */ -void __cheri_compartment("Firewall") firewall_dns_server_ip_set(uint32_t ip); +int __cheri_compartment("Firewall") firewall_dns_server_ip_set(uint32_t ip); /** * Toggle whether DNS is permitted. This is used to open a hole in the @@ -97,7 +97,7 @@ void __cheri_compartment("Firewall") firewall_dns_server_ip_set(uint32_t ip); * * This should be called only by the NetAPI compartment. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_permit_dns(bool dnsIsPermitted = true); /** @@ -107,7 +107,7 @@ void __cheri_compartment("Firewall") * * This should be called only by the NetAPI compartment. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_add_tcpipv4_endpoint(uint32_t remoteAddress, uint16_t localPort, uint16_t remotePort); @@ -119,7 +119,7 @@ void __cheri_compartment("Firewall") * * This should be called only by the NetAPI compartment. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_add_udpipv4_endpoint(uint32_t remoteAddress, uint16_t localPort, uint16_t remotePort); @@ -135,7 +135,7 @@ void __cheri_compartment("Firewall") * calling it is DoS itself. There is limited risk that it would fail to call * it when a connection should be closed. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_tcpipv4_local_endpoint(uint16_t localPort); /** @@ -144,7 +144,7 @@ void __cheri_compartment("Firewall") * This is called from the TCP/IP compartment when a TCP connection is closed * (see discussion in `firewall_remove_tcpipv4_local_endpoint`). */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_tcpipv4_remote_endpoint(uint32_t remoteAddress, uint16_t localPort, uint16_t remotePort); @@ -157,7 +157,7 @@ void __cheri_compartment("Firewall") * can do by calling it is DoS itself. There is limited risk that it would * fail to call it when a connection should be closed. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_udpipv4_local_endpoint(uint16_t endpoint); /** @@ -166,7 +166,7 @@ void __cheri_compartment("Firewall") * This is called from the TCP/IP compartment when a UDP "connection" is closed * (see discussion in `firewall_remove_udpipv4_local_endpoint`). */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_udpipv4_remote_endpoint(uint32_t remoteAddress, uint16_t localPort, uint16_t remotePort); @@ -189,7 +189,7 @@ void __cheri_compartment("Firewall") * * This should be called only by the NetAPI compartment. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_add_tcpipv4_server_port(uint16_t localPort); /** @@ -200,7 +200,7 @@ void __cheri_compartment("Firewall") * security risk: the worst that the TCP/IP compartment can do by calling it is * DoS itself. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_tcpipv4_server_port(uint16_t localPort); #if CHERIOT_RTOS_OPTION_IPv6 @@ -211,7 +211,7 @@ void __cheri_compartment("Firewall") * * This should be called only by the NetAPI compartment. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_add_tcpipv6_endpoint(uint8_t *remoteAddress, uint16_t localPort, uint16_t remotePort); @@ -223,7 +223,7 @@ void __cheri_compartment("Firewall") * * This should be called only by the NetAPI compartment. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_add_udpipv6_endpoint(uint8_t *remoteAddress, uint16_t localPort, uint16_t remotePort); @@ -239,7 +239,7 @@ void __cheri_compartment("Firewall") * calling it is DoS itself. There is limited risk that it would fail to call * it when a connection should be closed. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_tcpipv6_local_endpoint(uint16_t localPort); /** @@ -248,7 +248,7 @@ void __cheri_compartment("Firewall") * This is called from the TCP/IP compartment when a TCP connection is closed * (see discussion in `firewall_remove_tcpipv6_local_endpoint`). */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_tcpipv6_remote_endpoint(uint8_t *remoteAddress, uint16_t localPort, uint16_t remotePort); @@ -261,7 +261,7 @@ void __cheri_compartment("Firewall") * can do by calling it is DoS itself. There is limited risk that it would * fail to call it when a connection should be closed. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_udpipv6_local_endpoint(uint16_t endpoint); /** @@ -270,7 +270,7 @@ void __cheri_compartment("Firewall") * This is called from the TCP/IP compartment when a UDP "connection" is closed * (see discussion in `firewall_remove_udpipv6_local_endpoint`). */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_udpipv6_remote_endpoint(uint8_t *remoteAddress, uint16_t localPort, uint16_t remotePort); @@ -286,13 +286,13 @@ void __cheri_compartment("Firewall") * * This should be called only by the NetAPI compartment. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_add_tcpipv6_server_port(uint16_t localPort); /** * Remove a server port from the firewall. */ -void __cheri_compartment("Firewall") +int __cheri_compartment("Firewall") firewall_remove_tcpipv6_server_port(uint16_t localPort); #else