diff --git a/lib/firewall/firewall.cc b/lib/firewall/firewall.cc index d83928d..9a320dc 100644 --- a/lib/firewall/firewall.cc +++ b/lib/firewall/firewall.cc @@ -13,6 +13,7 @@ #include #include +#include "protocol-addresses-debug.hh" #include "protocol-headers.hh" using Debug = ConditionalDebug; @@ -472,8 +473,8 @@ namespace } else { - std::array macAddress; - EntropySource entropy; + MACAddress macAddress; + EntropySource entropy; for (auto &byte : macAddress) { byte = entropy(); @@ -483,13 +484,7 @@ namespace macAddress[0] |= 0b10; // Make sure that the broadcast bit is 0 macAddress[0] &= ~0b1; - Debug::log("MAC address: {}:{}:{}:{}:{}:{}", - macAddress[0], - macAddress[1], - macAddress[2], - macAddress[3], - macAddress[4], - macAddress[5]); + Debug::log("MAC address: {}", macAddress); return macAddress; } }(); @@ -545,7 +540,7 @@ namespace return GuardedTable{LockGuard{permittedEndpointsLock}, protocol == IPProtocolNumber::TCP ? permittedTCPEndpoints - : permittedUDPEndpoints}; + : permittedUDPEndpoints}; } public: @@ -665,7 +660,7 @@ namespace return false; } - uint32_t dnsServerAddress; + IPv4Address dnsServerAddress; _Atomic(uint32_t) dnsIsPermitted; /** @@ -693,9 +688,9 @@ namespace ForwardFlags packet_filter_ipv4(const uint8_t *data, size_t length, - uint32_t(IPv4Header::*remoteAddress), - uint16_t(TCPUDPCommonPrefix::*localPort), - uint16_t(TCPUDPCommonPrefix::*remotePort), + IPv4Address IPv4Header::*remoteAddress, + uint16_t TCPUDPCommonPrefix::*localPort, + uint16_t TCPUDPCommonPrefix::*remotePort, bool permitBroadcast) { if (__predict_false(length < sizeof(IPv4Header))) @@ -746,9 +741,9 @@ namespace auto *tcpudpHeader = reinterpret_cast( data + ipv4Header->body_offset()); - uint32_t endpoint = ipv4Header->*remoteAddress; - uint16_t localPortNumber = tcpudpHeader->*localPort; - uint16_t remotePortNumber = tcpudpHeader->*remotePort; + IPv4Address endpoint = ipv4Header->*remoteAddress; + uint16_t localPortNumber = tcpudpHeader->*localPort; + uint16_t remotePortNumber = tcpudpHeader->*remotePort; bool isIngress = (remoteAddress == &IPv4Header::sourceAddress); // Permit DNS requests during a DNS query. if (dnsIsPermitted > 0) @@ -764,19 +759,16 @@ namespace return ForwardFlags::ForwardDNS; } } - if (EndpointsTable::instance().is_endpoint_permitted( - ipv4Header->protocol, - endpoint, - localPortNumber, - remotePortNumber)) + if (EndpointsTable::instance() + .is_endpoint_permitted(ipv4Header->protocol, + endpoint, + localPortNumber, + remotePortNumber)) { - Debug::log("Permitting {} {} {}.{}.{}.{}", + Debug::log("Permitting {} {} {}", ipv4Header->protocol, isIngress ? "from" : "to", - static_cast(endpoint) & 0xff, - static_cast(endpoint >> 8) & 0xff, - static_cast(endpoint >> 16) & 0xff, - static_cast(endpoint >> 24) & 0xff); + endpoint); return ForwardFlags::ForwardNetworkStack; } // First SYN to a local server port should @@ -787,7 +779,7 @@ namespace // (e.g., retransmissions). if ((isIngress) && (ipv4Header->protocol == IPProtocolNumber::TCP) && - (EndpointsTable::instance().is_server_port( + (EndpointsTable::instance().is_server_port( localPortNumber))) { if (ipv4Header->body_offset() + sizeof(TCPHeader) > length) @@ -814,13 +806,10 @@ namespace } currentClientCount++; Debug::log("Permitting new client TCP connection from " - "{}.{}.{}.{}:{}", - static_cast(endpoint) & 0xff, - static_cast(endpoint >> 8) & 0xff, - static_cast(endpoint >> 16) & 0xff, - static_cast(endpoint >> 24) & 0xff, + "{}:{}", + endpoint, static_cast(ntohs(remotePortNumber))); - EndpointsTable::instance().add_endpoint( + EndpointsTable::instance().add_endpoint( IPProtocolNumber::TCP, endpoint, localPortNumber, @@ -924,14 +913,8 @@ namespace if ((ethernetHeader->destination != mac_address()) && (ethernetHeader->destination != broadcastMAC)) { - Debug::log( - "Dropping frame with destination MAC address {}:{}:{}:{}:{}:{}", - ethernetHeader->destination[0], - ethernetHeader->destination[1], - ethernetHeader->destination[2], - ethernetHeader->destination[3], - ethernetHeader->destination[4], - ethernetHeader->destination[5]); + Debug::log("Dropping frame with destination MAC address {}", + ethernetHeader->destination); return ForwardFlags::Discard; } switch (ethernetHeader->etherType) @@ -1031,7 +1014,7 @@ bool ethernet_link_is_up() return ethernet.phy_link_status(); } -void firewall_dns_server_ip_set(uint32_t ip) +void firewall_dns_server_ip_set(IPv4Address 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 @@ -1051,27 +1034,27 @@ void firewall_permit_dns(bool dnsIsPermitted) void firewall_add_tcpipv4_server_port(uint16_t localPort) { - EndpointsTable::instance().add_server_port(localPort); + EndpointsTable::instance().add_server_port(localPort); } void firewall_remove_tcpipv4_server_port(uint16_t localPort) { - EndpointsTable::instance().remove_server_port(localPort); + EndpointsTable::instance().remove_server_port(localPort); } -void firewall_add_tcpipv4_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort) +void firewall_add_tcpipv4_endpoint(IPv4Address remoteAddress, + uint16_t localPort, + uint16_t remotePort) { - EndpointsTable::instance().add_endpoint( + EndpointsTable::instance().add_endpoint( IPProtocolNumber::TCP, remoteAddress, localPort, remotePort); } -void firewall_add_udpipv4_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort) +void firewall_add_udpipv4_endpoint(IPv4Address remoteAddress, + uint16_t localPort, + uint16_t remotePort) { - EndpointsTable::instance().add_endpoint( + EndpointsTable::instance().add_endpoint( IPProtocolNumber::UDP, remoteAddress, localPort, remotePort); } @@ -1080,19 +1063,19 @@ void firewall_remove_tcpipv4_local_endpoint(uint16_t localPort) // Server ports are likely to be associated to more than one entry in // the firewall. Debug::Assert( - !EndpointsTable::instance().is_server_port(localPort), + !EndpointsTable::instance().is_server_port(localPort), "Trying to remove a local endpoint on a server port."); - EndpointsTable::instance().remove_endpoint(IPProtocolNumber::TCP, - localPort); + EndpointsTable::instance().remove_endpoint( + IPProtocolNumber::TCP, localPort); } -void firewall_remove_tcpipv4_remote_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort) +void firewall_remove_tcpipv4_remote_endpoint(IPv4Address remoteAddress, + uint16_t localPort, + uint16_t remotePort) { - if (EndpointsTable::instance().remove_endpoint( + if (EndpointsTable::instance().remove_endpoint( IPProtocolNumber::TCP, remoteAddress, localPort, remotePort) && - EndpointsTable::instance().is_server_port(localPort)) + EndpointsTable::instance().is_server_port(localPort)) { // Decrease the number of clients only if we actually removed // an entry from the endpoints table. @@ -1102,15 +1085,15 @@ void firewall_remove_tcpipv4_remote_endpoint(uint32_t remoteAddress, void firewall_remove_udpipv4_local_endpoint(uint16_t localPort) { - EndpointsTable::instance().remove_endpoint(IPProtocolNumber::UDP, - localPort); + EndpointsTable::instance().remove_endpoint( + IPProtocolNumber::UDP, localPort); } -void firewall_remove_udpipv4_remote_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort) +void firewall_remove_udpipv4_remote_endpoint(IPv4Address remoteAddress, + uint16_t localPort, + uint16_t remotePort) { - EndpointsTable::instance().remove_endpoint( + EndpointsTable::instance().remove_endpoint( IPProtocolNumber::UDP, remoteAddress, localPort, remotePort); } @@ -1231,8 +1214,8 @@ bool ethernet_driver_start(std::atomic *state) EndpointsTable::instance().clear(IPProtocolNumber::UDP); EndpointsTable::instance().clear(IPProtocolNumber::TCP); #endif - EndpointsTable::instance().clear(IPProtocolNumber::UDP); - EndpointsTable::instance().clear(IPProtocolNumber::TCP); + EndpointsTable::instance().clear(IPProtocolNumber::UDP); + EndpointsTable::instance().clear(IPProtocolNumber::TCP); return true; } // Protect against double entry. If the barrier state is 0, no diff --git a/lib/firewall/firewall.hh b/lib/firewall/firewall.hh index 65b0108..ad18206 100644 --- a/lib/firewall/firewall.hh +++ b/lib/firewall/firewall.hh @@ -5,6 +5,8 @@ #include #include +#include "protocol-addresses.hh" + /** * Unless specified otherwise, all APIs exposed in this header take IP * addresses and ports in network byte order. @@ -89,7 +91,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); +void __cheri_compartment("Firewall") firewall_dns_server_ip_set(IPv4Address ip); /** * Toggle whether DNS is permitted. This is used to open a hole in the @@ -108,9 +110,9 @@ void __cheri_compartment("Firewall") * This should be called only by the NetAPI compartment. */ void __cheri_compartment("Firewall") - firewall_add_tcpipv4_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort); + firewall_add_tcpipv4_endpoint(IPv4Address remoteAddress, + uint16_t localPort, + uint16_t remotePort); /** * Open a hole in the firewall for UDP packets to and from the given endpoint. @@ -120,9 +122,9 @@ void __cheri_compartment("Firewall") * This should be called only by the NetAPI compartment. */ void __cheri_compartment("Firewall") - firewall_add_udpipv4_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort); + firewall_add_udpipv4_endpoint(IPv4Address remoteAddress, + uint16_t localPort, + uint16_t remotePort); /** * Close a hole in the firewall for TCP packets to and from the given endpoint. @@ -145,9 +147,9 @@ void __cheri_compartment("Firewall") * (see discussion in `firewall_remove_tcpipv4_local_endpoint`). */ void __cheri_compartment("Firewall") - firewall_remove_tcpipv4_remote_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort); + firewall_remove_tcpipv4_remote_endpoint(IPv4Address remoteAddress, + uint16_t localPort, + uint16_t remotePort); /** * Close a hole in the firewall for UDP packets to and from the given endpoint. @@ -167,9 +169,9 @@ void __cheri_compartment("Firewall") * (see discussion in `firewall_remove_udpipv4_local_endpoint`). */ void __cheri_compartment("Firewall") - firewall_remove_udpipv4_remote_endpoint(uint32_t remoteAddress, - uint16_t localPort, - uint16_t remotePort); + firewall_remove_udpipv4_remote_endpoint(IPv4Address remoteAddress, + uint16_t localPort, + uint16_t remotePort); /** * Register a local TCP port as server port into the firewall. diff --git a/lib/firewall/protocol-addresses-debug.hh b/lib/firewall/protocol-addresses-debug.hh new file mode 100644 index 0000000..a2ac8c1 --- /dev/null +++ b/lib/firewall/protocol-addresses-debug.hh @@ -0,0 +1,57 @@ +#pragma once + +#include "protocol-headers.hh" +#include + +/** + * Pretty-print a MACAddress. + * + * This relies on the address being valid across the duration of the call. + */ +template<> +struct DebugFormatArgumentAdaptor +{ + __always_inline static DebugFormatArgument construct(MACAddress &address) + { + return {reinterpret_cast(&address), + reinterpret_cast(&print)}; + } + + private: + static void print(uintptr_t value, DebugWriter &writer) + { + auto *address = reinterpret_cast(value); + writer.write_hex_byte((*address)[0]); + for (size_t ix = 1; ix < sizeof(*address); ix++) + { + writer.write(':'); + writer.write_hex_byte((*address)[ix]); + } + } +}; + +/** + * Pretty-print an IPv4Address. + */ +template<> +struct DebugFormatArgumentAdaptor +{ + __always_inline static DebugFormatArgument construct(IPv4Address &address) + { + return {static_cast(address.raw), + reinterpret_cast(&print)}; + } + + private: + static void print(uintptr_t value, DebugWriter &writer) + { + auto address = static_cast(value); + writer.write(static_cast((address >> 0) & 0xFF)); + writer.write('.'); + writer.write(static_cast((address >> 8) & 0xFF)); + writer.write('.'); + writer.write(static_cast((address >> 16) & 0xFF)); + writer.write('.'); + writer.write(static_cast((address >> 24) & 0xFF)); + } +}; diff --git a/lib/firewall/protocol-addresses.hh b/lib/firewall/protocol-addresses.hh new file mode 100644 index 0000000..7a8675e --- /dev/null +++ b/lib/firewall/protocol-addresses.hh @@ -0,0 +1,112 @@ +// Copyright SCI Semiconductor and CHERIoT Contributors. +// SPDX-License-Identifier: MIT + +#pragma once + +#include +#include + +/** + * Ethernet MAC address. + */ +struct MACAddress +{ + using Raw = std::array; + + Raw raw; + + constexpr MACAddress() = default; + + constexpr MACAddress(Raw &&r) : raw(r) {} + + /// Pass-through aggregate initialization + template + constexpr MACAddress(const T &&...r) : raw({static_cast(r)...}) + { + } + + template + auto data(this Self &&self) + { + return self.raw.data(); + } + + template + auto begin(this Self &&self) + { + return self.raw.begin(); + } + + template + auto end(this Self &&self) + { + return self.raw.end(); + } + + template + auto &operator[](this Self &&self, size_t ix) + { + return self.raw[ix]; + } + + template + constexpr bool operator==(this Self &&self, MACAddress &other) + { + return self.raw == other.raw; + } + + template + constexpr operator Raw &(this Self &&self) + { + return self.raw; + } +}; +static_assert(sizeof(MACAddress) == sizeof(MACAddress::Raw)); + +struct IPv4Address +{ + uint32_t raw; + + constexpr IPv4Address() = default; + + constexpr IPv4Address(uint32_t r) : raw(r) {}; + + constexpr IPv4Address(const IPv4Address &) = default; + + operator uint32_t() const + { + return raw; + } +}; + +/** + * IPv6 address. + * + * This should be `std::array` but our version of `std::array` + * does not yet have a three-way comparison operator. + */ +struct IPv6Address +{ + /** + * The bytes of the address. + */ + uint8_t bytes[16]; + /** + * Returns a pointer to the bytes of this address. + */ + auto data() + { + return bytes; + } + /** + * Returns the size of an address. + */ + [[nodiscard]] size_t size() const + { + return sizeof(bytes); + } + /// Comparison operator. + // A clang-tidy bug thinks that this should be = nullptr instead of = + // default. + auto operator<=>(const IPv6Address &) const = default; // NOLINT +}; diff --git a/lib/firewall/protocol-headers.hh b/lib/firewall/protocol-headers.hh index 173e5ba..175c71e 100644 --- a/lib/firewall/protocol-headers.hh +++ b/lib/firewall/protocol-headers.hh @@ -3,7 +3,7 @@ #pragma once -#include +#include "protocol-addresses.hh" /** * EtherType values, for Ethernet headers. These are defined in network @@ -35,11 +35,6 @@ const char *ethertype_as_string(EtherType etherType) } } -/** - * Ethernet MAC address. - */ -using MACAddress = std::array; - /** * Ethernet header. */ @@ -66,38 +61,6 @@ enum IPProtocolNumber : uint8_t UDP = 17, }; -/** - * IPv6 address. - * - * This should be `std::array` but our version of `std::array` - * does not yet have a three-way comparison operator. - */ -struct IPv6Address -{ - /** - * The bytes of the address. - */ - uint8_t bytes[16]; - /** - * Returns a pointer to the bytes of this address. - */ - auto data() - { - return bytes; - } - /** - * Returns the size of an address. - */ - [[nodiscard]] size_t size() const - { - return sizeof(bytes); - } - /// Comparison operator. - // A clang-tidy bug thinks that this should be = nullptr instead of = - // default. - auto operator<=>(const IPv6Address &) const = default; // NOLINT -}; - struct IPv4Header { /** @@ -136,11 +99,11 @@ struct IPv4Header /** * Source IP address. */ - uint32_t sourceAddress; + IPv4Address sourceAddress; /** * Destination IP address. */ - uint32_t destinationAddress; + IPv4Address destinationAddress; /** * Returns the offset of the start of the body of this packet. diff --git a/lib/tcpip/startup.cc b/lib/tcpip/startup.cc index df99b7d..895cede 100644 --- a/lib/tcpip/startup.cc +++ b/lib/tcpip/startup.cc @@ -156,11 +156,11 @@ void __cheri_compartment("TCPIP") network_start() Debug::log("Kicking IP stack"); if (FreeRTOS_IPInit_Multi() == pdPASS) { - Debug::log("Successfully initialized IP stack\n"); + Debug::log("Successfully initialized IP stack"); } else { - Debug::log("Failed to initialize IP stack\n"); + Debug::log("Failed to initialize IP stack"); } if (restartState.load() == 0) {