diff --git a/include/NetAPI.h b/include/NetAPI.h index a6b081b..3c5bc67 100644 --- a/include/NetAPI.h +++ b/include/NetAPI.h @@ -219,8 +219,8 @@ int __cheri_compartment("TCPIP") * freeing this buffer. * * The `address` and `port` arguments are used to return the address and port - * of the sender. These can be null if the caller is not interested in the - * sender's address or port. + * of the sender, in host byte order. These can be null if the caller is not + * interested in the sender's address or port. * * The `bytesReceived` field of the result will be negative if an error * occurred. The `buffer` field will be an untagged value if no data were @@ -255,9 +255,9 @@ ssize_t __cheri_compartment("TCPIP") network_socket_send(Timeout *timeout, /** * Send data over a UDP socket to a specified host / port. The address and - * port must have been previously authorised with - * `network_socket_udp_authorise_host` (the packets will be silently dropped if - * not, there will be no error reported). + * port must be provided in host byte order, and must have been previously + * authorised with `network_socket_udp_authorise_host` (the packets will be + * silently dropped if not, there will be no error reported). * * This will block until the data have been sent or the timeout expires. The * return value is the number of bytes sent or a negative error code. @@ -370,7 +370,7 @@ struct BindCapability DECLARE_AND_DEFINE_STATIC_SEALED_VALUE( \ struct { \ ConnectionType type; \ - short port; \ + uint16_t port; \ size_t nameLength; \ const char hostname[sizeof(authorisedHost)]; \ }, \ diff --git a/include/endianness.hh b/include/endianness.hh new file mode 100644 index 0000000..28ba4e7 --- /dev/null +++ b/include/endianness.hh @@ -0,0 +1,26 @@ +// Copyright SCI Semiconductor and CHERIoT Contributors. +// SPDX-License-Identifier: MIT + +#include + +uint16_t constexpr ntohs(uint16_t value) +{ + return +#ifdef __LITTLE_ENDIAN__ + __builtin_bswap16(value) +#else + value +#endif + ; +} + +uint16_t constexpr htons(uint16_t value) +{ + return +#ifdef __LITTLE_ENDIAN__ + __builtin_bswap16(value) +#else + value +#endif + ; +} diff --git a/include/mqtt.h b/include/mqtt.h index b60f885..496da68 100644 --- a/include/mqtt.h +++ b/include/mqtt.h @@ -11,7 +11,9 @@ * will be called on all PUBLISH notifications from the broker. * * `topicName` and `payload` (and their respective size arguments) indicate the - * topic of the PUBLISH, and the corresponding payload. + * topic of the PUBLISH, and the corresponding payload. Both are only valid + * within the context of the callback and thus passed as a read-only, + * non-capturable capabilities. */ typedef void __cheri_callback (*MQTTPublishCallback)(const char *topicName, size_t topicNameLength, diff --git a/include/tls.h b/include/tls.h index 9fc70d1..f385544 100644 --- a/include/tls.h +++ b/include/tls.h @@ -6,12 +6,13 @@ #include /** - * Creates a new TLS connection. Returns null on failure or a sealed TLS - * connection object on success. + * Creates a new TLS connection. This will block until the connection is + * established, an error happens, or the timeout expires. Returns an untagged + * value on failure or a sealed TLS connection object on success. * * The state for the TLS connection will be allocated with `allocator`. The - * connection will be made to the host identified by the connection capability. - * This must authorise a TCP connection. Once the connection is made, the + * connection will be made to the host identified by the connection capability, + * which must authorise a TCP connection. Once the connection is made, the * certificates will be validated against the trust anchors provided via the * `trustAnchors` parameter, which contains a pointer to an array of * `trustAnchorsCount` trust anchors. @@ -57,13 +58,16 @@ enum TLSSendFlags }; /** - * Sends `length` bytes from `buffer` to the remote host. Returns the number - * of bytes sent, or a negative error code. The `sealedConnection` parameter - * is a pointer to a TLS connection, returned by `tls_connection_create`. + * Sends `length` bytes from `buffer` to the remote host. Returns the + * number of bytes sent, or a negative error code. + * + * The `sealedConnection` parameter is a pointer to a TLS connection, returned + * by `tls_connection_create`. * * If `flags` is set to `TLSSendNoFlush`, the TLS engine may buffer the data * and not send until a later send call. If this is not provided then the - * timeout may be exceeded. + * timeout may be exceeded. In the general case, this will block until the data + * is sent, an error happens, or the timeout expires. */ ssize_t __cheri_compartment("TLS") tls_connection_send(Timeout *t, SObj sealedConnection, @@ -72,10 +76,12 @@ ssize_t __cheri_compartment("TLS") tls_connection_send(Timeout *t, int flags); /** - * Receive data from the TLS connection. This returns a newly allocated buffer - * (allocated with the allocator provided to `tls_connection_create`) - * containing the received data along with its length, or null and a negative - * error code. The caller is responsible for freeing this buffer. + * Receive data from the TLS connection. This will block until data are + * received, an error happens, or the timeout expires. If data are received, + * they will be stored in a newly allocated buffer (allocated with the + * allocator provided to `tls_connection_create`) and returned along with their + * length. The caller is responsible for freeing this buffer. On error, the + * return value is an untagged value and a negative error code. * * The negative values will be errno values: * @@ -88,8 +94,8 @@ NetworkReceiveResult __cheri_compartment("TLS") /** * Receive data from the TLS connection into a preallocated buffer. This will - * block until data are received or the timeout expires. If data are received, - * they will be stored in the provided buffer. + * block until data are received, an error happens, or the timeout expires. If + * data are received, they will be stored in the provided buffer. * * The return value is either the number of bytes received, zero if the * connection is closed, or a negative error code. diff --git a/lib/firewall/firewall.cc b/lib/firewall/firewall.cc index a111739..697a2c4 100644 --- a/lib/firewall/firewall.cc +++ b/lib/firewall/firewall.cc @@ -5,6 +5,7 @@ #include #include //#include +#include #include #include #include @@ -16,31 +17,6 @@ using Debug = ConditionalDebug; #include "firewall.hh" -namespace -{ - // TODO These should probably be in their own library. - uint16_t constexpr ntohs(uint16_t value) - { - return -#ifdef __LITTLE_ENDIAN__ - __builtin_bswap16(value) -#else - value -#endif - ; - } - uint16_t constexpr htons(uint16_t value) - { - return -#ifdef __LITTLE_ENDIAN__ - __builtin_bswap16(value) -#else - value -#endif - ; - } -} // namespace - namespace { diff --git a/lib/firewall/firewall.hh b/lib/firewall/firewall.hh index 870fe1c..598b766 100644 --- a/lib/firewall/firewall.hh +++ b/lib/firewall/firewall.hh @@ -5,6 +5,11 @@ #include #include +/** + * Unless specified otherwise, all APIs exposed in this header take IP + * addresses and ports in network byte order. + */ + /** * Send a frame through the on-device firewall. This returns true if the * packet is successfully sent, false otherwise. diff --git a/lib/mqtt/mqtt.cc b/lib/mqtt/mqtt.cc index 5738f92..6392680 100644 --- a/lib/mqtt/mqtt.cc +++ b/lib/mqtt/mqtt.cc @@ -468,9 +468,17 @@ namespace "The packet is of type PUBLISH, but topic or payload " "are not set."); - publishCallback(publishInfo->pTopicName, + // The payload and topic are only valid within the + // context of the callback: make them read-only and + // non-capturable. + Capability topic{publishInfo->pTopicName}; + Capability payload{publishInfo->pPayload}; + topic.permissions() &= CHERI::Permission::Load; + payload.permissions() &= CHERI::Permission::Load; + + publishCallback(topic, publishInfo->topicNameLength, - publishInfo->pPayload, + payload, publishInfo->payloadLength); } else if (ackCallback) diff --git a/lib/netapi/NetAPI.cc b/lib/netapi/NetAPI.cc index d45feba..47a90bc 100644 --- a/lib/netapi/NetAPI.cc +++ b/lib/netapi/NetAPI.cc @@ -6,6 +6,7 @@ #include #include +#include #include using Debug = ConditionalDebug; @@ -14,19 +15,6 @@ using Debug = ConditionalDebug; namespace { - uint16_t ntohs(uint16_t value) - { - return __builtin_bswap16(value); - } - uint16_t htons(uint16_t value) - { - return __builtin_bswap16(value); - } -} // namespace - -namespace -{ - using Debug = ConditionalDebug; constexpr bool UseIPv6 = CHERIOT_RTOS_OPTION_IPv6; /** diff --git a/lib/tcpip/network-internal.h b/lib/tcpip/network-internal.h index 6291cee..f18862e 100644 --- a/lib/tcpip/network-internal.h +++ b/lib/tcpip/network-internal.h @@ -46,7 +46,7 @@ __cheri_compartment("TCPIP") int network_host_resolve( * * This returns a sealed capability to a socket on success, or null on failure. * - * This should be called only from the NetAPI compartment. + * This should be called only from the NetAPI or TCP/IP compartments. */ SObj __cheri_compartment("TCPIP") network_socket_create_and_bind(Timeout *timeout, diff --git a/lib/tcpip/network_wrapper.cc b/lib/tcpip/network_wrapper.cc index 3e06ba8..7145c6c 100644 --- a/lib/tcpip/network_wrapper.cc +++ b/lib/tcpip/network_wrapper.cc @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -73,28 +74,6 @@ using CHERI::PermissionSet; namespace { - // TODO These should probably be in their own library. - uint16_t constexpr ntohs(uint16_t value) - { - return -#ifdef __LITTLE_ENDIAN__ - __builtin_bswap16(value) -#else - value -#endif - ; - } - uint16_t constexpr htons(uint16_t value) - { - return -#ifdef __LITTLE_ENDIAN__ - __builtin_bswap16(value) -#else - value -#endif - ; - } - /** * Returns the key with which SealedSocket instances are sealed. */ @@ -631,7 +610,7 @@ SObj network_socket_create_and_bind(Timeout *timeout, // or passing pxAddress as NULL will result in the socket being // bound to a port number from the private range'. Here, // `localPort` will be 0 if the caller wants to bind to any port. - localAddress.sin_port = FreeRTOS_htons(localPort); + localAddress.sin_port = htons(localPort); localAddress.sin_family = Family; auto bindResult = FreeRTOS_bind(socket, &localAddress, sizeof(localAddress)); @@ -831,7 +810,7 @@ int network_socket_connect_tcp_internal(Timeout *timeout, struct freertos_sockaddr server; memset(&server, 0, sizeof(server)); server.sin_len = sizeof(server); - server.sin_port = FreeRTOS_htons(port); + server.sin_port = htons(port); if (isIPv6) { server.sin_family = FREERTOS_AF_INET6; @@ -923,7 +902,7 @@ int network_socket_close(Timeout *t, SObj mallocCapability, SObj sealedSocket) // happen in practice and has no impact here. FreeRTOS_shutdown(rawSocket, FREERTOS_SHUT_RDWR); - auto localPort = ntohs(rawSocket->usLocalPort); + auto localPort = htons(rawSocket->usLocalPort); if (rawSocket->bits.bIsIPv6) { if (isTCP) @@ -1303,7 +1282,7 @@ ssize_t network_socket_send_to(Timeout *timeout, struct freertos_sockaddr server; memset(&server, 0, sizeof(server)); server.sin_len = sizeof(server); - server.sin_port = FreeRTOS_htons(port); + server.sin_port = htons(port); if (heap_claim_fast(timeout, address) < 0) { Debug::log("Failed to claim address"); diff --git a/lib/tcpip/tcpip_error_handler.h b/lib/tcpip/tcpip_error_handler.h index ae37ef5..75b6b18 100644 --- a/lib/tcpip/tcpip_error_handler.h +++ b/lib/tcpip/tcpip_error_handler.h @@ -232,8 +232,8 @@ extern "C" void reset_network_stack_state() // Upgrade the message queue lock for destruction DebugErrorHandler::log("Upgrading the message queue for destruction."); - auto *queueHandle = &xNetworkEventQueue->handle; - if (int err = queue_destroy(MALLOC_CAPABILITY, queueHandle); err != 0) + if (int err = queue_destroy(MALLOC_CAPABILITY, xNetworkEventQueue); + err != 0) { DebugErrorHandler::log( "Failed to upgrade the message queue for destruction (error {}).", diff --git a/network_stack.rego b/network_stack.rego index 7d8f033..bf52968 100644 --- a/network_stack.rego +++ b/network_stack.rego @@ -164,7 +164,7 @@ valid(ethernetDevice) { firewall_thread_is_valid network_thread_is_valid data.compartment.compartment_call_allow_list("TCPIP", "network_host_resolve.*", {"NetAPI"}) - data.compartment.compartment_call_allow_list("TCPIP", "network_socket_create_and_bind.*", {"NetAPI"}) + data.compartment.compartment_call_allow_list("TCPIP", "network_socket_create_and_bind.*", {"NetAPI", "TCPIP"}) data.compartment.compartment_call_allow_list("TCPIP", "network_socket_connect_tcp_internal.*", {"NetAPI"}) data.compartment.compartment_call_allow_list("TCPIP", "ethernet_receive_frame.*", {"Firewall"}) data.compartment.compartment_call_allow_list("TCPIP", "ip_thread_entry.*", set())