fix: validate Origin against localhost by default in DNS rebinding protection (#840)
Default the `Origin` allowlist to localhost when `DnsRebindingProtection` relies on the localhost host defaults, so a request with a valid `Host` but a hostile (or `null`) `Origin` is no longer accepted. ## How Has This Been Tested? unit tests ## Breaking Changes Behavioral change only ## Types of changes - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [x] I have added or updated documentation as needed
This commit is contained in:
parent
4103b87d69
commit
cd91652be1
3 changed files with 72 additions and 5 deletions
|
|
@ -19,6 +19,16 @@ import io.modelcontextprotocol.kotlin.sdk.types.RPCError
|
|||
*/
|
||||
internal val LOCALHOST_ALLOWED_HOSTS: List<String> = listOf("localhost", "127.0.0.1", "[::1]")
|
||||
|
||||
/**
|
||||
* Default list of `Origin` values allowed for localhost DNS rebinding protection.
|
||||
*
|
||||
* Mirrors [LOCALHOST_ALLOWED_HOSTS] but carries a scheme so the values parse as URLs:
|
||||
* [extractOriginHost] rejects schemeless input. Comparison is hostname-only and
|
||||
* scheme-agnostic, so these entries also match `https://` origins on the same host.
|
||||
*/
|
||||
internal val LOCALHOST_ALLOWED_ORIGINS: List<String> =
|
||||
listOf("http://localhost", "http://127.0.0.1", "http://[::1]")
|
||||
|
||||
/**
|
||||
* Characters that are valid in a URL but must not appear in an HTTP `Host` header.
|
||||
* Rejecting them prevents the parser from accepting malformed values
|
||||
|
|
|
|||
|
|
@ -41,7 +41,9 @@ private val logger = KotlinLogging.logger {}
|
|||
* @param allowedHosts hostnames allowed in the `Host` header. Defaults to `localhost`, `127.0.0.1`, `[::1]`.
|
||||
* @param allowedOrigins origins allowed in the `Origin` header, compared by hostname only
|
||||
* (scheme and port are ignored). Requests without an `Origin` header are allowed.
|
||||
* Pass `null` to skip origin validation.
|
||||
* When `null` while the localhost host defaults are in effect (no custom `allowedHosts`),
|
||||
* the `Origin` header is validated against `localhost`, `127.0.0.1`, `[::1]`.
|
||||
* With custom `allowedHosts`, `null` skips origin validation.
|
||||
* @param block factory block with access to the [ServerSSESession]
|
||||
* that creates and returns the [Server] to handle the connection.
|
||||
* @throws IllegalStateException if the [SSE] plugin is not installed.
|
||||
|
|
@ -71,7 +73,9 @@ public fun Route.mcp(
|
|||
* @param allowedHosts hostnames allowed in the `Host` header. Defaults to `localhost`, `127.0.0.1`, `[::1]`.
|
||||
* @param allowedOrigins origins allowed in the `Origin` header, compared by hostname only
|
||||
* (scheme and port are ignored). Requests without an `Origin` header are allowed.
|
||||
* Pass `null` to skip origin validation.
|
||||
* When `null` while the localhost host defaults are in effect (no custom `allowedHosts`),
|
||||
* the `Origin` header is validated against `localhost`, `127.0.0.1`, `[::1]`.
|
||||
* With custom `allowedHosts`, `null` skips origin validation.
|
||||
* @param block factory block with access to the [ServerSSESession]
|
||||
* that creates and returns the [Server] to handle the connection.
|
||||
* @throws IllegalStateException if the [SSE] plugin is not installed.
|
||||
|
|
@ -119,7 +123,9 @@ public fun Route.mcp(
|
|||
* @param allowedHosts hostnames allowed in the `Host` header. Defaults to `localhost`, `127.0.0.1`, `[::1]`.
|
||||
* @param allowedOrigins origins allowed in the `Origin` header, compared by hostname only
|
||||
* (scheme and port are ignored). Requests without an `Origin` header are allowed.
|
||||
* Pass `null` to skip origin validation.
|
||||
* When `null` while the localhost host defaults are in effect (no custom `allowedHosts`),
|
||||
* the `Origin` header is validated against `localhost`, `127.0.0.1`, `[::1]`.
|
||||
* With custom `allowedHosts`, `null` skips origin validation.
|
||||
* @param block factory block with access to the [ServerSSESession]
|
||||
* that creates and returns the [Server] to handle the connection.
|
||||
*/
|
||||
|
|
@ -205,7 +211,9 @@ private fun Application.mcpStreamableHttp(
|
|||
* If `null` and DNS rebinding protection is enabled, defaults to `localhost`, `127.0.0.1`, `[::1]`.
|
||||
* @param allowedOrigins A list of allowed `Origin` header values, compared by hostname only
|
||||
* (scheme and port are ignored). Requests without an `Origin` header are allowed.
|
||||
* If `null`, origin validation is disabled.
|
||||
* When `null` while the localhost host defaults are in effect (no custom `allowedHosts`),
|
||||
* the `Origin` header is validated against `localhost`, `127.0.0.1`, `[::1]`.
|
||||
* With custom `allowedHosts`, `null` skips origin validation.
|
||||
* @param eventStore An optional [EventStore] instance to enable resumable event stream functionality.
|
||||
* Allows storing and replaying events.
|
||||
* @param block factory block with access to the [RoutingContext] (for reading request headers)
|
||||
|
|
@ -451,6 +459,9 @@ private fun Route.installDnsRebindingProtection(enabled: Boolean, hosts: List<St
|
|||
if (!enabled) return
|
||||
install(DnsRebindingProtection) {
|
||||
allowedHosts = hosts ?: LOCALHOST_ALLOWED_HOSTS
|
||||
origins?.let { allowedOrigins = it }
|
||||
// Secure-by-default: when relying on the localhost host defaults, validate the Origin
|
||||
// header against localhost too, so a request with a valid Host but a hostile Origin
|
||||
// (e.g. a DNS-rebinding page) is rejected. Callers with custom hosts opt in explicitly.
|
||||
allowedOrigins = origins ?: LOCALHOST_ALLOWED_ORIGINS.takeIf { hosts == null }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ package io.modelcontextprotocol.kotlin.sdk.server
|
|||
import io.kotest.assertions.ktor.client.shouldHaveStatus
|
||||
import io.kotest.assertions.throwables.shouldThrow
|
||||
import io.kotest.matchers.shouldBe
|
||||
import io.kotest.matchers.shouldNotBe
|
||||
import io.kotest.matchers.string.shouldContain
|
||||
import io.kotest.matchers.string.shouldNotContain
|
||||
import io.ktor.client.request.header
|
||||
|
|
@ -347,6 +348,51 @@ class DnsRebindingProtectionTest {
|
|||
response.shouldHaveStatus(HttpStatusCode.Forbidden)
|
||||
}
|
||||
|
||||
// -- default Origin validation (secure-by-default for localhost) --
|
||||
|
||||
@Test
|
||||
fun `mcpStreamableHttp rejects hostile Origin by default`() = testApplication {
|
||||
application {
|
||||
mcpStreamableHttp { testServer() }
|
||||
}
|
||||
|
||||
val response = client.post("/mcp") {
|
||||
header(HttpHeaders.Host, "localhost")
|
||||
header(HttpHeaders.Origin, "http://evil.com")
|
||||
contentType(ContentType.Application.Json)
|
||||
}
|
||||
response.shouldHaveStatus(HttpStatusCode.Forbidden)
|
||||
response.bodyAsText() shouldContain "Invalid Origin host: evil.com"
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `mcpStreamableHttp allows localhost Origin by default`() = testApplication {
|
||||
application {
|
||||
mcpStreamableHttp { testServer() }
|
||||
}
|
||||
|
||||
val response = client.post("/mcp") {
|
||||
header(HttpHeaders.Host, "localhost")
|
||||
header(HttpHeaders.Origin, "http://localhost:5173")
|
||||
contentType(ContentType.Application.Json)
|
||||
}
|
||||
response.status shouldNotBe HttpStatusCode.Forbidden
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `mcpStreamableHttp with custom allowedHosts does not auto-validate Origin`() = testApplication {
|
||||
application {
|
||||
mcpStreamableHttp(allowedHosts = listOf("myapp.com")) { testServer() }
|
||||
}
|
||||
|
||||
val response = client.post("/mcp") {
|
||||
header(HttpHeaders.Host, "myapp.com")
|
||||
header(HttpHeaders.Origin, "http://evil.com")
|
||||
contentType(ContentType.Application.Json)
|
||||
}
|
||||
response.status shouldNotBe HttpStatusCode.Forbidden
|
||||
}
|
||||
|
||||
// -- extractHostname unit tests --
|
||||
|
||||
@ParameterizedTest
|
||||
|
|
|
|||
Loading…
Reference in a new issue