🔥 feat: Host auth middleware #4199
Conversation
|
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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new hostauthorization middleware: implementation, configuration, documentation, README index entry, and comprehensive tests/benchmarks that enforce Host header allowlisting (exact, leading-dot wildcard, CIDR, dynamic validator) and rejection via a configurable error handler. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant App as Fiber App
participant HA as HostAuthorization
participant Store as AllowedHostsStore
participant Func as AllowedHostsFunc
participant Err as ErrorHandler
Client->>App: HTTP request (Host header)
App->>HA: invoke middleware
HA->>HA: normalize Host (strip port/brackets/trailing dot, lowercase)
HA->>Store: check exact / wildcard / CIDR matches
alt matched
Store-->>HA: allowed
HA->>App: c.Next() -> downstream handler
else not matched
HA->>Func: if configured, call AllowedHostsFunc(host)
alt func returns true
Func-->>HA: allowed
HA->>App: c.Next() -> downstream handler
else
HA->>Err: call ErrorHandler(c, ErrForbiddenHost)
Err-->>Client: 403 Forbidden (or custom response)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4199 +/- ##
==========================================
+ Coverage 91.18% 91.21% +0.03%
==========================================
Files 129 131 +2
Lines 12749 12833 +84
==========================================
+ Hits 11625 11706 +81
- Misses 709 711 +2
- Partials 415 416 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new hostauthorization middleware for Fiber v3, designed to protect against DNS rebinding attacks by validating the Host header against an allowlist of exact domains, subdomain wildcards, and CIDR ranges. The implementation includes comprehensive documentation and tests. Feedback focuses on making the configuration initialization more idiomatic for the Fiber framework and optimizing the performance of subdomain matching by pre-storing wildcard suffixes with their leading dots to avoid allocations during request processing.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/middleware/hostauthorization.md`:
- Around line 63-79: The CIDR Ranges doc misleadingly describes source-IP
filtering; update the wording to clarify that hostauthorization checks the
request Host header against AllowedHosts (not the client's IP). Edit the
examples and comments around hostauthorization.New and hostauthorization.Config
AllowedHosts entries to say things like "allowed Host header IP range" or "Host
header value" instead of "internal network" or "respond to requests from known
IP ranges" so readers understand this matches Host header strings/CIDR-like IPs
in the Host header only.
In `@middleware/hostauthorization/config.go`:
- Around line 13-41: Add the missing Exclude hook to the public Config by
declaring a field Exclude func(c fiber.Ctx) bool (optional, default nil) next to
Next and ErrorHandler in the Config struct; update the middleware code path that
currently checks Next to also call Exclude (both should be evaluated before any
host validation/AllowedHosts/AllowedHostsFunc logic) and treat a true return
from Exclude as a signal to skip host validation and not invoke ErrorHandler.
Ensure the field name is exactly Exclude and that callers can provide it the
same way as Next/ErrorHandler.
In `@middleware/hostauthorization/hostauthorization.go`:
- Around line 24-45: Normalize each configured host string using normalizeHost()
before any checks and storage: call normalizeHost(h) (after trimming) and skip
if it becomes empty; for CIDR parsing still use the normalized value for
net.ParseCIDR and append the result to parsed.cidrNets; for wildcard entries
(case strings.HasPrefix(h, ".")) store the suffix without the leading dot from
the normalized value into parsed.wildcardSuffixes; for exact matches insert the
normalized host into parsed.exact. Ensure all references use the normalized host
variable so comparisons match request hosts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: abe37dce-637a-44c8-80d2-c96bfa96bba2
📒 Files selected for processing (5)
.github/README.mddocs/middleware/hostauthorization.mdmiddleware/hostauthorization/config.gomiddleware/hostauthorization/hostauthorization.gomiddleware/hostauthorization/hostauthorization_test.go
|
Thanks for the contribution @mutantkeyboard , DNS rebinding protection is a valid concern and I appreciate the thorough work on docs and tests. Before we move forward, I want to raise a design question: should this be a standalone middleware or an extension of the existing
If we do keep it as a standalone middleware, a few things need addressing: Performance
API / Consistency
Documentation
Scope question
I'd like to hear your thoughts on the |
|
Hey @ReneWerner87, thank you for the review. Regarding I'm genuinely curious what did you have in mind there. My original inspiration for this was brannondorsey/host-validation, which is explicitly a standalone Performance
This one is actually a good catch - config entries with trailing dots won't match runtime-normalized hosts. Easy fix. CIDR docs This is actually a fair point. I can fix the wording to make clear it's matching the But regarding the overall architecture, I can integrate it into the existing WDYT? |
|
ok Go ahead with the leading-dot fix at init + normalizeHost on config entries, that alone should clean up the hot path nicely. And yes, please add an isolated benchmark for just the matching logic so we have real numbers for the middleware cost without pls also try the utils functions for the better performance |
|
Given the current implementation and the distinct purposes, I recommend keeping Here's why:
Therefore, maintaining it as a dedicated middleware seems to be the most robust and flexible approach for this security feature. |
There was a problem hiding this comment.
Pull request overview
Adds a new hostauthorization middleware to validate the incoming Host header against an allowlist to mitigate DNS rebinding / Host-header attacks, with support for exact hosts, subdomain wildcards, CIDR ranges, and an optional dynamic validator.
Changes:
- Introduces
middleware/hostauthorizationwith config, parsing/normalization, and request-time host matching. - Adds a comprehensive test suite plus benchmarks for matching and end-to-end middleware behavior.
- Adds end-user documentation and lists the middleware in
.github/README.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| middleware/hostauthorization/hostauthorization.go | Implements allowlist parsing + host matching and the middleware handler. |
| middleware/hostauthorization/config.go | Defines middleware configuration, defaults, and exported error. |
| middleware/hostauthorization/hostauthorization_test.go | Adds unit/integration tests and benchmarks for matching behavior and proxy semantics. |
| docs/middleware/hostauthorization.md | Documents usage, matching/normalization rules, and proxy/RFC notes. |
| .github/README.md | Adds the new middleware to the project’s middleware list. |
|
|
||
| ### Subdomain Wildcards | ||
|
|
||
| A leading dot matches any subdomain but **not** the bare domain itself: |
There was a problem hiding this comment.
This not matching the bare domain, is a weird behavior.
|
|
||
| // Host: api.myapp.com → 200 OK | ||
| // Host: www.myapp.com → 200 OK | ||
| // Host: myapp.com → 403 Forbidden |
There was a problem hiding this comment.
This a confusing behavior. Requiring 2 allow rules
| Useful for services accessed directly by IP (e.g. internal tooling) where the `Host` header will be a raw IP address. This matches the **Host header value** against a CIDR range — it does not filter by client IP address: | ||
|
|
||
| ```go | ||
| app.Use(hostauthorization.New(hostauthorization.Config{ |
There was a problem hiding this comment.
This is kind of confusing with https://docs.gofiber.io/whats_new#trusted-proxies
I rather see the same TrustedProxyConfig here, to avoid duplicated code.
| continue | ||
| } | ||
|
|
||
| if hostIP, cidr, err := net.ParseCIDR(h); err == nil { |
There was a problem hiding this comment.
This is all duplicated logic from TrustedProxyConfig handling. Maybe we should make that a public function that can be used from here
ReneWerner87
left a comment
ReneWerner87
left a comment
There was a problem hiding this comment.
Going through the latest version of the branch (3334763), here is a follow-up on top of what's already been discussed. Most of the high-priority code-level findings are in inline comments. The points below are either architectural (don't fit a single line) or reinforcements of things @gaby and others already raised that I think deserve a more concrete recommendation.
IDN / Punycode
Browsers always send internationalised domains as Punycode in the Host header: münchen.example.com arrives as xn--mnchen-3ya.example.com. With the current implementation, configuring "münchen.example.com" in AllowedHosts will never match a real browser request.
There's already precedent in the codebase (req.go, Subdomains()):
if strings.Contains(host, "xn--") {
if u, err := idna.Lookup.ToUnicode(host); err == nil {
host = utilsstrings.ToLower(u)
}
}Suggest normalising both sides (config entries and request hostnames) consistently, ideally to ASCII via idna.Lookup.ToASCII since browsers consistently use Punycode and ASCII comparisons stay alloc-free. At minimum this should be a documented "Known limitations" entry.
TrustedProxyConfig consolidation (reinforcing @gaby)
The CIDR parse and contains-check logic is essentially identical to app.handleTrustedProxy. A shared helper would benefit both modules and keep behaviour consistent (the canonical-CIDR panic exists here but not in handleTrustedProxy). Worth doing, even if as a follow-up PR.
Subdomain wildcard semantics (reinforcing @gaby)
The leading-dot convention that excludes the apex is ergonomically weak and forces two entries for the most common case. Worth deciding now since changing it later is breaking. My preference would be *.myapp.com for subdomains-only and myapp.com for apex (familiar glob syntax), but implicit apex inclusion would also be defensible.
Test gaps
A few that would be worth adding alongside the inline points:
-
AllowedHostsFuncas fallback (with call-counter assertion, see L80 inline) -
IPv6 + port in
AllowedHostsconfig entry ("[::1]:8080"). IPv4 is covered, IPv6 isn't. -
Concurrent requests with
-race. The implementation is read-only after init but the contract should be locked in. -
A scaling benchmark with many wildcards (e.g. 100 entries) so we know when the linear
HasSuffixscan starts mattering. -
A
FuzzNormalizeHosttarget. Trivial to add and standard hygiene for any header parser:func FuzzNormalizeHost(f *testing.F) { f.Add("example.com") f.Add("[::1]:8080") f.Add(".myapp.com") f.Fuzz(func(t *testing.T, input string) { _ = normalizeHost(input) }) }
Sentinel error stability
ErrForbiddenHost is exported, so users can do errors.Is(...). A single test asserting the exact Error() string locks in the textual contract for those who match on the message. Minor but useful.
Scope question
CIDR matching accounts for the bulk of the surface area in this PR (parsing, the non-canonical panic, documentation confusion with TrustedProxyConfig, and the duplicated logic). For an initial merge, dropping CIDR and shipping with exact + subdomain wildcards only would make this a tighter, more obviously-correct module. CIDR can land in a follow-up where it's properly consolidated with the trusted-proxy infrastructure.
Not a blocker, just a thought given how much of the review feedback (mine and @gaby's) orbits the CIDR feature.
Overall really nice work on the iterations so far. The split between matchHost micro-benchmarks and the full HTTP-pipeline benchmarks is a great addition for isolating actual middleware cost.
| - **RFC 9110 Section 7.2** — Host and port are separate components; port is stripped before matching | ||
| - **RFC 9110 Section 17.1** — Origin servers should reject misdirected requests | ||
| - **RFC 9112 Section 3.2** — Requests with missing Host headers should be rejected | ||
| - Returns **403 Forbidden** (not 400) because the request is syntactically valid but semantically unauthorized |
There was a problem hiding this comment.
Worth considering 421 Misdirected Request as the default instead of 403. RFC 9110 §15.5.20 defines it as:
The request was directed at a server that is unable or unwilling to produce an authoritative response for the target URI.
Which fits the "wrong host for this server" semantics better than 403 ("understood, but refused due to permissions"). Cloudflare, Fastly and other CDNs use 421 for exactly this case.
Either response is defensible, but 403 is currently presented here as the only correct option, which I don't think it is. At minimum, 421 deserves a mention as an alternative.
|
|
||
| ## Proxy Support | ||
|
|
||
| The middleware uses Fiber's `c.Hostname()`, which respects `X-Forwarded-Host` when [`TrustProxy`](https://docs.gofiber.io/api/fiber#config) is enabled. When `TrustProxy` is disabled (the default), `X-Forwarded-Host` is ignored and the raw `Host` header is used. |
There was a problem hiding this comment.
Worth a sentence here noting HTTP/2 behavior: clients send :authority instead of Host. fasthttp transparently maps it, so the middleware works the same in both protocols, but operators reading this section currently might assume HTTP/1.1-only.
|
@mutantkeyboard pls check the last comments |
|
@ReneWerner87 - This is an excellent wrap-up by both you and @gaby - thank you guys so much. I'll try to fix all of these ASAP, and will re-request your review once I'm done. |
Re-review summaryVerified the addressed inline points against Blocking (please address or explicitly defer with a follow-up issue)
Non-blocking but worth doing
Already verified addressedAll 8 inline code findings from my CHANGES_REQUESTED review (map type, CIDR footgun, two hot-path allocations, AllowedHostsFunc fallback ordering, both godoc inconsistencies, hardcoded status string, empty-host test) are resolved in their individual threads. Once items 1-3 land (or are explicitly deferred via tracking issues), I am happy to approve. |
|
@mutantkeyboard can you check the open points |
Description
Add a new
hostauthorizationmiddleware that validates the incoming Host header against a configurable allowlist, rejecting unauthorized hosts with 403 Forbidden. This protects against DNS rebinding attackswhere an attacker-controlled domain resolves to the application's internal IP, causing browsers to send requests with a malicious Host header.
The middleware supports exact host matching, subdomain wildcards (.myapp.com), and CIDR ranges (10.0.0.0/8), with an optional dynamic validator function for runtime host resolution. It complements the
Domain()router (#4100) —
Domain()handleshost-basedrouting whilehostauthorizationacts as a security gate.Fixes #4148
Changes introduced
middleware. Next, ErrorHandler follow established Fiber conventions.
dependencies, no state, no caching.
Domain()router.Type of change
Please delete options that are not relevant.
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