diff --git a/.changes/captive-portal-host-routing.md b/.changes/captive-portal-host-routing.md new file mode 100644 index 00000000..a4796b75 --- /dev/null +++ b/.changes/captive-portal-host-routing.md @@ -0,0 +1,16 @@ +--- +kind: fixed +--- +Route captive portal requests by Host header, and serve the portal only on its own network + +## Release Note +More reliable captive portal detection + +Phones and computers joining the hub's setup network now reliably open the sign-in screen, whatever address they check. The setup portal and its DNS server are now only reachable from the setup network, not from your home network. + +- Any request to another website over the setup network is sent to the portal, not just a fixed list of check addresses +- Unknown pages on the hub show "Not Found" instead of jumping back to the start page +- The captive portal API also answers at /captiveportal/api + +## Notices +- note: **The setup portal no longer answers on your home network.** Opening the hub's home network address in a browser now fails. Join the hub's setup network to reach the portal. diff --git a/components/captive_portal/include/captiveportal/Manager.h b/components/captive_portal/include/captiveportal/Manager.h index 0e75b19f..93a78725 100644 --- a/components/captive_portal/include/captiveportal/Manager.h +++ b/components/captive_portal/include/captiveportal/Manager.h @@ -22,6 +22,10 @@ namespace OpenShock::CaptivePortal { /// @brief The fixed IPv4 address the captive portal AP serves from (e.g. "4.3.2.1"). const char* ApIPv4String(); + /// @brief The lwIP name of the captive portal AP's network interface (e.g. "ap1"), for binding sockets to it. + /// Empty until the portal has started once. + const char* ApInterfaceName(); + bool SendMessageTXT(uint8_t socketId, std::string_view data); bool SendMessageBIN(uint8_t socketId, std::span data); diff --git a/components/captive_portal/src/captiveportal/ApiHandlers.cpp b/components/captive_portal/src/captiveportal/ApiHandlers.cpp index 54888084..2b2aca29 100644 --- a/components/captive_portal/src/captiveportal/ApiHandlers.cpp +++ b/components/captive_portal/src/captiveportal/ApiHandlers.cpp @@ -387,19 +387,19 @@ static esp_err_t apiOtaCheck(httpd_req_t* req) void CaptivePortal::RegisterApiHandlers(httpd_handle_t server, CaptivePortalInstance* instance) { - registerHandler(server, "/api/board", HTTP_GET, apiBoard, instance); - registerHandler(server, "/api/portal/close", HTTP_POST, apiPortalClose, instance); - registerHandler(server, "/api/wifi/scan", HTTP_POST, apiWifiScan, instance); - registerHandler(server, "/api/wifi/networks", HTTP_DELETE, apiWifiNetworksDelete, instance); - registerHandler(server, "/api/wifi/networks", HTTP_POST, apiWifiNetworksAdd, instance); - registerHandler(server, "/api/wifi/connect", HTTP_POST, apiWifiConnect, instance); - registerHandler(server, "/api/wifi/disconnect", HTTP_POST, apiWifiDisconnect, instance); - registerHandler(server, "/api/account/link", HTTP_POST, apiAccountLink, instance); - registerHandler(server, "/api/account", HTTP_DELETE, apiAccountDelete, instance); - registerHandler(server, "/api/config/rf/pin", HTTP_PUT, apiConfigRfPin, instance); + registerHandler(server, "/api/board", HTTP_GET, instance); + registerHandler(server, "/api/portal/close", HTTP_POST, instance); + registerHandler(server, "/api/wifi/scan", HTTP_POST, instance); + registerHandler(server, "/api/wifi/networks", HTTP_DELETE, instance); + registerHandler(server, "/api/wifi/networks", HTTP_POST, instance); + registerHandler(server, "/api/wifi/connect", HTTP_POST, instance); + registerHandler(server, "/api/wifi/disconnect", HTTP_POST, instance); + registerHandler(server, "/api/account/link", HTTP_POST, instance); + registerHandler(server, "/api/account", HTTP_DELETE, instance); + registerHandler(server, "/api/config/rf/pin", HTTP_PUT, instance); // No /api/config/estop/*: the portal is an open, unauthenticated AP, so the E-Stop is configured over serial only. // No /api/ota/domain: the portal is an open, unauthenticated AP, and OTA trusts the hashes served by that domain, so // changing it would let anyone in range install firmware. It can only be changed over serial (jsonconfig). - registerHandler(server, "/api/ota/settings", HTTP_PUT, apiOtaSettings, instance); - registerHandler(server, "/api/ota/check", HTTP_POST, apiOtaCheck, instance); + registerHandler(server, "/api/ota/settings", HTTP_PUT, instance); + registerHandler(server, "/api/ota/check", HTTP_POST, instance); } diff --git a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index 59a888b7..7a58b7c8 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -11,6 +11,9 @@ const char* const TAG = "CaptivePortalInstance"; #include +#include +#include + const uint16_t HTTP_PORT = 80; // httpd needs 3 lwIP sockets besides its client sockets; the DNS server, the gateway connection and an outbound HTTP request need one each static_assert(CONFIG_LWIP_MAX_SOCKETS >= CONFIG_OPENSHOCK_PORTAL_HTTPD_MAX_SOCKETS + 3 + 3, "httpd client sockets would leave no lwIP sockets for the DNS server and outbound connections"); @@ -23,8 +26,8 @@ void CaptivePortal::CaptivePortalInstance::registerHandlers() // API endpoints (registered before the static wildcard so they take priority). RegisterApiHandlers(m_server, this); - // OS captive-detection probes + RFC 8908 endpoint + 404 redirect (rfc8908 component). - // Registered before the static wildcard so the specific probe paths take priority. + // RFC 8908 endpoint (rfc8908 component); probes fall through to the static handler's redirect. + // Registered before the static wildcard so the API paths take priority. RFC8908::RegisterProbeHandlers(m_server, CaptivePortal::ApIPv4String()); m_webSocket.registerHandler(m_server); @@ -46,6 +49,18 @@ static esp_err_t onSessionOpen(httpd_handle_t /*server*/, int sockfd) bool CaptivePortal::CaptivePortalInstance::startHttpServer() { + // Bound to the AP interface (SO_BINDTODEVICE), so lwIP only hands it connections that arrive on the AP. Checking + // the local address is not enough: lwIP accepts a packet for any of the hub's addresses on any interface, so a LAN + // client that routes the AP address via the hub's STA address would get through. The portal's API is the local + // protocol, which has no compatibility rules: nothing on the LAN may come to depend on it. + ifreq apInterface = {}; // zeroed, so the copied name is NUL-terminated + std::string_view apIfName = CaptivePortal::ApInterfaceName(); + if (apIfName.empty() || apIfName.size() >= sizeof(apInterface.ifr_name)) { + OS_LOGE(TAG, "No usable AP interface name (\"%.*s\"), not starting the HTTP server", static_cast(apIfName.size()), apIfName.data()); + return false; + } + apIfName.copy(apInterface.ifr_name, apIfName.size()); + httpd_config_t config = HTTPD_DEFAULT_CONFIG(); config.server_port = HTTP_PORT; config.max_uri_handlers = 40; @@ -57,6 +72,7 @@ bool CaptivePortal::CaptivePortalInstance::startHttpServer() config.send_wait_timeout = CONFIG_OPENSHOCK_PORTAL_HTTPD_TIMEOUT_S; config.uri_match_fn = httpd_uri_match_wildcard; config.open_fn = onSessionOpen; + config.if_name = &apInterface; // only read during httpd_start // Same core as the WiFi/lwIP tasks, away from the E-Stop and RF tasks on core 1 config.core_id = 0; @@ -78,8 +94,9 @@ CaptivePortal::CaptivePortalInstance::CaptivePortalInstance() return; } - // Start the wildcard DNS responder (all A queries -> the portal AP IP). - m_dnsStarted = m_dnsServer.start(CaptivePortal::ApIPv4String()); + // Start the wildcard DNS responder (all A queries -> the portal AP IP), on the AP interface only: the LAN must not + // get its answers. + m_dnsStarted = m_dnsServer.start(CaptivePortal::ApIPv4String(), CaptivePortal::ApIPv4String(), CaptivePortal::ApInterfaceName()); if (!m_dnsStarted) { OS_LOGE(TAG, "Failed to start DNS server"); } diff --git a/components/captive_portal/src/captiveportal/HttpUtils.cpp b/components/captive_portal/src/captiveportal/HttpUtils.cpp index d5db29b3..0dc33ba5 100644 --- a/components/captive_portal/src/captiveportal/HttpUtils.cpp +++ b/components/captive_portal/src/captiveportal/HttpUtils.cpp @@ -21,7 +21,7 @@ esp_err_t CaptivePortal::sendResp(httpd_req_t* req, const char* status, const ch return httpd_resp_send(req, body.data(), body.size()); } -void CaptivePortal::registerHandler(httpd_handle_t server, const char* uri, httpd_method_t method, esp_err_t (*handler)(httpd_req_t*), void* userCtx) +void CaptivePortal::detail::registerHandler(httpd_handle_t server, const char* uri, httpd_method_t method, esp_err_t (*handler)(httpd_req_t*), void* userCtx) { httpd_uri_t def = {}; def.uri = uri; diff --git a/components/captive_portal/src/captiveportal/HttpUtils.h b/components/captive_portal/src/captiveportal/HttpUtils.h index c2a252ac..9cd9e3aa 100644 --- a/components/captive_portal/src/captiveportal/HttpUtils.h +++ b/components/captive_portal/src/captiveportal/HttpUtils.h @@ -1,5 +1,7 @@ #pragma once +#include "rfc8908/RFC8908Handler.h" + #include #include @@ -26,8 +28,29 @@ namespace OpenShock::CaptivePortal { esp_err_t sendResp(httpd_req_t* req, const char* status, const char* type, std::string_view body); - /// @brief Registers a route, logging a failure. With wildcard matching, routes registered earlier take priority. - void registerHandler(httpd_handle_t server, const char* uri, httpd_method_t method, esp_err_t (*handler)(httpd_req_t*), void* userCtx); + namespace detail { + void registerHandler(httpd_handle_t server, const char* uri, httpd_method_t method, esp_err_t (*handler)(httpd_req_t*), void* userCtx); + + // httpd has no hook that runs before every request, so each route is wrapped in this. A request addressed to + // another host (a connectivity probe, a page on a hijacked or rebound name) never reaches a portal handler; it + // gets the captive redirect instead. + template + esp_err_t ownHostOnly(httpd_req_t* req) + { + if (RFC8908::IsForeignHost(req)) { + return RFC8908::EmitRedirect(req); + } + return Handler(req); + } + } // namespace detail + + /// @brief Registers a route that only answers requests addressed to us (see detail::ownHostOnly), logging a failure. + /// With wildcard matching, routes registered earlier take priority. + template + void registerHandler(httpd_handle_t server, const char* uri, httpd_method_t method, void* userCtx) + { + detail::registerHandler(server, uri, method, &detail::ownHostOnly, userCtx); + } // Read and URL-decode a query-string parameter (key matched case-insensitively, as httpd does). The query is // parsed in place in the request URI: no copy, and no limit on the value's length. diff --git a/components/captive_portal/src/captiveportal/Manager.cpp b/components/captive_portal/src/captiveportal/Manager.cpp index a9da284a..f499ca98 100644 --- a/components/captive_portal/src/captiveportal/Manager.cpp +++ b/components/captive_portal/src/captiveportal/Manager.cpp @@ -17,6 +17,7 @@ const char* const TAG = "CaptivePortal"; #include "Temporal.h" #include "wifi/WiFiConfig.h" +#include #include #include #include @@ -74,6 +75,12 @@ static std::atomic s_instanceLive = false; // Instances taken down whose destruction waits for other tasks to drop their references; destroyed by the manager task. static RetiredList s_retiredInstances; static esp_netif_t* s_apNetif = nullptr; +// Given once WIFI_EVENT_AP_START has added s_apNetif to lwIP, so its interface name can be read. +static SemaphoreHandle_t s_apStarted = nullptr; +// lwIP's name for s_apNetif, which the portal's servers bind to. Re-read on every start: lwIP only knows the netif +// while the AP runs, and gives it a new number each time the AP starts again. +static char s_apIfName[8] = {}; +static constexpr TickType_t kApStartTimeout = pdMS_TO_TICKS(2000); static_assert(IPv4Utils::IsSlash24Host(CONFIG_OPENSHOCK_PORTAL_AP_IP), "CONFIG_OPENSHOCK_PORTAL_AP_IP must be a dotted-decimal IPv4 address with a host octet of 1-254"); @@ -139,6 +146,13 @@ static bool DestroyInstance() return false; } +// Registered after esp_wifi's default WIFI_EVENT_AP_START handler, so it runs once that one has added the AP netif +// to lwIP: only from then on does lwIP hand out the netif's real interface name. +static void captiveportal_onapstart(void*, esp_event_base_t, int32_t, void*) +{ + xSemaphoreGive(s_apStarted); +} + static bool captiveportal_start() { if (GetInstance() != nullptr) { @@ -178,6 +192,17 @@ static bool captiveportal_start() esp_netif_dhcps_start(s_apNetif); } + // esp_netif_create_default_wifi_ap() registered esp_wifi's default handlers, so this one runs after them. + static bool apStartHandlerRegistered = false; + if (!apStartHandlerRegistered) { + esp_err_t evtErr = esp_event_handler_register(WIFI_EVENT, WIFI_EVENT_AP_START, captiveportal_onapstart, nullptr); + if (evtErr != ESP_OK) { + OS_LOGE(TAG, "Failed to register the AP start handler: %s", esp_err_to_name(evtErr)); + return false; + } + apStartHandlerRegistered = true; + } + // AP SSID = prefix + this device's STA MAC (matches the old Arduino naming). uint8_t mac[6]; esp_read_mac(mac, ESP_MAC_WIFI_STA); @@ -190,6 +215,9 @@ static bool captiveportal_start() apConfig.ap.max_connection = CONFIG_OPENSHOCK_PORTAL_MAX_CLIENTS; apConfig.ap.beacon_interval = 100; + s_apIfName[0] = '\0'; + xSemaphoreTake(s_apStarted, 0); // drop a start left over from an earlier run + esp_err_t err = esp_wifi_set_mode(WIFI_MODE_APSTA); if (err != ESP_OK) { OS_LOGE(TAG, "Failed to enable AP mode: %s", esp_err_to_name(err)); @@ -203,6 +231,22 @@ static bool captiveportal_start() return false; } + // The servers bind to the AP's lwIP interface, which only exists once the AP has started. Reading its name earlier + // returns another interface's (the STA's), and would put the portal on the LAN. + if (!esp_netif_is_netif_up(s_apNetif) && xSemaphoreTake(s_apStarted, kApStartTimeout) != pdTRUE) { + OS_LOGE(TAG, "AP did not start"); + esp_wifi_set_mode(WIFI_MODE_STA); + return false; + } + + err = esp_netif_get_netif_impl_name(s_apNetif, s_apIfName); + if (err != ESP_OK) { + s_apIfName[0] = '\0'; + OS_LOGE(TAG, "Failed to get the AP interface name: %s", esp_err_to_name(err)); + esp_wifi_set_mode(WIFI_MODE_STA); + return false; // the servers could not be bound to the AP, and must not listen on the LAN + } + if (!CreateInstance()) { esp_wifi_set_mode(WIFI_MODE_STA); return false; @@ -367,8 +411,9 @@ bool CaptivePortal::Init() } s_forceCloseDone = xSemaphoreCreateBinary(); - if (s_forceCloseDone == nullptr) { - OS_LOGE(TAG, "Failed to create captive portal force-close semaphore"); + s_apStarted = xSemaphoreCreateBinary(); + if (s_forceCloseDone == nullptr || s_apStarted == nullptr) { + OS_LOGE(TAG, "Failed to create captive portal semaphores"); delete initialState; return false; } @@ -455,6 +500,11 @@ const char* CaptivePortal::ApIPv4String() return CONFIG_OPENSHOCK_PORTAL_AP_IP; } +const char* CaptivePortal::ApInterfaceName() +{ + return s_apIfName; +} + bool CaptivePortal::SendMessageTXT(uint8_t socketId, std::string_view data) { auto instance = GetInstance(); diff --git a/components/captive_portal/src/captiveportal/StaticFileServer.cpp b/components/captive_portal/src/captiveportal/StaticFileServer.cpp index 65963542..5d5b8761 100644 --- a/components/captive_portal/src/captiveportal/StaticFileServer.cpp +++ b/components/captive_portal/src/captiveportal/StaticFileServer.cpp @@ -158,7 +158,7 @@ static esp_err_t sendNotModified(httpd_req_t* req, const Representation& rep) static esp_err_t sendNotAcceptable(httpd_req_t* req) { httpd_resp_set_hdr(req, "Vary", "Accept-Encoding"); - return CaptivePortal::sendResp(req, CaptivePortal::S406, HTTP::ContentType::TextPlain, "Only available with Content-Encoding: gzip\n"); + return CaptivePortal::sendResp(req, CaptivePortal::S406, HTTP::ContentType::TextPlain, "Only available with Content-Encoding: gzip"); } // httpd derives Content-Length from the buffer it is handed and has no bodiless send, so the HEAD response (the GET's @@ -211,15 +211,20 @@ void CaptivePortal::StaticFileServer::registerHandler(httpd_handle_t server) { // Decided once: the assets are mounted in the constructor and stay that way until destruction if (m_www.isMounted()) { - CaptivePortal::registerHandler(server, "/*", HTTP_GET, &StaticFileServer::staticFileHandler, this); - CaptivePortal::registerHandler(server, "/*", HTTP_HEAD, &StaticFileServer::staticFileHandler, this); + CaptivePortal::registerHandler<&StaticFileServer::staticFileHandler>(server, "/*", HTTP_GET, this); + CaptivePortal::registerHandler<&StaticFileServer::staticFileHandler>(server, "/*", HTTP_HEAD, this); } else { - CaptivePortal::registerHandler(server, "/*", HTTP_GET, &StaticFileServer::missingAssetsHandler, nullptr); + CaptivePortal::registerHandler<&StaticFileServer::missingAssetsHandler>(server, "/*", HTTP_GET, nullptr); } } esp_err_t CaptivePortal::StaticFileServer::missingAssetsHandler(httpd_req_t* req) { + // A probe still has to fail the connectivity check, or the OS never opens the portal to show this page + if (RFC8908::IsProbeRequest(req)) { + return RFC8908::EmitRedirect(req); + } + // No usable assets on the device; serve the help page for any request. return sendResp( req, @@ -245,7 +250,7 @@ esp_err_t CaptivePortal::StaticFileServer::staticFileHandler(httpd_req_t* req) // A ".." segment is never a valid asset path. Browsers normalize these away before // sending, so this only comes from hand-built requests: refuse it rather than answer like a captive probe. if (uri.find("..") != std::string_view::npos) { - return sendResp(req, S400, HTTP::ContentType::TextPlain, "Bad Request\n"); + return sendResp(req, S400, HTTP::ContentType::TextPlain, "Bad Request"); } if (uri == "/") { @@ -255,8 +260,7 @@ esp_err_t CaptivePortal::StaticFileServer::staticFileHandler(httpd_req_t* req) // Stored without the leading slash. Every body in the image is gzipped. auto asset = self->m_www.find(uri.substr(1)); if (!asset) { - // Unknown path -> captive redirect (the index fallback of a static server). - return RFC8908::EmitRedirect(req); + return RFC8908::HandleMiss(req); } if (!clientAcceptsGzip(req)) { return sendNotAcceptable(req); diff --git a/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp b/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp index de616d4e..6d3612d5 100644 --- a/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp +++ b/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp @@ -7,12 +7,19 @@ const char* const TAG = "CaptivePortalWs"; #include "GatewayConnectionManager.h" #include "Logging.h" #include "message_handlers/WebSocket.h" +#include "rfc8908/RFC8908Handler.h" #include "serialization/WSLocal.h" #include "wifi/WiFiManager.h" #include "wifi/WiFiScanManager.h" #include +// Without it httpd_uri_t has no ws_pre_handshake_cb. sdkconfig.defaults enables it, but an existing sdkconfig wins over +// the defaults, so a build directory configured before the option was added still has it off. +#if !CONFIG_HTTPD_WS_PRE_HANDSHAKE_CB_SUPPORT +#error "CONFIG_HTTPD_WS_PRE_HANDSHAKE_CB_SUPPORT is off: this build directory's sdkconfig predates it. Delete build//sdkconfig and rebuild." +#endif + using namespace OpenShock; void CaptivePortal::WebSocketEndpoint::registerHandler(httpd_handle_t server) @@ -27,6 +34,7 @@ void CaptivePortal::WebSocketEndpoint::registerHandler(httpd_handle_t server) ws.is_websocket = true; ws.handle_ws_control_frames = false; ws.supported_subprotocol = "flatbuffers"; + ws.ws_pre_handshake_cb = &WebSocketEndpoint::wsPreHandshake; ws.ws_post_handshake_cb = &WebSocketEndpoint::wsPostHandshake; if (httpd_register_uri_handler(m_server, &ws) != ESP_OK) { OS_LOGE(TAG, "Failed to register WebSocket handler"); @@ -138,6 +146,18 @@ void CaptivePortal::WebSocketEndpoint::wsSessCtxFree(void* ctx) // httpd does not call the URI handler for the handshake (esp_http_server's httpd_uri.c), only this callback, right // after it has switched the socket to WebSocket: allocate an id, arm the disconnect hook and greet the client. +// The /ws route is registered directly (it needs the websocket fields), so it does its own Host check, here before +// httpd answers the handshake. Failing closes the connection after the redirect is sent. +esp_err_t CaptivePortal::WebSocketEndpoint::wsPreHandshake(httpd_req_t* req) +{ + if (RFC8908::IsForeignHost(req)) { + OS_LOGW(TAG, "Rejecting WebSocket for another host on fd %d", httpd_req_to_sockfd(req)); + RFC8908::EmitRedirect(req); + return ESP_FAIL; + } + return ESP_OK; +} + esp_err_t CaptivePortal::WebSocketEndpoint::wsPostHandshake(httpd_req_t* req) { auto* self = static_cast(req->user_ctx); @@ -162,6 +182,11 @@ esp_err_t CaptivePortal::WebSocketEndpoint::wsHandler(httpd_req_t* req) auto* self = static_cast(req->user_ctx); int fd = httpd_req_to_sockfd(req); + // A plain GET /ws without the upgrade headers reaches here as an ordinary request: treat it like any other miss + if (httpd_ws_get_fd_info(req->handle, fd) != HTTPD_WS_CLIENT_WEBSOCKET) { + return RFC8908::HandleMiss(req); + } + // Data frame: two-call recv (length first, then payload). httpd_ws_frame_t frame = {}; esp_err_t ret = httpd_ws_recv_frame(req, &frame, 0); diff --git a/components/captive_portal/src/captiveportal/WebSocketEndpoint.h b/components/captive_portal/src/captiveportal/WebSocketEndpoint.h index 7baed6cd..9c0f896d 100644 --- a/components/captive_portal/src/captiveportal/WebSocketEndpoint.h +++ b/components/captive_portal/src/captiveportal/WebSocketEndpoint.h @@ -50,6 +50,7 @@ namespace OpenShock::CaptivePortal { std::vector reasm; // reassembly buffer for continuation frames }; + static esp_err_t wsPreHandshake(httpd_req_t* req); static esp_err_t wsPostHandshake(httpd_req_t* req); static esp_err_t wsHandler(httpd_req_t* req); diff --git a/components/dns_server/include/dns_server/DNSServer.h b/components/dns_server/include/dns_server/DNSServer.h index fa30331e..5e4a6608 100644 --- a/components/dns_server/include/dns_server/DNSServer.h +++ b/components/dns_server/include/dns_server/DNSServer.h @@ -26,10 +26,12 @@ namespace OpenShock { DNSServer(); ~DNSServer(); - /// Start answering A queries with responseIpv4 (dotted-decimal, e.g. "4.3.2.1"). + /// Start answering A queries with responseIpv4 (dotted-decimal, e.g. "4.3.2.1"), listening only on bindIpv4 and only + /// for queries that arrive on the lwIP interface bindIfName (e.g. "ap1"): the interface whose clients should be + /// captured. The address alone is not enough, as lwIP accepts a datagram for any local address on any interface. /// Idempotent, first call wins: if the server is already running this returns - /// true and keeps the address and port it was originally started with. - bool start(const char* responseIpv4, uint16_t port = 53); + /// true and keeps the addresses, interface and port it was originally started with. + bool start(const char* responseIpv4, const char* bindIpv4, const char* bindIfName, uint16_t port = 53); void stop(); bool isRunning() const { return m_task.running(); } diff --git a/components/dns_server/src/DNSServer.cpp b/components/dns_server/src/DNSServer.cpp index 6ecbdddc..8bec4d2d 100644 --- a/components/dns_server/src/DNSServer.cpp +++ b/components/dns_server/src/DNSServer.cpp @@ -14,6 +14,7 @@ const char* const TAG = "DNSServer"; #include #include #include +#include using namespace OpenShock; @@ -29,7 +30,7 @@ DNSServer::~DNSServer() stop(); } -bool DNSServer::start(const char* responseIpv4, uint16_t port) +bool DNSServer::start(const char* responseIpv4, const char* bindIpv4, const char* bindIfName, uint16_t port) { if (m_task.running()) { return true; // already running @@ -46,6 +47,20 @@ bool DNSServer::start(const char* responseIpv4, uint16_t port) static_assert(sizeof(parsed.s_addr) == sizeof(m_ip), "s_addr and m_ip must be the same size"); memcpy(m_ip, &parsed.s_addr, sizeof(m_ip)); // already in network order, i.e. a.b.c.d + struct in_addr bindAddr = {}; + if (bindIpv4 == nullptr || inet_pton(AF_INET, bindIpv4, &bindAddr) != 1) { + OS_LOGE(TAG, "Invalid bind IPv4: %s", bindIpv4 != nullptr ? bindIpv4 : "(null)"); + return false; + } + + struct ifreq bindIf = {}; // zeroed, so the copied name is NUL-terminated + std::string_view ifNameView = bindIfName != nullptr ? bindIfName : ""; + if (ifNameView.empty() || ifNameView.size() >= sizeof(bindIf.ifr_name)) { + OS_LOGE(TAG, "Invalid bind interface: \"%.*s\"", static_cast(ifNameView.size()), ifNameView.data()); + return false; + } + ifNameView.copy(bindIf.ifr_name, ifNameView.size()); + m_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if (m_socket < 0) { OS_LOGE(TAG, "Failed to create DNS socket"); @@ -65,13 +80,24 @@ bool DNSServer::start(const char* responseIpv4, uint16_t port) return false; } + // Bound to one interface and one address rather than INADDR_ANY: lwIP then delivers only datagrams that arrive on + // that interface addressed to that address, so the hub does not answer every name with the portal address to anyone + // on the LAN it is connected to. The address alone would not do: lwIP accepts a datagram for any local address on + // any interface, so a LAN client could route the AP address via the hub's STA address. + if (setsockopt(m_socket, SOL_SOCKET, SO_BINDTODEVICE, &bindIf, sizeof(bindIf)) < 0) { + OS_LOGE(TAG, "Failed to bind DNS socket to interface %s (errno %d)", bindIfName, errno); + close(m_socket); + m_socket = -1; + return false; + } + struct sockaddr_in addr = {}; addr.sin_family = AF_INET; - addr.sin_addr.s_addr = htonl(INADDR_ANY); + addr.sin_addr = bindAddr; addr.sin_port = htons(port); if (bind(m_socket, reinterpret_cast(&addr), sizeof(addr)) < 0) { - OS_LOGE(TAG, "Failed to bind DNS socket to port %u", port); + OS_LOGE(TAG, "Failed to bind DNS socket to %s:%u", bindIpv4, port); close(m_socket); m_socket = -1; return false; diff --git a/components/rfc8908/CMakeLists.txt b/components/rfc8908/CMakeLists.txt index c75f0428..f51c8838 100644 --- a/components/rfc8908/CMakeLists.txt +++ b/components/rfc8908/CMakeLists.txt @@ -5,5 +5,6 @@ idf_component_register( SRCS "src/RFC8908Handler.cpp" INCLUDE_DIRS "include" REQUIRES esp_http_server # esp_http_server.h in public header - PRIV_REQUIRES logging osjson # .cpp-only (common was unused) + PRIV_REQUIRES common logging osjson # .cpp-only (StringHelpers, IPv4Utils for the Host check) + http # ContentTypes.h ) diff --git a/components/rfc8908/include/rfc8908/RFC8908Handler.h b/components/rfc8908/include/rfc8908/RFC8908Handler.h index 209c319c..b5da4076 100644 --- a/components/rfc8908/include/rfc8908/RFC8908Handler.h +++ b/components/rfc8908/include/rfc8908/RFC8908Handler.h @@ -4,17 +4,28 @@ namespace OpenShock::RFC8908 { // Registers the OS captive-detection surface on an esp_http_server instance: - // - well-known probe URLs (/gen_204, /hotspot-detect*, /ncsi.txt, ...) -> 302 to the portal - // - the RFC 8908 endpoint /captive-portal/api -> application/captive+json - // - a 404 fallback that redirects any other unknown request to the portal + // - the RFC 8908 endpoint /captive-portal/api (or /captiveportal/api) -> application/captive+json + // Connectivity probes (/generate_204, /hotspot-detect.html, ...) get no routes: the portal's routes redirect any + // request for another host (IsForeignHost), and the static file handler's HandleMiss catches the same probes sent to + // our address. The full list is in the .cpp. // - // `apIpv4` is the portal AP address (e.g. "4.3.2.1"); the portal URL is built from it once, here. + // Assumes the server only accepts connections from the portal AP; the portal binds it to the AP interface. // - // The server must be configured with httpd_uri_match_wildcard for the prefix probe - // paths to match. + // `apIpv4` is the portal AP address (e.g. "4.3.2.1"); the portal URL is built from it once, here. esp_err_t RegisterProbeHandlers(httpd_handle_t server, const char* apIpv4); - // Emit the standard captive redirect (302 -> http:///). Exposed so a static - // file handler can share the exact same fallthrough behaviour on unknown paths. + // True when the request's Host header is a name rather than an IP literal. On the AP our own pages are always + // requested by IP, so a name is one the wildcard DNS server sent here: a connectivity probe or a user browsing + // elsewhere, which should get the captive redirect whatever the path. + bool IsForeignHost(httpd_req_t* req); + + // True when the request's path is one of the known connectivity probe URLs (/generate_204, ...). + bool IsProbeRequest(httpd_req_t* req); + + // Emit the standard captive redirect (302 -> http:///). esp_err_t EmitRedirect(httpd_req_t* req); + + // Answer a request the server has nothing for: the captive redirect if it is addressed to another host or is a + // known connectivity probe path, otherwise a plain 404. For a catch-all handler's not-found branch. + esp_err_t HandleMiss(httpd_req_t* req); } // namespace OpenShock::RFC8908 diff --git a/components/rfc8908/src/RFC8908Handler.cpp b/components/rfc8908/src/RFC8908Handler.cpp index cd960ed0..9bf102d7 100644 --- a/components/rfc8908/src/RFC8908Handler.cpp +++ b/components/rfc8908/src/RFC8908Handler.cpp @@ -3,11 +3,16 @@ const char* const TAG = "RFC8908Handler"; #include "Logging.h" +#include "StringHelpers.h" +#include "util/IPv4Utils.h" +#include "http/ContentTypes.h" #include "json/Json.h" +#include #include #include +#include using namespace OpenShock; @@ -17,7 +22,54 @@ using namespace OpenShock; static char s_portalUrl[sizeof("http://255.255.255.255/")] = "http://4.3.2.1/"; static std::size_t s_portalUrlLen = sizeof("http://4.3.2.1/") - 1; -static const char* const captivePortalApiPath = "/captive-portal/api"; +bool RFC8908::IsForeignHost(httpd_req_t* req) +{ + // Long enough for any IPv4 literal with a port. A longer Host comes back truncated, so it is a name (or an IPv6 + // literal, which the '[' check below catches either way). + char host[sizeof("255.255.255.255:65535")]; + esp_err_t err = httpd_req_get_hdr_value_str(req, "Host", host, sizeof(host)); + if (err != ESP_OK && err != ESP_ERR_HTTPD_RESULT_TRUNC) { + return false; // No Host header (HTTP/1.0): assume it's for us + } + + std::string_view hostView(host); + if (StringHasPrefix(hostView, '[')) { + return false; // IPv6 literal + } + if (err != ESP_OK) { + return true; + } + + return !IPv4Utils::IsDottedDecimal(StringBeforeFirst(hostView, ':')); +} + +// httpd derives Content-Length from the buffer it is handed and has no bodiless send, so the HEAD response (the GET's +// headers, RFC 9110 ยง9.3.2) is written out directly. +static esp_err_t SendNotFound(httpd_req_t* req) +{ + static constexpr std::string_view kBody = "Not Found"; + + if (req->method != HTTP_HEAD) { + httpd_resp_set_status(req, "404 Not Found"); + httpd_resp_set_type(req, HTTP::ContentType::TextPlain); + return httpd_resp_send(req, kBody.data(), kBody.size()); + } + + // Fixed at compile time, so spelled out; the asserts keep it in step with the GET response above + static constexpr std::string_view kHead = "HTTP/1.1 404 Not Found\r\nContent-Type: text/plain\r\nContent-Length: 9\r\n\r\n"; + static_assert(std::string_view(HTTP::ContentType::TextPlain) == "text/plain"); + static_assert(kBody.size() == 9); + + std::size_t sent = 0; + while (sent < kHead.size()) { + int n = httpd_send(req, kHead.data() + sent, kHead.size() - sent); + if (n <= 0) { + return ESP_FAIL; + } + sent += static_cast(n); + } + return ESP_OK; +} esp_err_t RFC8908::EmitRedirect(httpd_req_t* req) { @@ -29,16 +81,50 @@ esp_err_t RFC8908::EmitRedirect(httpd_req_t* req) return httpd_resp_send(req, nullptr, 0); } -// Catch-all handler for OS connectivity probes. Operating systems attempt to detect -// internet access by requesting well-known URLs; redirecting these to the portal root -// intentionally fails the connectivity check and activates captive-portal mode. -static esp_err_t probeHandler(httpd_req_t* req) +// Paths of the OS connectivity probes. These normally arrive addressed to the probe's own host (wildcard DNS sends +// every host to us) and get redirected on that alone; the table catches the same probes sent to our address. +// clang-format off +static constexpr std::string_view kProbePaths[] = { + "/generate_204", // Android, ChromeOS, Chrome: connectivitycheck.gstatic.com, connectivitycheck.android.com, www.gstatic.com, clients3.google.com, www.googleapis.cn (China), connect.rom.miui.com (Xiaomi) + "/generate204", // Unknown Android OEM, seen in the field + "/gen_204", // Google + "/gen204", // Variant of the above + "/captiveportal/generate_204", // Edge: edge-http.microsoft.com + "/hotspot-detect.html", // Apple iOS/macOS: captive.apple.com + "/connecttest.txt", // Windows: www.msftconnecttest.com + "/ncsi.txt", // Windows (legacy NCSI): www.msftncsi.com + "/success.txt", // Firefox: detectportal.firefox.com + "/canonical.html", // Firefox: detectportal.firefox.com + "/check_network_status.txt", // GNOME NetworkManager: nmcheck.gnome.org +}; +// clang-format on + +static bool IsProbePath(std::string_view path) +{ + return std::ranges::any_of(kProbePaths, [path](std::string_view probePath) { return StringIEquals(path, probePath); }); +} + +bool RFC8908::IsProbeRequest(httpd_req_t* req) +{ + // Path only: httpd leaves the query string in req->uri + std::string_view path(req->uri); + return IsProbePath(path.substr(0, path.find('?'))); +} + +esp_err_t RFC8908::HandleMiss(httpd_req_t* req) { - return RFC8908::EmitRedirect(req); + if (IsForeignHost(req) || IsProbeRequest(req)) { + return EmitRedirect(req); + } + + // Addressed to us, so a real miss (a stale asset name, a typo): say so instead of answering with the portal + return SendNotFound(req); } // RFC 8908 Captive Portal API endpoint: machine-readable captive state. Complements // the classic redirect-based detection used by Android, iOS/macOS, and Windows. +// Registered directly rather than through the portal's Host-checked routes: the API is meant to be fetched by the OS +// and only reports public state. static esp_err_t captiveApiHandler(httpd_req_t* req) { OS_LOGI(TAG, "Got Captive API request"); @@ -60,11 +146,6 @@ static esp_err_t captiveApiHandler(httpd_req_t* req) return httpd_resp_send(req, jsonStr.data(), jsonStr.size()); } -static esp_err_t err404Handler(httpd_req_t* req, httpd_err_code_t) -{ - return RFC8908::EmitRedirect(req); -} - esp_err_t RFC8908::RegisterProbeHandlers(httpd_handle_t server, const char* apIpv4) { if (apIpv4 != nullptr) { @@ -76,37 +157,26 @@ esp_err_t RFC8908::RegisterProbeHandlers(httpd_handle_t server, const char* apIp s_portalUrlLen = static_cast(len); } - // Well-known OS probe paths. Prefix forms (e.g. "/hotspot-detect*") require the - // server to be configured with httpd_uri_match_wildcard. - static const char* const probePaths[] = { - captivePortalApiPath, // handled specially below - "/gen_204", - "/generate_204", - "/ncsi.txt", - "/hotspot-detect*", - "/success*", - "/connecttest*", - "/check_network_status*", - "/canonical*", + // RFC 8908 API. The DHCP option 114 URI (Manager.cpp) points at the first path. + static const char* const apiPaths[] = { + "/captive-portal/api", + "/captiveportal/api", }; esp_err_t result = ESP_OK; - for (const char* path : probePaths) { + for (const char* path : apiPaths) { httpd_uri_t def = {}; def.uri = path; def.method = HTTP_GET; - def.handler = (path == captivePortalApiPath) ? captiveApiHandler : probeHandler; + def.handler = captiveApiHandler; def.user_ctx = nullptr; esp_err_t err = httpd_register_uri_handler(server, &def); if (err != ESP_OK) { - OS_LOGE(TAG, "Failed to register probe handler %s: %s", path, esp_err_to_name(err)); + OS_LOGE(TAG, "Failed to register captive API handler %s: %s", path, esp_err_to_name(err)); result = err; } } - // Any other unknown request also redirects to the portal. - httpd_register_err_handler(server, HTTPD_404_NOT_FOUND, err404Handler); - return result; } diff --git a/sdkconfig.defaults b/sdkconfig.defaults index 8a03b7f5..06ebe841 100644 --- a/sdkconfig.defaults +++ b/sdkconfig.defaults @@ -23,6 +23,8 @@ CONFIG_HTTPD_WS_SUPPORT=y # The URI handler is not called for the handshake itself, so greeting a new client (the Ready message with the config # and valid GPIO pins) needs the post-handshake callback. CONFIG_HTTPD_WS_POST_HANDSHAKE_CB_SUPPORT=y +# Lets /ws refuse a handshake addressed to another host before httpd answers it (WebSocketEndpoint::wsPreHandshake). +CONFIG_HTTPD_WS_PRE_HANDSHAKE_CB_SUPPORT=y # Nothing subscribes to ESP_HTTP_SERVER_EVENT; posting them only adds traffic to the default event loop's queue, # which the WiFi, IP and OpenShock events depend on. CONFIG_HTTPD_ENABLE_EVENTS=n