🔥Feature (v3): Add TrustInternalIPs Config Option #3137
Conversation
xEricL
requested review from
gaby,
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
WalkthroughThe changes introduce two new boolean fields, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Proxy
User->>App: Request with X-Forwarded-For
App->>Proxy: Check if Proxy is Trusted
Proxy->>App: Return IP
App->>App: Check EnableTrustedProxyCheck
App->>App: Check TrustInternalIPs
App->>User: Respond based on trust evaluation
Assessment against linked issues
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (5)
Files skipped from review due to trivial changes (1)
Additional comments not posted (8)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3137 +/- ##
==========================================
- Coverage 80.19% 80.11% -0.08%
==========================================
Files 117 117
Lines 9048 9052 +4
==========================================
- Hits 7256 7252 -4
- Misses 1360 1365 +5
- Partials 432 435 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Per discord conversation something like this will be added to make this more user-friendly: type TrustedProxyConfig struct {
IPs []string
Ranges []string
Unicast bool
Multicast bool
LinkLocalUnicast bool
LinkLocalMulticast bool
Loopback bool
Private bool
} |
|
@xEricL thx for the PR |
|
@ReneWerner87 I talked to @xEricL on Discord, he will be implemented the options from Express.js |
|
@ReneWerner87 hey, I'm sorry I've been very busy with school so I haven't had a chance to finish it up. I've had it mostly done for a while now but I'm having second thoughts about how it looks; It just feels a bit verbose. Here are some examples. What do you guys think? app := fiber.New(fiber.Config{
EnableTrustedProxyCheck: true,
TrustedProxyConfig: fiber.TrustedProxyConfig{
Proxies: []string{"192.168.0.0/24", "10.0.0.0/16", "0.0.0.0/8"},
},
})app := New(Config{
EnableTrustedProxyCheck: true,
TrustedProxyConfig: fiber.TrustedProxyConfig{
Proxies: []string{"172.16.0.0/12"},
Loopback: true,
},
}) |
|
@xEricL In express.js this is actually called "trust proxy". See here https://expressjs.com/en/guide/behind-proxies.html How about this? app := New(Config{
TrustProxy: true,
TrustProxyConfig: fiber.TrustProxyConfig{
Proxies: []string{"172.16.0.0/12"},
Loopback: true,
},
})@ReneWerner87 @efectn Thoughts? |
It seems OK to me. |
|
I've opened a new PR with a new implementation for this feature. #3170 |
Description
This adds a new config option called
TrustInternalIPs, allowing developers to trust Loop-back, Private, and Link-local IP addresses without adding the ranges toTrustedProxieswhenEnableTrustedProxyCheckis enabled. If these ranges are added manually, it can negatively impact performance (see #2933 benchmarks).I chose these three ranges because these are the same ranges enabled by default when configuring an IP extractor in Echo.
Fixes #2930
Changes introduced
Type of change
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md