From 7f4c260f2d652c781e37d92b4c8b3eaa221d178b Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Tue, 3 Nov 2026 23:26:43 +0100 Subject: [PATCH 01/10] fix(captive-portal): route by Host header instead of probe path list Requests addressed to another host (connectivity probes, pages on hijacked names) now get the captive redirect from every portal route, via a Host check wrapped around registerHandler. Misses addressed to the hub itself get a 404 instead of a redirect to the index, except for known probe paths, which still redirect. - Drop the per-path probe routes; document the probe URLs in a table used by RFC8908::HandleMiss - Reject /api/* and /ws for other hosts (/ws before the handshake, needs CONFIG_HTTPD_WS_PRE_HANDSHAKE_CB_SUPPORT) - Answer a plain GET /ws with HandleMiss instead of dropping it - Serve /captive-portal/api also as /captiveportal/api - Remove the unreachable 404 error handler --- .../src/captiveportal/ApiHandlers.cpp | 24 ++--- .../captiveportal/CaptivePortalInstance.cpp | 4 +- .../src/captiveportal/HttpUtils.cpp | 2 +- .../src/captiveportal/HttpUtils.h | 27 ++++- .../src/captiveportal/StaticFileServer.cpp | 13 ++- .../src/captiveportal/WebSocketEndpoint.cpp | 19 ++++ .../src/captiveportal/WebSocketEndpoint.h | 1 + components/rfc8908/CMakeLists.txt | 2 +- .../rfc8908/include/rfc8908/RFC8908Handler.h | 22 ++-- components/rfc8908/src/RFC8908Handler.cpp | 100 +++++++++++++----- sdkconfig.defaults | 2 + 11 files changed, 154 insertions(+), 62 deletions(-) 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..026cf12d 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -23,8 +23,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); 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/StaticFileServer.cpp b/components/captive_portal/src/captiveportal/StaticFileServer.cpp index 65963542..ad51c891 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,10 +211,10 @@ 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); } } @@ -245,7 +245,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 +255,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..00268da2 100644 --- a/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp +++ b/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp @@ -7,6 +7,7 @@ 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" @@ -27,6 +28,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 +140,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 +176,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/rfc8908/CMakeLists.txt b/components/rfc8908/CMakeLists.txt index c75f0428..7b98e05e 100644 --- a/components/rfc8908/CMakeLists.txt +++ b/components/rfc8908/CMakeLists.txt @@ -5,5 +5,5 @@ 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) ) diff --git a/components/rfc8908/include/rfc8908/RFC8908Handler.h b/components/rfc8908/include/rfc8908/RFC8908Handler.h index 209c319c..9998e310 100644 --- a/components/rfc8908/include/rfc8908/RFC8908Handler.h +++ b/components/rfc8908/include/rfc8908/RFC8908Handler.h @@ -4,17 +4,23 @@ 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. - // - // The server must be configured with httpd_uri_match_wildcard for the prefix probe - // paths to match. 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. Our own pages are always requested by + // IP (the AP address, or the hub's LAN address), 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); + + // 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..213e3893 100644 --- a/components/rfc8908/src/RFC8908Handler.cpp +++ b/components/rfc8908/src/RFC8908Handler.cpp @@ -3,11 +3,14 @@ const char* const TAG = "RFC8908Handler"; #include "Logging.h" +#include "StringHelpers.h" +#include "util/IPv4Utils.h" #include "json/Json.h" #include #include +#include using namespace OpenShock; @@ -17,7 +20,26 @@ 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, ':')); +} esp_err_t RFC8908::EmitRedirect(httpd_req_t* req) { @@ -29,12 +51,48 @@ 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 RFC8908::EmitRedirect(req); + for (std::string_view probePath : kProbePaths) { + if (StringIEquals(path, probePath)) { + return true; + } + } + return false; +} + +esp_err_t RFC8908::HandleMiss(httpd_req_t* req) +{ + // Path only: httpd leaves the query string in req->uri + std::string_view path(req->uri); + path = path.substr(0, path.find('?')); + + if (IsForeignHost(req) || IsProbePath(path)) { + return EmitRedirect(req); + } + + // Addressed to us, so a real miss (a stale asset name, a typo): say so instead of answering with the portal + httpd_resp_set_status(req, "404 Not Found"); + httpd_resp_set_type(req, "text/plain"); + return httpd_resp_sendstr(req, "Not Found"); } // RFC 8908 Captive Portal API endpoint: machine-readable captive state. Complements @@ -60,11 +118,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 +129,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 From 8f6b7c471799c5171a272854afef74e3df883a0b Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Tue, 3 Nov 2026 23:41:26 +0100 Subject: [PATCH 02/10] fix(captive-portal): apply captive routing only to AP connections The portal server also answers on the hub's LAN address, where a Host name is normal (the DHCP hostname in the router's DNS). Redirecting those to the AP address broke LAN access by name. Captive behaviour now requires the connection's local address to be the AP IP. - Rename IsForeignHost to IsCaptured: a Host name and an AP connection - HandleMiss redirects only for AP connections; the RFC 8908 API answers 404 on the LAN - Send HEAD 404s without a body - Fail with a clear #error when a stale sdkconfig lacks CONFIG_HTTPD_WS_PRE_HANDSHAKE_CB_SUPPORT --- .../src/captiveportal/HttpUtils.h | 8 +- .../src/captiveportal/WebSocketEndpoint.cpp | 8 +- components/rfc8908/CMakeLists.txt | 2 + .../rfc8908/include/rfc8908/RFC8908Handler.h | 23 ++-- components/rfc8908/src/RFC8908Handler.cpp | 116 ++++++++++++++++-- 5 files changed, 130 insertions(+), 27 deletions(-) diff --git a/components/captive_portal/src/captiveportal/HttpUtils.h b/components/captive_portal/src/captiveportal/HttpUtils.h index 9cd9e3aa..84750f9c 100644 --- a/components/captive_portal/src/captiveportal/HttpUtils.h +++ b/components/captive_portal/src/captiveportal/HttpUtils.h @@ -31,13 +31,13 @@ namespace OpenShock::CaptivePortal { 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. + // httpd has no hook that runs before every request, so each route is wrapped in this. A request that came in over + // the AP 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. LAN requests always pass. template esp_err_t ownHostOnly(httpd_req_t* req) { - if (RFC8908::IsForeignHost(req)) { + if (RFC8908::IsCaptured(req)) { return RFC8908::EmitRedirect(req); } return Handler(req); diff --git a/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp b/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp index 00268da2..bb88b9f0 100644 --- a/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp +++ b/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp @@ -14,6 +14,12 @@ const char* const TAG = "CaptivePortalWs"; #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) @@ -144,7 +150,7 @@ void CaptivePortal::WebSocketEndpoint::wsSessCtxFree(void* ctx) // 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)) { + if (RFC8908::IsCaptured(req)) { OS_LOGW(TAG, "Rejecting WebSocket for another host on fd %d", httpd_req_to_sockfd(req)); RFC8908::EmitRedirect(req); return ESP_FAIL; diff --git a/components/rfc8908/CMakeLists.txt b/components/rfc8908/CMakeLists.txt index 7b98e05e..df9ba827 100644 --- a/components/rfc8908/CMakeLists.txt +++ b/components/rfc8908/CMakeLists.txt @@ -6,4 +6,6 @@ idf_component_register( INCLUDE_DIRS "include" REQUIRES esp_http_server # esp_http_server.h in public header PRIV_REQUIRES common logging osjson # .cpp-only (StringHelpers, IPv4Utils for the Host check) + http # ContentTypes.h + lwip # getsockname/inet_pton for the AP check ) diff --git a/components/rfc8908/include/rfc8908/RFC8908Handler.h b/components/rfc8908/include/rfc8908/RFC8908Handler.h index 9998e310..d8917a5e 100644 --- a/components/rfc8908/include/rfc8908/RFC8908Handler.h +++ b/components/rfc8908/include/rfc8908/RFC8908Handler.h @@ -6,21 +6,26 @@ namespace OpenShock::RFC8908 { // Registers the OS captive-detection surface on an esp_http_server instance: // - 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. + // captured request (IsCaptured), 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. + // All of this applies only to connections that came in over the portal AP. The same server also answers on the + // hub's LAN address, where it is a plain web server: no redirects, and the RFC 8908 endpoint answers 404. + // + // `apIpv4` is the portal AP address (e.g. "4.3.2.1"); the portal URL and the AP check are built from it once, here. esp_err_t RegisterProbeHandlers(httpd_handle_t server, const char* apIpv4); - // True when the request's Host header is a name rather than an IP literal. Our own pages are always requested by - // IP (the AP address, or the hub's LAN address), 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 came in over the portal AP and its 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. On the LAN a name + // is normal (the hub's DHCP hostname in the router's DNS) and never counts. + bool IsCaptured(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. + // Answer a request the server has nothing for: the captive redirect if it came in over the AP and 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 213e3893..a75e42c5 100644 --- a/components/rfc8908/src/RFC8908Handler.cpp +++ b/components/rfc8908/src/RFC8908Handler.cpp @@ -6,9 +6,14 @@ const char* const TAG = "RFC8908Handler"; #include "StringHelpers.h" #include "util/IPv4Utils.h" +#include "http/ContentTypes.h" #include "json/Json.h" +#include + +#include #include +#include #include #include @@ -20,7 +25,41 @@ 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; -bool RFC8908::IsForeignHost(httpd_req_t* req) +// The same address in network byte order, matched against a connection's local address. Zero (no match) until +// registration sets it. +static in_addr_t s_apAddr = 0; + +// True when the connection came in over the portal AP: its local address is the AP's. A client on the hub's LAN +// connects to the STA address instead, and must get the plain server: no captive redirects, no RFC 8908. +static bool ArrivedOnAp(httpd_req_t* req) +{ + sockaddr_storage local = {}; + socklen_t len = sizeof(local); + if (getsockname(httpd_req_to_sockfd(req), reinterpret_cast(&local), &len) != 0) { + return false; + } + + in_addr_t addr; + if (local.ss_family == AF_INET) { + addr = reinterpret_cast(&local)->sin_addr.s_addr; + } else if (local.ss_family == AF_INET6) { + // httpd listens on a dual-stack IPv6 socket when lwIP has IPv6, so an IPv4 client shows up as ::ffff:a.b.c.d + static constexpr std::uint8_t kV4MappedPrefix[12] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff}; + const in6_addr& addr6 = reinterpret_cast(&local)->sin6_addr; + if (std::memcmp(addr6.s6_addr, kV4MappedPrefix, sizeof(kV4MappedPrefix)) != 0) { + return false; + } + std::memcpy(&addr, addr6.s6_addr + sizeof(kV4MappedPrefix), sizeof(addr)); + } else { + return false; + } + + return addr == s_apAddr; +} + +// 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. +static bool HostIsName(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). @@ -41,6 +80,43 @@ bool RFC8908::IsForeignHost(httpd_req_t* req) return !IPv4Utils::IsDottedDecimal(StringBeforeFirst(hostView, ':')); } +// The Host check first: it is a header lookup, while getsockname goes through the lwIP core +bool RFC8908::IsCaptured(httpd_req_t* req) +{ + return HostIsName(req) && ArrivedOnAp(req); +} + +// 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()); + } + + std::string head; + head.reserve(96); + head += "HTTP/1.1 404 Not Found\r\nContent-Type: "; + head += HTTP::ContentType::TextPlain; + head += "\r\nContent-Length: "; + head += std::to_string(kBody.size()); + head += "\r\n\r\n"; + + std::size_t sent = 0; + while (sent < head.size()) { + int n = httpd_send(req, head.data() + sent, head.size() - sent); + if (n <= 0) { + return ESP_FAIL; + } + sent += static_cast(n); + } + return ESP_OK; +} + esp_err_t RFC8908::EmitRedirect(httpd_req_t* req) { httpd_resp_set_status(req, "302 Found"); @@ -85,20 +161,25 @@ esp_err_t RFC8908::HandleMiss(httpd_req_t* req) std::string_view path(req->uri); path = path.substr(0, path.find('?')); - if (IsForeignHost(req) || IsProbePath(path)) { + if ((HostIsName(req) || IsProbePath(path)) && ArrivedOnAp(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 - httpd_resp_set_status(req, "404 Not Found"); - httpd_resp_set_type(req, "text/plain"); - return httpd_resp_sendstr(req, "Not Found"); + // Addressed to us, or from the LAN: a real miss (a stale asset name, a typo), so 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. On the LAN the hub is not captive, so there it does not exist. static esp_err_t captiveApiHandler(httpd_req_t* req) { + if (!ArrivedOnAp(req)) { + return SendNotFound(req); + } + OS_LOGI(TAG, "Got Captive API request"); OpenShock::JSON::StringWriter writer; @@ -120,14 +201,23 @@ static esp_err_t captiveApiHandler(httpd_req_t* req) esp_err_t RFC8908::RegisterProbeHandlers(httpd_handle_t server, const char* apIpv4) { - if (apIpv4 != nullptr) { - int len = snprintf(s_portalUrl, sizeof(s_portalUrl), "http://%s/", apIpv4); - if (len < 0 || static_cast(len) >= sizeof(s_portalUrl)) { - OS_LOGE(TAG, "Not an IPv4 address: %s", apIpv4); - return ESP_ERR_INVALID_ARG; - } - s_portalUrlLen = static_cast(len); + if (apIpv4 == nullptr) { + apIpv4 = "4.3.2.1"; // the s_portalUrl default + } + + in_addr addr = {}; + if (inet_pton(AF_INET, apIpv4, &addr) != 1) { + OS_LOGE(TAG, "Not an IPv4 address: %s", apIpv4); + return ESP_ERR_INVALID_ARG; + } + s_apAddr = addr.s_addr; + + int len = snprintf(s_portalUrl, sizeof(s_portalUrl), "http://%s/", apIpv4); + if (len < 0 || static_cast(len) >= sizeof(s_portalUrl)) { + OS_LOGE(TAG, "Not an IPv4 address: %s", apIpv4); + return ESP_ERR_INVALID_ARG; } + s_portalUrlLen = static_cast(len); // RFC 8908 API. The DHCP option 114 URI (Manager.cpp) points at the first path. static const char* const apiPaths[] = { From a50c11297a86c88883a05f08de330195b0be3014 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Tue, 3 Nov 2026 23:45:46 +0100 Subject: [PATCH 03/10] chore(changes): add change file for captive portal host routing --- .changes/captive-portal-host-routing.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .changes/captive-portal-host-routing.md diff --git a/.changes/captive-portal-host-routing.md b/.changes/captive-portal-host-routing.md new file mode 100644 index 00000000..c60855e7 --- /dev/null +++ b/.changes/captive-portal-host-routing.md @@ -0,0 +1,13 @@ +--- +kind: fixed +--- +Route captive portal requests by Host header, and only on the portal AP + +## Release Note +More reliable captive portal detection, and the portal works normally on your home network + +Phones and computers joining the hub's setup network now reliably open the sign-in screen, whatever address they check. When the hub is reached over your home network, including by its hostname, the portal behaves like a normal web page and no longer redirects. + +- 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, and only on the setup network From 04d34cb425b272818803c943684ce204b4d71285 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Tue, 3 Nov 2026 23:48:32 +0100 Subject: [PATCH 04/10] fix(dns-server): listen only on the portal AP address The wildcard DNS responder bound INADDR_ANY, so while the portal ran alongside a WiFi connection it also answered every query on the hub's LAN address with the AP IP. Bind it to the AP address instead; lwIP then drops queries sent to any other address. --- .../src/captiveportal/CaptivePortalInstance.cpp | 5 +++-- .../dns_server/include/dns_server/DNSServer.h | 8 +++++--- components/dns_server/src/DNSServer.cpp | 14 +++++++++++--- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index 026cf12d..d4888bd3 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -78,8 +78,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 address only: the hub's LAN + // address must not answer DNS. + m_dnsStarted = m_dnsServer.start(CaptivePortal::ApIPv4String(), CaptivePortal::ApIPv4String()); if (!m_dnsStarted) { OS_LOGE(TAG, "Failed to start DNS server"); } diff --git a/components/dns_server/include/dns_server/DNSServer.h b/components/dns_server/include/dns_server/DNSServer.h index fa30331e..316d450e 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: the + /// address of the interface whose clients should be captured. Queries sent to the hub's other addresses (its LAN + /// address while connected to WiFi) never reach the server. /// 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 and port it was originally started with. + bool start(const char* responseIpv4, const char* bindIpv4, 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..1cc8b827 100644 --- a/components/dns_server/src/DNSServer.cpp +++ b/components/dns_server/src/DNSServer.cpp @@ -29,7 +29,7 @@ DNSServer::~DNSServer() stop(); } -bool DNSServer::start(const char* responseIpv4, uint16_t port) +bool DNSServer::start(const char* responseIpv4, const char* bindIpv4, uint16_t port) { if (m_task.running()) { return true; // already running @@ -46,6 +46,12 @@ 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; + } + m_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if (m_socket < 0) { OS_LOGE(TAG, "Failed to create DNS socket"); @@ -65,13 +71,15 @@ bool DNSServer::start(const char* responseIpv4, uint16_t port) return false; } + // Bound to one address rather than INADDR_ANY: lwIP then delivers only datagrams addressed to it, so the hub does + // not answer every name with the portal address to anyone on the LAN it is connected to. 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; From ed2695f08042d117bd0c2febc7ca98c8f982a04e Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Tue, 3 Nov 2026 23:48:38 +0100 Subject: [PATCH 05/10] chore(changes): note the DNS bind fix --- .changes/captive-portal-host-routing.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changes/captive-portal-host-routing.md b/.changes/captive-portal-host-routing.md index c60855e7..683524a6 100644 --- a/.changes/captive-portal-host-routing.md +++ b/.changes/captive-portal-host-routing.md @@ -11,3 +11,4 @@ Phones and computers joining the hub's setup network now reliably open the sign- - 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, and only on the setup network +- The setup network's DNS server no longer answers queries from your home network From 68eb3119f0245eb855389653442667dbb19b2726 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Tue, 3 Nov 2026 23:51:50 +0100 Subject: [PATCH 06/10] fix(captive-portal): refuse connections that do not come in over the AP The portal's HTTP API and /ws are the local protocol, which has no compatibility rules, so nothing on the LAN should reach them. httpd always listens on every interface and has no bind address, so the portal's open_fn now closes any connection whose local address is not the AP IP. With the whole server AP-only, the per-request AP checks in rfc8908 go away again: IsCaptured is back to IsForeignHost, and the RFC 8908 API no longer needs a LAN 404. --- .changes/captive-portal-host-routing.md | 12 +-- .../captiveportal/CaptivePortalInstance.cpp | 48 +++++++++++ .../src/captiveportal/HttpUtils.h | 8 +- .../src/captiveportal/WebSocketEndpoint.cpp | 2 +- components/rfc8908/CMakeLists.txt | 1 - .../rfc8908/include/rfc8908/RFC8908Handler.h | 23 +++--- components/rfc8908/src/RFC8908Handler.cpp | 80 +++---------------- 7 files changed, 81 insertions(+), 93 deletions(-) diff --git a/.changes/captive-portal-host-routing.md b/.changes/captive-portal-host-routing.md index 683524a6..a4796b75 100644 --- a/.changes/captive-portal-host-routing.md +++ b/.changes/captive-portal-host-routing.md @@ -1,14 +1,16 @@ --- kind: fixed --- -Route captive portal requests by Host header, and only on the portal AP +Route captive portal requests by Host header, and serve the portal only on its own network ## Release Note -More reliable captive portal detection, and the portal works normally on your home network +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. When the hub is reached over your home network, including by its hostname, the portal behaves like a normal web page and no longer redirects. +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, and only on the setup network -- The setup network's DNS server no longer answers queries from your home network +- 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/src/captiveportal/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index d4888bd3..70d8e5b7 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"); @@ -33,10 +36,48 @@ void CaptivePortal::CaptivePortalInstance::registerHandlers() m_staticFiles.registerHandler(m_server); } +// The portal AP address in network byte order, set before the server starts +static in_addr_t s_apAddr = 0; + +// True when the connection came in over the portal AP: its local address is the AP's. A client on the hub's LAN +// connects to the STA address instead. +static bool arrivedOnAp(int sockfd) +{ + sockaddr_storage local = {}; + socklen_t len = sizeof(local); + if (getsockname(sockfd, reinterpret_cast(&local), &len) != 0) { + return false; + } + + in_addr_t addr; + if (local.ss_family == AF_INET) { + addr = reinterpret_cast(&local)->sin_addr.s_addr; + } else if (local.ss_family == AF_INET6) { + // httpd listens on a dual-stack IPv6 socket when lwIP has IPv6, so an IPv4 client shows up as ::ffff:a.b.c.d + static constexpr uint8_t kV4MappedPrefix[12] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff}; + const in6_addr& addr6 = reinterpret_cast(&local)->sin6_addr; + if (memcmp(addr6.s6_addr, kV4MappedPrefix, sizeof(kV4MappedPrefix)) != 0) { + return false; + } + memcpy(&addr, addr6.s6_addr + sizeof(kV4MappedPrefix), sizeof(addr)); + } else { + return false; + } + + return addr == s_apAddr; +} + // httpd turns Nagle off only around error responses. A websocket frame goes out as two writes (header, then payload), // and Nagle holds the second until the first is acknowledged, which a delayed-ACK client sends up to ~200 ms later. static esp_err_t onSessionOpen(httpd_handle_t /*server*/, int sockfd) { + // httpd always listens on every interface and has no bind address, so the LAN is refused here instead. The portal's + // API is the local protocol, which has no compatibility rules: nothing on the LAN may come to depend on it. + if (!arrivedOnAp(sockfd)) { + OS_LOGW(TAG, "Refusing a connection that did not come in over the portal AP (fd %d)", sockfd); + return ESP_FAIL; // httpd closes the socket + } + int one = 1; if (setsockopt(sockfd, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one)) != 0) { OS_LOGW(TAG, "Failed to set TCP_NODELAY on fd %d (errno %d)", sockfd, errno); @@ -46,6 +87,13 @@ static esp_err_t onSessionOpen(httpd_handle_t /*server*/, int sockfd) bool CaptivePortal::CaptivePortalInstance::startHttpServer() { + in_addr apAddr = {}; + if (inet_pton(AF_INET, CaptivePortal::ApIPv4String(), &apAddr) != 1) { + OS_LOGE(TAG, "Invalid portal AP address: %s", CaptivePortal::ApIPv4String()); + return false; + } + s_apAddr = apAddr.s_addr; + httpd_config_t config = HTTPD_DEFAULT_CONFIG(); config.server_port = HTTP_PORT; config.max_uri_handlers = 40; diff --git a/components/captive_portal/src/captiveportal/HttpUtils.h b/components/captive_portal/src/captiveportal/HttpUtils.h index 84750f9c..9cd9e3aa 100644 --- a/components/captive_portal/src/captiveportal/HttpUtils.h +++ b/components/captive_portal/src/captiveportal/HttpUtils.h @@ -31,13 +31,13 @@ namespace OpenShock::CaptivePortal { 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 that came in over - // the AP 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. LAN requests always pass. + // 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::IsCaptured(req)) { + if (RFC8908::IsForeignHost(req)) { return RFC8908::EmitRedirect(req); } return Handler(req); diff --git a/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp b/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp index bb88b9f0..6d3612d5 100644 --- a/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp +++ b/components/captive_portal/src/captiveportal/WebSocketEndpoint.cpp @@ -150,7 +150,7 @@ void CaptivePortal::WebSocketEndpoint::wsSessCtxFree(void* ctx) // 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::IsCaptured(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; diff --git a/components/rfc8908/CMakeLists.txt b/components/rfc8908/CMakeLists.txt index df9ba827..f51c8838 100644 --- a/components/rfc8908/CMakeLists.txt +++ b/components/rfc8908/CMakeLists.txt @@ -7,5 +7,4 @@ idf_component_register( REQUIRES esp_http_server # esp_http_server.h in public header PRIV_REQUIRES common logging osjson # .cpp-only (StringHelpers, IPv4Utils for the Host check) http # ContentTypes.h - lwip # getsockname/inet_pton for the AP check ) diff --git a/components/rfc8908/include/rfc8908/RFC8908Handler.h b/components/rfc8908/include/rfc8908/RFC8908Handler.h index d8917a5e..8e21e8c0 100644 --- a/components/rfc8908/include/rfc8908/RFC8908Handler.h +++ b/components/rfc8908/include/rfc8908/RFC8908Handler.h @@ -6,26 +6,23 @@ namespace OpenShock::RFC8908 { // Registers the OS captive-detection surface on an esp_http_server instance: // - 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 - // captured request (IsCaptured), and the static file handler's HandleMiss catches the same probes sent to our - // address. The full list is in the .cpp. + // 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. // - // All of this applies only to connections that came in over the portal AP. The same server also answers on the - // hub's LAN address, where it is a plain web server: no redirects, and the RFC 8908 endpoint answers 404. + // Assumes the server only accepts connections from the portal AP; the portal refuses the rest when they open. // - // `apIpv4` is the portal AP address (e.g. "4.3.2.1"); the portal URL and the AP check are built from it once, here. + // `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); - // True when the request came in over the portal AP and its 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. On the LAN a name - // is normal (the hub's DHCP hostname in the router's DNS) and never counts. - bool IsCaptured(httpd_req_t* req); + // 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); // 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 came in over the AP and 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. + // 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 a75e42c5..0b2c4531 100644 --- a/components/rfc8908/src/RFC8908Handler.cpp +++ b/components/rfc8908/src/RFC8908Handler.cpp @@ -9,11 +9,7 @@ const char* const TAG = "RFC8908Handler"; #include "http/ContentTypes.h" #include "json/Json.h" -#include - -#include #include -#include #include #include @@ -25,41 +21,7 @@ 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; -// The same address in network byte order, matched against a connection's local address. Zero (no match) until -// registration sets it. -static in_addr_t s_apAddr = 0; - -// True when the connection came in over the portal AP: its local address is the AP's. A client on the hub's LAN -// connects to the STA address instead, and must get the plain server: no captive redirects, no RFC 8908. -static bool ArrivedOnAp(httpd_req_t* req) -{ - sockaddr_storage local = {}; - socklen_t len = sizeof(local); - if (getsockname(httpd_req_to_sockfd(req), reinterpret_cast(&local), &len) != 0) { - return false; - } - - in_addr_t addr; - if (local.ss_family == AF_INET) { - addr = reinterpret_cast(&local)->sin_addr.s_addr; - } else if (local.ss_family == AF_INET6) { - // httpd listens on a dual-stack IPv6 socket when lwIP has IPv6, so an IPv4 client shows up as ::ffff:a.b.c.d - static constexpr std::uint8_t kV4MappedPrefix[12] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff}; - const in6_addr& addr6 = reinterpret_cast(&local)->sin6_addr; - if (std::memcmp(addr6.s6_addr, kV4MappedPrefix, sizeof(kV4MappedPrefix)) != 0) { - return false; - } - std::memcpy(&addr, addr6.s6_addr + sizeof(kV4MappedPrefix), sizeof(addr)); - } else { - return false; - } - - return addr == s_apAddr; -} - -// 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. -static bool HostIsName(httpd_req_t* req) +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). @@ -80,12 +42,6 @@ static bool HostIsName(httpd_req_t* req) return !IPv4Utils::IsDottedDecimal(StringBeforeFirst(hostView, ':')); } -// The Host check first: it is a header lookup, while getsockname goes through the lwIP core -bool RFC8908::IsCaptured(httpd_req_t* req) -{ - return HostIsName(req) && ArrivedOnAp(req); -} - // 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) @@ -161,25 +117,20 @@ esp_err_t RFC8908::HandleMiss(httpd_req_t* req) std::string_view path(req->uri); path = path.substr(0, path.find('?')); - if ((HostIsName(req) || IsProbePath(path)) && ArrivedOnAp(req)) { + if (IsForeignHost(req) || IsProbePath(path)) { return EmitRedirect(req); } - // Addressed to us, or from the LAN: a real miss (a stale asset name, a typo), so say so instead of answering with - // the portal + // 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. On the LAN the hub is not captive, so there it does not exist. +// and only reports public state. static esp_err_t captiveApiHandler(httpd_req_t* req) { - if (!ArrivedOnAp(req)) { - return SendNotFound(req); - } - OS_LOGI(TAG, "Got Captive API request"); OpenShock::JSON::StringWriter writer; @@ -201,23 +152,14 @@ static esp_err_t captiveApiHandler(httpd_req_t* req) esp_err_t RFC8908::RegisterProbeHandlers(httpd_handle_t server, const char* apIpv4) { - if (apIpv4 == nullptr) { - apIpv4 = "4.3.2.1"; // the s_portalUrl default - } - - in_addr addr = {}; - if (inet_pton(AF_INET, apIpv4, &addr) != 1) { - OS_LOGE(TAG, "Not an IPv4 address: %s", apIpv4); - return ESP_ERR_INVALID_ARG; - } - s_apAddr = addr.s_addr; - - int len = snprintf(s_portalUrl, sizeof(s_portalUrl), "http://%s/", apIpv4); - if (len < 0 || static_cast(len) >= sizeof(s_portalUrl)) { - OS_LOGE(TAG, "Not an IPv4 address: %s", apIpv4); - return ESP_ERR_INVALID_ARG; + if (apIpv4 != nullptr) { + int len = snprintf(s_portalUrl, sizeof(s_portalUrl), "http://%s/", apIpv4); + if (len < 0 || static_cast(len) >= sizeof(s_portalUrl)) { + OS_LOGE(TAG, "Not an IPv4 address: %s", apIpv4); + return ESP_ERR_INVALID_ARG; + } + s_portalUrlLen = static_cast(len); } - s_portalUrlLen = static_cast(len); // RFC 8908 API. The DHCP option 114 URI (Manager.cpp) points at the first path. static const char* const apiPaths[] = { From d003167c4532e9482ebbc310dac98d0a4dd36510 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Wed, 4 Nov 2026 00:08:54 +0100 Subject: [PATCH 07/10] fix(captive-portal): bind the portal's HTTP and DNS servers to the AP interface Checking or binding the AP address does not restrict where a packet came from: lwIP accepts a packet for any local address on any interface, so a LAN client that routes the AP address via the hub's STA address could still reach the portal API, /ws and the wildcard DNS server. Bind both to the AP netif with SO_BINDTODEVICE (httpd_config_t::if_name for httpd), which lwIP matches against the ingress interface. This replaces the getsockname check in open_fn. The AP netif name is read once when the portal first starts; without it the portal does not start. --- .../include/captiveportal/Manager.h | 4 ++ .../captiveportal/CaptivePortalInstance.cpp | 58 +++++-------------- .../src/captiveportal/Manager.cpp | 16 +++++ .../dns_server/include/dns_server/DNSServer.h | 10 ++-- components/dns_server/src/DNSServer.cpp | 22 ++++++- 5 files changed, 57 insertions(+), 53 deletions(-) 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/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index 70d8e5b7..5cfa1b98 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -36,48 +36,10 @@ void CaptivePortal::CaptivePortalInstance::registerHandlers() m_staticFiles.registerHandler(m_server); } -// The portal AP address in network byte order, set before the server starts -static in_addr_t s_apAddr = 0; - -// True when the connection came in over the portal AP: its local address is the AP's. A client on the hub's LAN -// connects to the STA address instead. -static bool arrivedOnAp(int sockfd) -{ - sockaddr_storage local = {}; - socklen_t len = sizeof(local); - if (getsockname(sockfd, reinterpret_cast(&local), &len) != 0) { - return false; - } - - in_addr_t addr; - if (local.ss_family == AF_INET) { - addr = reinterpret_cast(&local)->sin_addr.s_addr; - } else if (local.ss_family == AF_INET6) { - // httpd listens on a dual-stack IPv6 socket when lwIP has IPv6, so an IPv4 client shows up as ::ffff:a.b.c.d - static constexpr uint8_t kV4MappedPrefix[12] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff}; - const in6_addr& addr6 = reinterpret_cast(&local)->sin6_addr; - if (memcmp(addr6.s6_addr, kV4MappedPrefix, sizeof(kV4MappedPrefix)) != 0) { - return false; - } - memcpy(&addr, addr6.s6_addr + sizeof(kV4MappedPrefix), sizeof(addr)); - } else { - return false; - } - - return addr == s_apAddr; -} - // httpd turns Nagle off only around error responses. A websocket frame goes out as two writes (header, then payload), // and Nagle holds the second until the first is acknowledged, which a delayed-ACK client sends up to ~200 ms later. static esp_err_t onSessionOpen(httpd_handle_t /*server*/, int sockfd) { - // httpd always listens on every interface and has no bind address, so the LAN is refused here instead. The portal's - // API is the local protocol, which has no compatibility rules: nothing on the LAN may come to depend on it. - if (!arrivedOnAp(sockfd)) { - OS_LOGW(TAG, "Refusing a connection that did not come in over the portal AP (fd %d)", sockfd); - return ESP_FAIL; // httpd closes the socket - } - int one = 1; if (setsockopt(sockfd, IPPROTO_TCP, TCP_NODELAY, &one, sizeof(one)) != 0) { OS_LOGW(TAG, "Failed to set TCP_NODELAY on fd %d (errno %d)", sockfd, errno); @@ -87,12 +49,17 @@ static esp_err_t onSessionOpen(httpd_handle_t /*server*/, int sockfd) bool CaptivePortal::CaptivePortalInstance::startHttpServer() { - in_addr apAddr = {}; - if (inet_pton(AF_INET, CaptivePortal::ApIPv4String(), &apAddr) != 1) { - OS_LOGE(TAG, "Invalid portal AP address: %s", CaptivePortal::ApIPv4String()); + // 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 = {}; + const char* apIfName = CaptivePortal::ApInterfaceName(); + if (apIfName[0] == '\0' || strlen(apIfName) >= sizeof(apInterface.ifr_name)) { + OS_LOGE(TAG, "No usable AP interface name (\"%s\"), not starting the HTTP server", apIfName); return false; } - s_apAddr = apAddr.s_addr; + memcpy(apInterface.ifr_name, apIfName, strlen(apIfName) + 1); httpd_config_t config = HTTPD_DEFAULT_CONFIG(); config.server_port = HTTP_PORT; @@ -105,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; @@ -126,9 +94,9 @@ CaptivePortal::CaptivePortalInstance::CaptivePortalInstance() return; } - // Start the wildcard DNS responder (all A queries -> the portal AP IP), on the AP address only: the hub's LAN - // address must not answer DNS. - m_dnsStarted = m_dnsServer.start(CaptivePortal::ApIPv4String(), 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/Manager.cpp b/components/captive_portal/src/captiveportal/Manager.cpp index a9da284a..04bcf2cc 100644 --- a/components/captive_portal/src/captiveportal/Manager.cpp +++ b/components/captive_portal/src/captiveportal/Manager.cpp @@ -74,6 +74,8 @@ 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; +// lwIP's name for s_apNetif, which the portal's servers bind to. Read once; the netif is never destroyed. +static char s_apIfName[8] = {}; 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"); @@ -178,6 +180,15 @@ static bool captiveportal_start() esp_netif_dhcps_start(s_apNetif); } + if (s_apIfName[0] == '\0') { + esp_err_t nameErr = esp_netif_get_netif_impl_name(s_apNetif, s_apIfName); + if (nameErr != ESP_OK) { + s_apIfName[0] = '\0'; + OS_LOGE(TAG, "Failed to get the AP interface name: %s", esp_err_to_name(nameErr)); + return false; // the servers could not be bound to the AP, and must not listen on the LAN + } + } + // 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); @@ -455,6 +466,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/dns_server/include/dns_server/DNSServer.h b/components/dns_server/include/dns_server/DNSServer.h index 316d450e..5e4a6608 100644 --- a/components/dns_server/include/dns_server/DNSServer.h +++ b/components/dns_server/include/dns_server/DNSServer.h @@ -26,12 +26,12 @@ namespace OpenShock { DNSServer(); ~DNSServer(); - /// Start answering A queries with responseIpv4 (dotted-decimal, e.g. "4.3.2.1"), listening only on bindIpv4: the - /// address of the interface whose clients should be captured. Queries sent to the hub's other addresses (its LAN - /// address while connected to WiFi) never reach the server. + /// 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 addresses and port it was originally started with. - bool start(const char* responseIpv4, const char* bindIpv4, 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 1cc8b827..923a27bd 100644 --- a/components/dns_server/src/DNSServer.cpp +++ b/components/dns_server/src/DNSServer.cpp @@ -29,7 +29,7 @@ DNSServer::~DNSServer() stop(); } -bool DNSServer::start(const char* responseIpv4, const char* bindIpv4, 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 @@ -52,6 +52,13 @@ bool DNSServer::start(const char* responseIpv4, const char* bindIpv4, uint16_t p return false; } + struct ifreq bindIf = {}; + if (bindIfName == nullptr || bindIfName[0] == '\0' || strlen(bindIfName) >= sizeof(bindIf.ifr_name)) { + OS_LOGE(TAG, "Invalid bind interface: %s", bindIfName != nullptr ? bindIfName : "(null)"); + return false; + } + memcpy(bindIf.ifr_name, bindIfName, strlen(bindIfName) + 1); + m_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if (m_socket < 0) { OS_LOGE(TAG, "Failed to create DNS socket"); @@ -71,8 +78,17 @@ bool DNSServer::start(const char* responseIpv4, const char* bindIpv4, uint16_t p return false; } - // Bound to one address rather than INADDR_ANY: lwIP then delivers only datagrams addressed to it, so the hub does - // not answer every name with the portal address to anyone on the LAN it is connected to. + // 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 = bindAddr; From 00c1b2162beb9f17a3f8af1453dcf19c7981e2d4 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Wed, 4 Nov 2026 00:08:54 +0100 Subject: [PATCH 08/10] fix(captive-portal): redirect probes when the www image is missing With no assets, every GET got the 200 help page, so a probe sent to the AP address passed the connectivity check and the OS never opened the portal to show it. Redirect known probe paths first, as HandleMiss does. --- .../src/captiveportal/StaticFileServer.cpp | 5 +++++ components/rfc8908/include/rfc8908/RFC8908Handler.h | 5 ++++- components/rfc8908/src/RFC8908Handler.cpp | 9 ++++++--- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/components/captive_portal/src/captiveportal/StaticFileServer.cpp b/components/captive_portal/src/captiveportal/StaticFileServer.cpp index ad51c891..5d5b8761 100644 --- a/components/captive_portal/src/captiveportal/StaticFileServer.cpp +++ b/components/captive_portal/src/captiveportal/StaticFileServer.cpp @@ -220,6 +220,11 @@ void CaptivePortal::StaticFileServer::registerHandler(httpd_handle_t server) 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, diff --git a/components/rfc8908/include/rfc8908/RFC8908Handler.h b/components/rfc8908/include/rfc8908/RFC8908Handler.h index 8e21e8c0..b5da4076 100644 --- a/components/rfc8908/include/rfc8908/RFC8908Handler.h +++ b/components/rfc8908/include/rfc8908/RFC8908Handler.h @@ -9,7 +9,7 @@ namespace OpenShock::RFC8908 { // 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. // - // Assumes the server only accepts connections from the portal AP; the portal refuses the rest when they open. + // Assumes the server only accepts connections from the portal AP; the portal binds it to the AP interface. // // `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); @@ -19,6 +19,9 @@ namespace OpenShock::RFC8908 { // 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); diff --git a/components/rfc8908/src/RFC8908Handler.cpp b/components/rfc8908/src/RFC8908Handler.cpp index 0b2c4531..f023add3 100644 --- a/components/rfc8908/src/RFC8908Handler.cpp +++ b/components/rfc8908/src/RFC8908Handler.cpp @@ -111,13 +111,16 @@ static bool IsProbePath(std::string_view path) return false; } -esp_err_t RFC8908::HandleMiss(httpd_req_t* req) +bool RFC8908::IsProbeRequest(httpd_req_t* req) { // Path only: httpd leaves the query string in req->uri std::string_view path(req->uri); - path = path.substr(0, path.find('?')); + return IsProbePath(path.substr(0, path.find('?'))); +} - if (IsForeignHost(req) || IsProbePath(path)) { +esp_err_t RFC8908::HandleMiss(httpd_req_t* req) +{ + if (IsForeignHost(req) || IsProbeRequest(req)) { return EmitRedirect(req); } From 5407d14f6bad67193b5020928f0d5706b8815ce7 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Wed, 4 Nov 2026 00:13:34 +0100 Subject: [PATCH 09/10] refactor(captive-portal): follow the ranges and string_view style of #559 - IsProbePath: std::ranges::any_of instead of a hand-written search loop - Interface names: std::string_view size and copy instead of strlen twice and memcpy - SendNotFound HEAD: a constexpr response instead of building a std::string at runtime, with asserts tying it to the GET response --- .../captiveportal/CaptivePortalInstance.cpp | 12 +++++----- components/dns_server/src/DNSServer.cpp | 10 ++++---- components/rfc8908/src/RFC8908Handler.cpp | 23 +++++++------------ 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp index 5cfa1b98..7a58b7c8 100644 --- a/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp +++ b/components/captive_portal/src/captiveportal/CaptivePortalInstance.cpp @@ -12,7 +12,7 @@ const char* const TAG = "CaptivePortalInstance"; #include #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 @@ -53,13 +53,13 @@ bool CaptivePortal::CaptivePortalInstance::startHttpServer() // 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 = {}; - const char* apIfName = CaptivePortal::ApInterfaceName(); - if (apIfName[0] == '\0' || strlen(apIfName) >= sizeof(apInterface.ifr_name)) { - OS_LOGE(TAG, "No usable AP interface name (\"%s\"), not starting the HTTP server", apIfName); + 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; } - memcpy(apInterface.ifr_name, apIfName, strlen(apIfName) + 1); + apIfName.copy(apInterface.ifr_name, apIfName.size()); httpd_config_t config = HTTPD_DEFAULT_CONFIG(); config.server_port = HTTP_PORT; diff --git a/components/dns_server/src/DNSServer.cpp b/components/dns_server/src/DNSServer.cpp index 923a27bd..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; @@ -52,12 +53,13 @@ bool DNSServer::start(const char* responseIpv4, const char* bindIpv4, const char return false; } - struct ifreq bindIf = {}; - if (bindIfName == nullptr || bindIfName[0] == '\0' || strlen(bindIfName) >= sizeof(bindIf.ifr_name)) { - OS_LOGE(TAG, "Invalid bind interface: %s", bindIfName != nullptr ? bindIfName : "(null)"); + 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; } - memcpy(bindIf.ifr_name, bindIfName, strlen(bindIfName) + 1); + ifNameView.copy(bindIf.ifr_name, ifNameView.size()); m_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if (m_socket < 0) { diff --git a/components/rfc8908/src/RFC8908Handler.cpp b/components/rfc8908/src/RFC8908Handler.cpp index f023add3..9bf102d7 100644 --- a/components/rfc8908/src/RFC8908Handler.cpp +++ b/components/rfc8908/src/RFC8908Handler.cpp @@ -9,6 +9,7 @@ const char* const TAG = "RFC8908Handler"; #include "http/ContentTypes.h" #include "json/Json.h" +#include #include #include #include @@ -54,17 +55,14 @@ static esp_err_t SendNotFound(httpd_req_t* req) return httpd_resp_send(req, kBody.data(), kBody.size()); } - std::string head; - head.reserve(96); - head += "HTTP/1.1 404 Not Found\r\nContent-Type: "; - head += HTTP::ContentType::TextPlain; - head += "\r\nContent-Length: "; - head += std::to_string(kBody.size()); - head += "\r\n\r\n"; + // 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 < head.size()) { - int n = httpd_send(req, head.data() + sent, head.size() - sent); + while (sent < kHead.size()) { + int n = httpd_send(req, kHead.data() + sent, kHead.size() - sent); if (n <= 0) { return ESP_FAIL; } @@ -103,12 +101,7 @@ static constexpr std::string_view kProbePaths[] = { static bool IsProbePath(std::string_view path) { - for (std::string_view probePath : kProbePaths) { - if (StringIEquals(path, probePath)) { - return true; - } - } - return false; + return std::ranges::any_of(kProbePaths, [path](std::string_view probePath) { return StringIEquals(path, probePath); }); } bool RFC8908::IsProbeRequest(httpd_req_t* req) From 406f6c8796139df5bafe25513de0ab7d817e7998 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Wed, 4 Nov 2026 00:59:57 +0100 Subject: [PATCH 10/10] fix(captive-portal): read the AP interface name after the AP has started lwIP only adds the AP netif in esp_wifi's default WIFI_EVENT_AP_START handler and removes it again when the AP stops. Reading its name before enabling AP mode could return the STA's name and bind the portal to the LAN, and caching it went stale once the AP restarted with a new netif number. Wait for the AP to start and read the name on every start. --- .../src/captiveportal/Manager.cpp | 54 +++++++++++++++---- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/components/captive_portal/src/captiveportal/Manager.cpp b/components/captive_portal/src/captiveportal/Manager.cpp index 04bcf2cc..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,8 +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; -// lwIP's name for s_apNetif, which the portal's servers bind to. Read once; the netif is never destroyed. -static char s_apIfName[8] = {}; +// 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"); @@ -141,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) { @@ -180,13 +192,15 @@ static bool captiveportal_start() esp_netif_dhcps_start(s_apNetif); } - if (s_apIfName[0] == '\0') { - esp_err_t nameErr = esp_netif_get_netif_impl_name(s_apNetif, s_apIfName); - if (nameErr != ESP_OK) { - s_apIfName[0] = '\0'; - OS_LOGE(TAG, "Failed to get the AP interface name: %s", esp_err_to_name(nameErr)); - return false; // the servers could not be bound to the AP, and must not listen on the LAN + // 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). @@ -201,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)); @@ -214,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; @@ -378,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; }