fix(fetch): add configurable SSL certificate verification#3179
Open
Tomo1912 wants to merge 2 commits into
Open
fix(fetch): add configurable SSL certificate verification#3179Tomo1912 wants to merge 2 commits into
Tomo1912 wants to merge 2 commits into
Conversation
12 tasks
Tomo1912
force-pushed
the
fix/ssl-verification-508
branch
4 times, most recently
from
c8d5751 to
99644a7
Compare
Tomo1912
force-pushed
the
fix/ssl-verification-508
branch
from
e0d7f08 to
dc626ba
Compare
Tomo1912
force-pushed
the
fix/ssl-verification-508
branch
from
0576c9a to
eea22fc
Compare
Tomo1912
force-pushed
the
fix/ssl-verification-508
branch
2 times, most recently
from
4c72451 to
c938f5b
Compare
cliffhall
added
bug
This comment was marked as abuse.
This comment was marked as abuse.
This PR adds Server-Side Request Forgery (SSRF) protection and a comprehensive security test suite to the fetch MCP server. - URL scheme validation (only http/https allowed) - Private IP range blocking (10.x, 172.16-31.x, 192.168.x, 127.x, etc.) - IPv6 private address blocking (::1, fe80::, fc00::, etc.) - Dangerous hostname blocking (localhost, metadata services, etc.) - DNS resolution validation to prevent DNS rebinding - Configurable via MCP_FETCH_ALLOW_PRIVATE_IPS env var - Whitelist support via MCP_FETCH_ALLOWED_PRIVATE_HOSTS - Configurable SSL verification via MCP_FETCH_SSL_VERIFY env var - Comprehensive SSL error handling with helpful messages - SSRF protection tests - Private IP blocking tests - Input validation tests - URL scheme validation tests - Integration tests - Edge case tests ```bash export MCP_FETCH_SSL_VERIFY=false export MCP_FETCH_ALLOW_PRIVATE_IPS=true export MCP_FETCH_ALLOWED_PRIVATE_HOSTS=internal.company.com,api.local ``` fix: address security review feedback - Disable follow_redirects to prevent SSRF bypass via open redirects - Add explicit IP obfuscation detection (decimal/octal/hex formats) - Fix SSL parsing to be fail-secure (only 'false' disables verification) - Clean up test headers (remove enterprise roleplay language) - Add comprehensive tests for IP obfuscation parsing fix: add octal integer IP parsing and fix test naming - Add octal integer format parsing (017700000001 = 127.0.0.1) - Rename SSL test to reflect fail-secure behavior (stays_enabled, not defaults_to_false) - Add tests for octal integer IP obfuscation
Address review feedback on SSRF protection: - Add SSRFSafeTransport custom async transport that resolves DNS, validates the resolved IP, and replaces the hostname with the validated IP before connecting. This eliminates the TOCTOU window between validate_url_for_ssrf() and the actual HTTP request. - Integrate SSRFSafeTransport into fetch_url() and check_may_autonomously_fetch_url() replacing direct AsyncClient usage. - Add 6 DNS rebinding tests including full attack scenario simulation. - Update existing tests to match new transport-based architecture.
Tomo1912
force-pushed
the
fix/ssl-verification-508
branch
from
baab765 to
9db0910
Compare
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
3 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.
Resolves #508
Description
Added
MCP_FETCH_SSL_VERIFYenvironment variable to control SSL certificate verification. This allows the fetch server to work with internal servers that use self-signed certificates.Changes
SSL_VERIFYconfiguration viaMCP_FETCH_SSL_VERIFYenv var (default:true)verify=SSL_VERIFYto bothAsyncClientinstancesMCP_FETCH_SSL_VERIFY=falsefor self-signed certsUsage
export MCP_FETCH_SSL_VERIFY=falseServer Details
fetchMotivation and Context
Users with internal servers using self-signed certificates cannot use the fetch server because SSL verification fails. This change allows users to optionally disable SSL verification while keeping it enabled by default for security.
How Has This Been Tested?
SSL_VERIFYenv var parsing works correctlyBreaking Changes
None. SSL verification remains enabled by default.
Types of changes
Checklist
Additional context
A follow-up PR with SSRF protection and comprehensive security tests is available at #3180.