diff --git a/README.md b/README.md index 9878f84..0308be7 100644 --- a/README.md +++ b/README.md @@ -287,7 +287,9 @@ Both HTTP servers (`mcp --http` and `catalog webui`) refuse cross-origin browser with 403 — a page you visit cannot post writes to your SAP system through them. Requests without an `Origin` header (curl, native MCP clients), same-origin requests and loopback origins are always allowed; add others with `--cors-origin`, and require a token with -`--auth-token` / `--auth-token-env`. See [docs/cli-usage.md](docs/cli-usage.md#http-transport-and-access-control). +`--auth-token` / `--auth-token-env`. Origin checking cannot see DNS rebinding, where the +attacker controls the `Host` too — pass `--allowed-hosts` to refuse any `Host` other than +loopback, an IP literal and the address bound. See [docs/cli-usage.md](docs/cli-usage.md#http-transport-and-access-control). The web UI ([`flutter/erpl_catalog_kit`](flutter/erpl_catalog_kit), compiled and embedded straight into the `erpl-adt` binary — see [Building from source](#building-from-source)) is **read-only against the cache except for curation**: Search, Browse, Entity Detail, Lineage, and Driver Tree all query the same fast `catalog_*` MCP tools the CLI and AI agents use; the Curate screen is the only one that writes, via `catalog_annotate`. There's no build/sync button — `catalog webui` doesn't hold a live SAP connection, so building, exporting, and syncing stay CLI-only operations. The Sync Status screen shows past sync runs and cache health, and Feed Export surfaces the exact `erpl-adt catalog build --format ...` command to run for each format, rather than re-implementing either client-side. diff --git a/docs/cli-usage.md b/docs/cli-usage.md index db585ec..ee16528 100644 --- a/docs/cli-usage.md +++ b/docs/cli-usage.md @@ -245,6 +245,7 @@ matters: | Flag | Effect | |------|--------| | `--cors-origin ` | Comma-separated extra browser origins allowed to call `/mcp`. `*` allows every origin. | +| `--allowed-hosts ` | Comma-separated `Host` header values this server answers to. Passing it refuses any other `Host` with 403. `*` allows every host. | | `--auth-token ` | Require `Authorization: Bearer `; requests without it get 401 and run nothing. | | `--auth-token-env ` | Read that token from an environment variable instead of the command line. | @@ -256,6 +257,24 @@ the case where a page the developer merely visited could otherwise post writes t SAP system, and binding to `127.0.0.1` does not prevent it because the browser is already inside the loopback boundary. +Origin validation alone does not stop **DNS rebinding**, which is why `--allowed-hosts` +exists. Once a page at `evil.example` makes `rebind.evil.example` resolve to `127.0.0.1`, +the browser believes it is talking to its own origin: the request arrives with `Host: +rebind.evil.example` and either a matching `Origin` or none at all, and both of those +satisfy the rules above — the attacker controls each side of the comparison. The `Host` +header is the half they cannot launder. + +`--allowed-hosts` names the hosts this server answers to; loopback names, IP literals (an +IP address has no DNS name to rebind, so `--mcp-host 0.0.0.0` reached at a LAN address +keeps working) and the address bound are always allowed. **Passing the flag is what turns +refusal on.** Without it, an unrecognised `Host` is still served, with one warning per +distinct host on stderr — so a deployment reached through a DNS name or a reverse proxy +does not break on upgrade, and closing the hole is one flag: + +```bash +erpl-adt mcp --http --mcp-host 0.0.0.0 --allowed-hosts mcp.internal.example +``` + Authentication is off unless a token is configured. Binding beyond loopback without one warns on stderr; `/healthz` never requires the token so liveness probes keep working. diff --git a/include/erpl_adt/mcp/http_security.hpp b/include/erpl_adt/mcp/http_security.hpp index 8762ef5..fc34762 100644 --- a/include/erpl_adt/mcp/http_security.hpp +++ b/include/erpl_adt/mcp/http_security.hpp @@ -19,16 +19,30 @@ namespace erpl_adt { // Binding to 127.0.0.1 does not help there — the browser is inside the // loopback boundary. // +// Host validation stops the same browser being aimed here by DNS +// rebinding, which Origin validation cannot see: once evil.example makes +// rebind.evil.example resolve to 127.0.0.1, the browser treats the call as +// same-origin and sends either a matching Origin or none at all. Both of +// those satisfy the Origin rules, because the attacker controls each side +// of the comparison. The Host header is the half they cannot launder. +// // The bearer token stops everything else that can reach the port. // -// Both are permissive by default so that no existing deployment breaks: a -// request without an Origin header is not a browser and is allowed, and the -// token is only enforced once one is configured. +// All three are permissive by default so that no existing deployment breaks: +// a request without an Origin header is not a browser and is allowed, an +// unrecognised Host is served with a warning until --allowed-hosts opts in to +// refusing it, and the token is only enforced once one is configured. // --------------------------------------------------------------------------- struct HttpSecurityOptions { // Extra origins allowed beyond same-origin and loopback. The single // entry "*" restores the historical allow-everything behaviour. std::vector allowed_origins; + // Hosts allowed beyond loopback and IP literals — the bind host and + // anything named by --allowed-hosts. "*" allows every host. + std::vector allowed_hosts; + // Whether an unrecognised Host is refused (true) or served with a + // warning (false, the default). Set by passing --allowed-hosts. + bool enforce_hosts = false; // When non-empty, /mcp requires "Authorization: Bearer ". std::string auth_token; }; @@ -55,6 +69,27 @@ enum class OriginVerdict { const std::string& host, const HttpSecurityOptions& options); +// Why a request's Host was accepted or refused. +enum class HostVerdict { + NoHost, // no Host header — browsers always send one, so not a browser + Loopback, // localhost / 127.0.0.1 / [::1] + IpLiteral, // an IP address — there is no name to rebind + Allowlisted, // the bind host, or named by --allowed-hosts + Wildcard, // --allowed-hosts '*' + Unrecognised, // a DNS name we were not told about — the rebinding shape +}; + +[[nodiscard]] constexpr bool IsAllowed(HostVerdict verdict) { + return verdict != HostVerdict::Unrecognised; +} + +// Classify a Host header against the options. `host` is the raw header value +// ("name" or "name:port"); an empty one means the header was absent. Whether +// an Unrecognised host is actually refused is the caller's decision, via +// HttpSecurityOptions::enforce_hosts. +[[nodiscard]] HostVerdict ClassifyHost(const std::string& host, + const HttpSecurityOptions& options); + // True when `header` carries the configured bearer token. Comparison is // constant-time so a token cannot be recovered one byte at a time by timing // the response. An empty configured token means "no auth required" and every @@ -62,18 +97,25 @@ enum class OriginVerdict { [[nodiscard]] bool BearerTokenMatches(const std::string& authorization_header, const std::string& expected_token); -// Split a comma-separated --cors-origin value into individual origins, -// trimming whitespace and dropping empties. -[[nodiscard]] std::vector ParseOriginList(const std::string& value); +// Split a comma-separated flag value into pieces, trimming whitespace and +// dropping empties. +[[nodiscard]] std::vector ParseCommaList(const std::string& value); + +// Historical name, kept because --cors-origin is the older flag. +[[nodiscard]] inline std::vector ParseOriginList( + const std::string& value) { + return ParseCommaList(value); +} // Build the options from CLI flag values, shared by `mcp --http` and -// `catalog webui`. Warns on `err` about the two configurations worth -// noticing — a wildcard origin, and binding somewhere other than loopback -// without a token — but does not refuse either, so nothing that runs today -// stops running. Returns nullopt only on an unusable configuration (an +// `catalog webui`. Warns on `err` about the configurations worth noticing — +// a wildcard origin or host, and binding somewhere other than loopback +// without a token — but does not refuse any of them, so nothing that runs +// today stops running. Returns nullopt only on an unusable configuration (an // --auth-token-env naming a variable that is not set), having reported it. [[nodiscard]] std::optional ResolveHttpSecurity( const std::string& cors_origin_flag, + const std::string& allowed_hosts_flag, const std::string& auth_token_flag, const std::string& auth_token_env_flag, const std::string& bind_host, diff --git a/src/cli/command_executor.cpp b/src/cli/command_executor.cpp index d1d81fd..c6321e6 100644 --- a/src/cli/command_executor.cpp +++ b/src/cli/command_executor.cpp @@ -2900,6 +2900,7 @@ int HandleCatalogWebui(const CommandArgs& args) { RegisterCatalogStoreTools(registry, store); auto security = ResolveHttpSecurity(GetFlag(args, "cors-origin"), + GetFlag(args, "allowed-hosts"), GetFlag(args, "auth-token"), GetFlag(args, "auth-token-env"), host, std::cerr); @@ -7952,13 +7953,19 @@ void RegisterAllCommands(CommandRouter& router) { "having been run first, this serves an instructional message instead of the app.\n\n" "Access control: requests without an Origin header (curl, native clients), " "same-origin requests and loopback origins are allowed; any other browser " - "origin is refused with 403 unless named with --cors-origin. Binding beyond " - "127.0.0.1 without --auth-token exposes the catalog API — including the " - "curation writes of catalog_annotate — to everyone who can reach the port."; + "origin is refused with 403 unless named with --cors-origin. Origin alone " + "does not stop DNS rebinding, though: a page that points its own name at " + "127.0.0.1 arrives with a Host it controls and an Origin to match. Pass " + "--allowed-hosts to refuse any Host other than loopback, an IP literal and " + "the address bound; without it such a request is served with a warning. " + "Binding beyond 127.0.0.1 without --auth-token exposes the catalog API — " + "including the curation writes of catalog_annotate — to everyone who can " + "reach the port."; help.flags = { {"port", "", "Port to listen on (default: 8383)", false}, {"host", "", "Host/address to bind (default: 127.0.0.1)", false}, {"cors-origin", "", "Comma-separated extra origins allowed to call the API", false}, + {"allowed-hosts", "", "Host headers this server answers to; passing it refuses any other with 403", false}, {"auth-token", "", "Require 'Authorization: Bearer ' on the API", false}, {"auth-token-env", "", "Read that token from an environment variable", false}, }; diff --git a/src/main.cpp b/src/main.cpp index bd4c94d..3a62375 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -123,6 +123,12 @@ void PrintMcpHelp(std::ostream& out) { out << " Same-origin, loopback and non-browser (no Origin\n"; out << " header) requests are always allowed; anything else is\n"; out << " refused with 403. '*' allows every origin.\n"; + out << " --allowed-hosts Comma-separated Host header values this server answers\n"; + out << " to. Loopback, IP literals and the bound address are\n"; + out << " always allowed. Passing this refuses any other Host\n"; + out << " with 403 (DNS-rebinding defence); without it such a\n"; + out << " request is served with a warning. '*' allows every\n"; + out << " Host.\n"; out << " --auth-token Require 'Authorization: Bearer ' on /mcp\n"; out << " --auth-token-env Read that token from an environment variable\n"; } @@ -625,8 +631,9 @@ int HandleMcpServer(int argc, const char* const* argv) { } auto mcp_host = get("mcp-host", "127.0.0.1"); - auto security = ResolveHttpSecurity(get("cors-origin"), get("auth-token"), - get("auth-token-env"), mcp_host, std::cerr); + auto security = ResolveHttpSecurity(get("cors-origin"), get("allowed-hosts"), + get("auth-token"), get("auth-token-env"), + mcp_host, std::cerr); if (!security.has_value()) { return kExitInternal; } diff --git a/src/mcp/http_security.cpp b/src/mcp/http_security.cpp index 6278406..c03f698 100644 --- a/src/mcp/http_security.cpp +++ b/src/mcp/http_security.cpp @@ -59,6 +59,25 @@ bool IsLoopbackHost(const std::string& host) { host == "::1"; } +// Is this host an IP address rather than a name? Only names can be pointed at +// 127.0.0.1 by an attacker's DNS server, so an IP literal cannot carry a +// rebinding attack — which is what lets `--host 0.0.0.0` stay reachable at a +// LAN address with no configuration. +// +// Deliberately loose: a bracketed value is IPv6, and anything made only of +// digits and dots is IPv4. Both tests are cheap and neither can be satisfied +// by a registrable domain name, which is the only property that matters here. +bool IsIpLiteral(const std::string& host) { + if (host.size() >= 2 && host.front() == '[' && host.back() == ']') { + return true; + } + if (host.empty()) { + return false; + } + return host.find_first_not_of("0123456789.") == std::string::npos && + host.find('.') != std::string::npos; +} + } // namespace OriginVerdict ClassifyOrigin(const std::string& origin, const std::string& host, @@ -98,6 +117,43 @@ OriginVerdict ClassifyOrigin(const std::string& origin, const std::string& host, return OriginVerdict::Denied; } +HostVerdict ClassifyHost(const std::string& host, + const HttpSecurityOptions& options) { + const auto authority = Authority(host); + if (authority.empty()) { + // Browsers always send Host, so an absent one is not the shape this + // check exists for — it is an HTTP/1.0 or hand-rolled client. + return HostVerdict::NoHost; + } + + for (const auto& allowed : options.allowed_hosts) { + if (Trim(allowed) == "*") { + return HostVerdict::Wildcard; + } + } + + const auto name = HostOf(authority); + if (IsLoopbackHost(name)) { + return HostVerdict::Loopback; + } + if (IsIpLiteral(name)) { + return HostVerdict::IpLiteral; + } + + for (const auto& allowed : options.allowed_hosts) { + const auto entry = Authority(allowed); + // An entry naming a port is held to it; a bare name matches any port, + // because "the host I serve on" is what the operator meant to say. + const bool matches = (HostOf(entry) == entry) ? (entry == name) + : (entry == authority); + if (matches) { + return HostVerdict::Allowlisted; + } + } + + return HostVerdict::Unrecognised; +} + bool BearerTokenMatches(const std::string& authorization_header, const std::string& expected_token) { if (expected_token.empty()) { @@ -128,11 +184,22 @@ bool BearerTokenMatches(const std::string& authorization_header, } std::optional ResolveHttpSecurity( - const std::string& cors_origin_flag, const std::string& auth_token_flag, - const std::string& auth_token_env_flag, const std::string& bind_host, - std::ostream& err) { + const std::string& cors_origin_flag, const std::string& allowed_hosts_flag, + const std::string& auth_token_flag, const std::string& auth_token_env_flag, + const std::string& bind_host, std::ostream& err) { HttpSecurityOptions options; - options.allowed_origins = ParseOriginList(cors_origin_flag); + options.allowed_origins = ParseCommaList(cors_origin_flag); + options.allowed_hosts = ParseCommaList(allowed_hosts_flag); + // Passing the flag at all is what turns refusal on. Without it an + // unrecognised Host is served with a warning, so no deployment reached + // through a DNS name today stops working. + options.enforce_hosts = !options.allowed_hosts.empty(); + // The address the operator bound to is allowed without having to name it + // twice. Loopback and IP literals are already allowed by ClassifyHost. + const auto bind_name = HostOf(Authority(bind_host)); + if (!bind_name.empty() && !IsLoopbackHost(bind_name) && !IsIpLiteral(bind_name)) { + options.allowed_hosts.push_back(bind_name); + } options.auth_token = auth_token_flag; if (options.auth_token.empty() && !auth_token_env_flag.empty()) { @@ -153,6 +220,15 @@ std::optional ResolveHttpSecurity( } } + for (const auto& host : options.allowed_hosts) { + if (host == "*") { + err << "Warning: --allowed-hosts '*' accepts any Host header, which " + "leaves DNS rebinding open. Name the hosts you serve on " + "instead.\n"; + break; + } + } + if (options.auth_token.empty() && !IsLoopbackHost(HostOf(Authority(bind_host)))) { err << "Warning: binding " << bind_host << " exposes this server beyond this machine with no authentication. " @@ -162,7 +238,7 @@ std::optional ResolveHttpSecurity( return options; } -std::vector ParseOriginList(const std::string& value) { +std::vector ParseCommaList(const std::string& value) { std::vector out; size_t pos = 0; while (pos <= value.size()) { diff --git a/src/mcp/mcp_http_server.cpp b/src/mcp/mcp_http_server.cpp index ba5ee8a..616cb28 100644 --- a/src/mcp/mcp_http_server.cpp +++ b/src/mcp/mcp_http_server.cpp @@ -3,7 +3,9 @@ #include #include +#include #include +#include #include #include @@ -61,9 +63,29 @@ struct McpHttpServer::Impl { std::mutex mcp_mutex; HttpSecurityOptions security; + // Hosts already warned about, so a scripted attack cannot flood stderr + // with one line per request. Guarded by warned_hosts_mutex because the + // warning is emitted from the pre-routing hook, which runs on httplib's + // thread pool — before /mcp's own lock is taken. + std::set warned_hosts; + std::mutex warned_hosts_mutex; Impl(ToolRegistry registry, HttpSecurityOptions security_options) : mcp(std::move(registry)), security(std::move(security_options)) {} + + // Emit the DNS-rebinding warning once per distinct Host. + void WarnAboutHostOnce(const std::string& host) { + { + std::lock_guard lock(warned_hosts_mutex); + if (!warned_hosts.insert(host).second) { + return; + } + } + std::cerr << "Warning: served a request for Host '" << host + << "', which is not loopback, an IP literal, or a configured " + "host. A page using DNS rebinding looks exactly like this. " + "Pass --allowed-hosts to refuse it.\n"; + } }; McpHttpServer::McpHttpServer(ToolRegistry registry, bool serve_webui) @@ -85,9 +107,25 @@ McpHttpServer::McpHttpServer(ToolRegistry registry, bool serve_webui, // can't accidentally omit it. impl_->http.set_pre_routing_handler([this](const httplib::Request& req, httplib::Response& res) { + // Host first, and on its own: DNS rebinding hands the attacker both + // the Host and the Origin, so the same-origin rule below cannot see + // it. Checking Host first means that once enforcement is on, a + // rebound name is refused whatever Origin it claims. + const auto host = req.get_header_value("Host"); + const auto host_verdict = ClassifyHost(host, impl_->security); + if (!IsAllowed(host_verdict)) { + if (impl_->security.enforce_hosts) { + res.status = 403; + res.set_content( + R"({"error":"host not allowed; pass --allowed-hosts to allow it"})", + "application/json"); + return httplib::Server::HandlerResponse::Handled; + } + impl_->WarnAboutHostOnce(host); + } + const auto origin = req.get_header_value("Origin"); - const auto verdict = - ClassifyOrigin(origin, req.get_header_value("Host"), impl_->security); + const auto verdict = ClassifyOrigin(origin, host, impl_->security); if (!IsAllowed(verdict)) { res.status = 403; diff --git a/test/mcp/test_http_security.cpp b/test/mcp/test_http_security.cpp index 8fe32ad..13eaf92 100644 --- a/test/mcp/test_http_security.cpp +++ b/test/mcp/test_http_security.cpp @@ -2,6 +2,9 @@ #include +#include +#include + using namespace erpl_adt; // =========================================================================== @@ -142,3 +145,128 @@ TEST_CASE("ParseOriginList: empty pieces are dropped", "[mcp][security]") { REQUIRE(parsed.size() == 1); CHECK(parsed[0] == "https://a.example"); } + +// =========================================================================== +// Host classification +// +// Origin validation alone does not stop DNS rebinding. Once a page at +// evil.example makes rebind.evil.example resolve to 127.0.0.1, the browser +// considers the request same-origin and sends Host: rebind.evil.example with +// either a matching Origin or none at all — and both of those are allowed by +// the Origin rules above. The Host header is the half the attacker cannot +// launder, so it is checked on its own. +// +// An IP literal is a deliberate pass: there is no name to rebind, which is +// what keeps `catalog webui --host 0.0.0.0` reachable at a LAN address with +// no configuration. +// =========================================================================== + +TEST_CASE("ClassifyHost: loopback names are recognised", "[mcp][security]") { + HttpSecurityOptions options; + CHECK(ClassifyHost("localhost:8383", options) == HostVerdict::Loopback); + CHECK(ClassifyHost("127.0.0.1:8383", options) == HostVerdict::Loopback); + CHECK(ClassifyHost("[::1]:8383", options) == HostVerdict::Loopback); + CHECK(ClassifyHost("LocalHost", options) == HostVerdict::Loopback); +} + +TEST_CASE("ClassifyHost: an IP literal cannot be rebound", "[mcp][security]") { + HttpSecurityOptions options; + CHECK(ClassifyHost("192.168.1.5:8383", options) == HostVerdict::IpLiteral); + CHECK(ClassifyHost("10.0.0.7", options) == HostVerdict::IpLiteral); + CHECK(ClassifyHost("[fe80::1]:8383", options) == HostVerdict::IpLiteral); +} + +TEST_CASE("ClassifyHost: an unknown name is the rebinding shape", + "[mcp][security]") { + HttpSecurityOptions options; + CHECK(ClassifyHost("rebind.evil.example:8383", options) == + HostVerdict::Unrecognised); + // A name that merely contains a loopback label is still a name. + CHECK(ClassifyHost("localhost.evil.example", options) == + HostVerdict::Unrecognised); + CHECK(ClassifyHost("127.0.0.1.evil.example", options) == + HostVerdict::Unrecognised); +} + +TEST_CASE("ClassifyHost: a configured host is allowed", "[mcp][security]") { + HttpSecurityOptions options; + options.allowed_hosts = {"mcp.internal.example"}; + CHECK(ClassifyHost("mcp.internal.example:8383", options) == + HostVerdict::Allowlisted); + CHECK(ClassifyHost("mcp.internal.example", options) == HostVerdict::Allowlisted); + CHECK(ClassifyHost("other.internal.example", options) == HostVerdict::Unrecognised); +} + +TEST_CASE("ClassifyHost: a configured host may name a port", "[mcp][security]") { + // "host:port" and a bare host both mean the same thing to a user; only + // the port-qualified form is picky, and then only about that port. + HttpSecurityOptions options; + options.allowed_hosts = {"mcp.internal.example:8383"}; + CHECK(ClassifyHost("mcp.internal.example:8383", options) == + HostVerdict::Allowlisted); + CHECK(ClassifyHost("mcp.internal.example:9999", options) == + HostVerdict::Unrecognised); +} + +TEST_CASE("ClassifyHost: '*' allows any host", "[mcp][security]") { + HttpSecurityOptions options; + options.allowed_hosts = {"*"}; + CHECK(ClassifyHost("rebind.evil.example", options) == HostVerdict::Wildcard); +} + +TEST_CASE("ClassifyHost: a missing Host header is not a browser", + "[mcp][security]") { + // Browsers always send Host, so an absent one cannot be the rebinding + // shape — it is an HTTP/1.0 client or a hand-rolled request. + HttpSecurityOptions options; + CHECK(ClassifyHost("", options) == HostVerdict::NoHost); + CHECK(ClassifyHost(" ", options) == HostVerdict::NoHost); +} + +TEST_CASE("ClassifyHost: only Unrecognised is ever refused", "[mcp][security]") { + CHECK(IsAllowed(HostVerdict::NoHost)); + CHECK(IsAllowed(HostVerdict::Loopback)); + CHECK(IsAllowed(HostVerdict::IpLiteral)); + CHECK(IsAllowed(HostVerdict::Allowlisted)); + CHECK(IsAllowed(HostVerdict::Wildcard)); + CHECK(!IsAllowed(HostVerdict::Unrecognised)); +} + +// =========================================================================== +// ResolveHttpSecurity — the flag-to-options mapping +// =========================================================================== + +TEST_CASE("ResolveHttpSecurity: enforcement is off until --allowed-hosts", + "[mcp][security]") { + // The default has to stay warn-only: every deployment reached by a DNS + // name today keeps working, and enforcement is one flag away. + std::ostringstream err; + auto options = ResolveHttpSecurity("", "", "", "", "127.0.0.1", err); + REQUIRE(options.has_value()); + CHECK(!options->enforce_hosts); + + std::ostringstream err2; + auto enforcing = + ResolveHttpSecurity("", "mcp.internal.example", "", "", "0.0.0.0", err2); + REQUIRE(enforcing.has_value()); + CHECK(enforcing->enforce_hosts); + CHECK(ClassifyHost("mcp.internal.example", *enforcing) == + HostVerdict::Allowlisted); +} + +TEST_CASE("ResolveHttpSecurity: the bind host is allowed without naming it twice", + "[mcp][security]") { + std::ostringstream err; + auto options = ResolveHttpSecurity("", "other.example", "", "", + "mcp.internal.example", err); + REQUIRE(options.has_value()); + CHECK(ClassifyHost("mcp.internal.example:8383", *options) == + HostVerdict::Allowlisted); +} + +TEST_CASE("ResolveHttpSecurity: --allowed-hosts '*' warns", "[mcp][security]") { + std::ostringstream err; + auto options = ResolveHttpSecurity("", "*", "", "", "0.0.0.0", err); + REQUIRE(options.has_value()); + CHECK(err.str().find("--allowed-hosts") != std::string::npos); +} diff --git a/test/mcp/test_mcp_http_server.cpp b/test/mcp/test_mcp_http_server.cpp index b20fdaa..7b3a4b5 100644 --- a/test/mcp/test_mcp_http_server.cpp +++ b/test/mcp/test_mcp_http_server.cpp @@ -466,3 +466,119 @@ TEST_CASE("McpHttpServer: the web UI catch-all does not swallow GET /mcp", server.Stop(); server_thread.join(); } + +// =========================================================================== +// Host validation (DNS rebinding) +// =========================================================================== + +namespace { + +// The message body used by the Host tests below — a tool call, so a request +// that is wrongly let through is visible as an executed tool. +std::string EchoCall() { + return nlohmann::json{{"jsonrpc", "2.0"}, + {"id", 1}, + {"method", "tools/call"}, + {"params", {{"name", "echo"}, + {"arguments", {{"text", "hi"}}}}}} + .dump(); +} + +} // anonymous namespace + +TEST_CASE("McpHttpServer: an unknown Host is served by default", + "[mcp][http][security]") { + // Warn-only until the operator opts in: a deployment reached through a + // DNS name or a reverse proxy keeps working with no new flag. + McpHttpServer server(MakeEchoRegistry()); + auto port = static_cast(TestPort() + 40); + + std::thread server_thread([&] { (void)server.Run("127.0.0.1", port); }); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + httplib::Client client("127.0.0.1", port); + const httplib::Headers rebound{{"Host", "rebind.evil.example"}}; + auto response = client.Post("/mcp", rebound, EchoCall(), "application/json"); + REQUIRE(response != nullptr); + CHECK(response->status == 200); + + server.Stop(); + server_thread.join(); +} + +TEST_CASE("McpHttpServer: with --allowed-hosts, a rebound Host is refused", + "[mcp][http][security]") { + // The DNS-rebinding shape: evil.example points rebind.evil.example at + // 127.0.0.1, so the browser calls this server believing it is same-origin + // and sends an Origin the Origin check cannot fault. Host is the half the + // attacker cannot launder. + HttpSecurityOptions security; + security.allowed_hosts = {"mcp.internal.example"}; + security.enforce_hosts = true; + + McpHttpServer server(MakeEchoRegistry(), false, security); + auto port = static_cast(TestPort() + 41); + + std::thread server_thread([&] { (void)server.Run("127.0.0.1", port); }); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + httplib::Client client("127.0.0.1", port); + + const httplib::Headers rebound{ + {"Host", "rebind.evil.example"}, + {"Origin", "http://rebind.evil.example"}}; + auto refused = client.Post("/mcp", rebound, EchoCall(), "application/json"); + REQUIRE(refused != nullptr); + CHECK(refused->status == 403); + CHECK(refused->get_header_value("Access-Control-Allow-Origin").empty()); + CHECK(refused->body.find("hi") == std::string::npos); // the tool did not run + + // Same request with no Origin at all — the other half of the shape, and + // the one an Origin-only check waves straight through. + const httplib::Headers rebound_no_origin{{"Host", "rebind.evil.example"}}; + auto refused_silent = + client.Post("/mcp", rebound_no_origin, EchoCall(), "application/json"); + REQUIRE(refused_silent != nullptr); + CHECK(refused_silent->status == 403); + + // Loopback and the configured name still work. + const httplib::Headers loopback{{"Host", "127.0.0.1"}}; + auto allowed = client.Post("/mcp", loopback, EchoCall(), "application/json"); + REQUIRE(allowed != nullptr); + CHECK(allowed->status == 200); + + const httplib::Headers configured{{"Host", "mcp.internal.example"}}; + auto named = client.Post("/mcp", configured, EchoCall(), "application/json"); + REQUIRE(named != nullptr); + CHECK(named->status == 200); + + server.Stop(); + server_thread.join(); +} + +TEST_CASE("McpHttpServer: enforcing hosts leaves the web UI's own origin alone", + "[mcp][http][security]") { + // The embedded UI is reached at whatever address the browser used, and + // its POSTs are same-origin. Enforcement must not cost it anything. + HttpSecurityOptions security; + security.allowed_hosts = {"catalog.internal"}; + security.enforce_hosts = true; + + McpHttpServer server(MakeEchoRegistry(), false, security); + auto port = static_cast(TestPort() + 42); + + std::thread server_thread([&] { (void)server.Run("127.0.0.1", port); }); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + httplib::Client client("127.0.0.1", port); + const httplib::Headers same_origin{{"Host", "catalog.internal"}, + {"Origin", "http://catalog.internal"}}; + auto response = client.Post("/mcp", same_origin, EchoCall(), "application/json"); + REQUIRE(response != nullptr); + CHECK(response->status == 200); + CHECK(response->get_header_value("Access-Control-Allow-Origin") == + "http://catalog.internal"); + + server.Stop(); + server_thread.join(); +}