Skip to content

chore: strengthen SSRF proxy default configuration #24393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

chore: strengthen SSRF proxy default configuration #24393

wants to merge 13 commits into from
+1,650 −47

Conversation

laipz8200
Copy link
Member

@laipz8200 laipz8200 commented Aug 23, 2025 •
edited
Loading

Summary

This PR implements a "secure by default" configuration for the SSRF proxy to address security vulnerability reports. The current configuration is too permissive, allowing access to internal networks and high port ranges that can be exploited for SSRF attacks.

Changes Made:

  • Block all private/internal networks (RFC 1918, loopback, link-local, multicast, etc.)
  • Restrict to HTTP/HTTPS only - removed FTP, Gopher, and high port ranges (1025-65535)
  • Deny-all-by-default policy - requires explicit whitelisting via config files
  • Added customization support - users can override via /etc/squid/conf.d/ configs
  • Comprehensive documentation - README and example configurations provided

Security Improvements:

Before After
Allows ports 1025-65535 Only ports 80 and 443
Allows access to localhost Blocks all private networks
Default allows marketplace.dify.ai No default allowlists
Permissive by default Deny-all by default
ssrf_test ssrf_result

User Impact:

  • Production: More secure by default, prevents SSRF attacks
  • Development: Can easily customize via config overrides
  • Migration: Existing deployments may need to add custom configs for specific domains

Fixes #24392

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. 💪 enhancement New feature or request 📚 documentation Improvements or additions to documentation labels Aug 23, 2025
@laipz8200 laipz8200 marked this pull request as draft August 23, 2025 19:56
@laipz8200 laipz8200 requested a review from Yeuoly August 23, 2025 21:36
@laipz8200 laipz8200 marked this pull request as ready for review August 23, 2025 21:36
@DavideDelbianco
Copy link
Contributor

DavideDelbianco commented Aug 24, 2025 •
edited
Loading

Will this new configuration block MCP server functionality where google.com is used to retrieve favicon?
Or Github to retrieve latest version / stars count / manual install of plugin from repositories ?

@laipz8200
Copy link
Member Author

@DavideDelbianco Our goal is to block all access to the internal network environment by default-without blocking access to internet resources. Before this PR is merged, we encourage you to try the new configuration and report any issues you encounter.

crazywoola
crazywoola previously approved these changes Aug 24, 2025
View reviewed changes
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 24, 2025
@laipz8200 laipz8200 force-pushed the chore/ssrf-config branch 2 times, most recently from e09adc6 to 3e77099 Compare August 25, 2025 15:45
Yeuoly
Yeuoly reviewed Aug 27, 2025
View reviewed changes
@crazywoola crazywoola mentioned this pull request Aug 29, 2025
Closed
5 tasks
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Sep 1, 2025
laipz8200 and others added 10 commits September 1, 2025 13:45
@laipz8200
chore: strengthen SSRF proxy default configuration
23c97ec 
- Block all private/internal networks by default to prevent SSRF attacks
- Restrict allowed ports to only HTTP (80) and HTTPS (443)
- Remove default domain allowlists (e.g., marketplace.dify.ai)
- Implement deny-all-by-default policy with explicit whitelisting
- Add example configuration files for common customization scenarios
- Provide comprehensive documentation for security configuration

Fixes #24392
@autofix-ci @laipz8200
[autofix.ci] apply automated fixes
9e2b632
@laipz8200
chore: harden SSRF proxy configuration with strict defaults
1a49feb 
- Block all private/internal networks by default to prevent SSRF attacks
- Restrict ports to only HTTP (80) and HTTPS (443)
- Deny all requests by default unless explicitly whitelisted
- Add customization support via conf.d directory for local overrides
- Provide example configurations for common use cases
- Add CI/testing setup script to ensure tests pass with strict config
- Update docker-compose files to support custom config mounting
- Add comprehensive documentation with security warnings
@laipz8200
chore: update docker compose tempalte
99ee64c 
Signed-off-by: -LAN- <[email protected]>
@laipz8200
chore: allow marketplace access by default in SSRF proxy
621ede0 
- Add marketplace.dify.ai to default allowed domains in squid.conf
- Remove separate marketplace configuration example as it's no longer needed
- Update documentation to reflect marketplace is allowed by default
@laipz8200
chore: reorder example configuration files after marketplace removal
1e971bd 
- Rename example configs to maintain sequential numbering (10, 20, 30)
- Update README to reflect new file numbering
- Keep testing config as 00 since it's a special case
@laipz8200
chore: consolidate gitignore rules to root .gitignore
fb36069 
- Move docker/ssrf_proxy/conf.d/ ignore rule to root .gitignore
- Remove redundant docker/ssrf_proxy/.gitignore file
- Keep all gitignore rules in a single location for better maintainability
@laipz8200
test(ssrf_proxy): Add integration test for ssrf proxy
42110a8 
Signed-off-by: -LAN- <[email protected]>
@laipz8200
feat(ssrf_proxy): Add dev-mode and tests for ssrf_proxy
6a54980 
Signed-off-by: -LAN- <[email protected]>
@autofix-ci @laipz8200
[autofix.ci] apply automated fixes
b7c8724
crazywoola
crazywoola previously approved these changes Sep 2, 2025
View reviewed changes
@laipz8200 laipz8200 marked this pull request as draft September 9, 2025 19:24
@dosubot dosubot bot mentioned this pull request Sep 10, 2025
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yeuoly Yeuoly Yeuoly left review comments

@crazywoola crazywoola crazywoola left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
📚 documentation Improvements or additions to documentation 💪 enhancement New feature or request lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Strengthen SSRF proxy default configuration to prevent security vulnerabilities
4 participants
@laipz8200 @DavideDelbianco @Yeuoly @crazywoola