feat: add automatic DNS rebinding protection for localhost servers#760
Draft
pcarleton wants to merge 2 commits intomodelcontextprotocol:mainfrom
Draft
feat: add automatic DNS rebinding protection for localhost servers#760pcarleton wants to merge 2 commits intomodelcontextprotocol:mainfrom
pcarleton wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
Add DNS rebinding protection that is automatically enabled when requests arrive via localhost (127.0.0.1, [::1]). This protects against malicious websites using DNS rebinding to interact with local MCP servers. Key changes: - Add DisableLocalhostProtection option to StreamableHTTPOptions - Add isLocalhostAddr and isLocalhostHost helper functions - Validate Host header at start of ServeHTTP, rejecting non-localhost Host headers with 403 Forbidden when the connection arrives via localhost The protection is enabled by default with no code changes required. Users can opt-out by setting DisableLocalhostProtection: true. This uses http.LocalAddrContextKey to detect the connection's local address, which means protection is enabled for any request arriving via localhost, regardless of whether the server listens on 127.0.0.1 or 0.0.0.0. Closes: relates to MCP spec security best practices See: https://modelcontextprotocol.io/specification/2025-11-25/basic/security_best_practices#local-mcp-server-compromise
maciej-kisiel
requested changes
Jan 27, 2026
Contributor
maciej-kisiel
left a comment
maciej-kisiel
left a comment
There was a problem hiding this comment.
Thanks for the PR! I added a few comments, but generally the approach looks sound.
I believe we should also add this protection to the legacy SSE transport.
| // DNS rebinding protection: auto-enabled for localhost servers. | ||
| // See: https://modelcontextprotocol.io/specification/2025-11-25/basic/security_best_practices#local-mcp-server-compromise | ||
| if !h.opts.DisableLocalhostProtection { | ||
| if localAddr, ok := req.Context().Value(http.LocalAddrContextKey).(net.Addr); ok { |
Contributor
There was a problem hiding this comment.
With this change below I think we could only have a single implementation of the isLocalhost function, since they are largely the same. It would be called with localAddr.String() and req.Host.
Suggested change
| if localAddr, ok := req.Context().Value(http.LocalAddrContextKey).(net.Addr); ok { | |
| if localAddr := req.Context().Value(http.LocalAddrContextKey).(net.Addr); localAddr != nil { |
| disableProt bool // DisableLocalhostProtection setting | ||
| wantStatus int | ||
| }{ | ||
| // Auto-enabled for localhost listeners (127.0.0.1) |
Contributor
There was a problem hiding this comment.
Extending the test cases to multiple lines and including field names is more readable. I would suggest to do it.
| name string | ||
| listenAddr string // Address to listen on | ||
| hostHeader string // Host header in request | ||
| disableProt bool // DisableLocalhostProtection setting |
Contributor
There was a problem hiding this comment.
disableProtection should be fine.
| } | ||
| handler := NewStreamableHTTPHandler(func(req *http.Request) *Server { return server }, opts) | ||
|
|
||
| // Create a listener on the specified address to control LocalAddrContextKey |
Contributor
There was a problem hiding this comment.
Suggested change
| // Create a listener on the specified address to control LocalAddrContextKey | |
| // Create a listener on the specified address to control LocalAddrContextKey. |
| } | ||
| defer listener.Close() | ||
|
|
||
| // Start server in background |
Contributor
There was a problem hiding this comment.
Suggested change
| // Start server in background | |
| // Start the server in the background. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add DNS rebinding protection that is automatically enabled when requests arrive via localhost (127.0.0.1, [::1]). This protects against malicious websites using DNS rebinding to interact with local MCP servers.
(note: this was a claude assisted PR, mostly wanted to see how difficult passing this test would be for this SDK)
Design Goal: Secure by Default
The primary goal is to make it difficult to run a localhost server without these protections by mistake. There are other approaches that could provide secure defaults (e.g., a helper for
ListenAndServethat does the localhost check), but usinghttp.LocalAddrContextKeyfor runtime detection seemed like the most backwards compatible approach and least likely to be disabled by accident.With this implementation:
DisableLocalhostProtection: trueChanges
DisableLocalhostProtectionoption toStreamableHTTPOptionsisLocalhostAddrandisLocalhostHosthelper functionsServeHTTP, rejecting non-localhost Host headers with 403 ForbiddenHow it works
The protection uses
http.LocalAddrContextKeyto detect the connection's local address at runtime. When a request arrives via localhost (127.0.0.1 or [::1]), the handler validates that the Host header also matches a localhost value. If not, the request is rejected with 403 Forbidden.This approach means:
127.0.0.1or0.0.0.0Edge case: Reverse proxies
If a reverse proxy (e.g., Envoy, nginx) runs on the same host and forwards requests to the MCP server via localhost while preserving the original Host header, those requests would be rejected. In this case, users should either:
DisableLocalhostProtection: trueTesting
isLocalhostAddrandisLocalhostHosthelper functionslocalhost-host-rebinding-rejected: PASSlocalhost-host-valid-accepted: PASSRelated
localhostHostValidation()middlewaredns-rebinding-protectionscenario