fix: don't pass EDNS0 DNS Cookies through - #2252
Conversation
Blocky doesn't implement DNS Cookies (RFC 7873), but forwarded them in both directions: an upstream's Server Cookie reached the client, and a Server Cookie the client returned reached an upstream. Whether a client saw a cookie therefore depended on which upstream answered (parallel_best picks two at random per query) and on whether the answer came from the cache, which stores responses without an OPT record. c-ares tracks cookie support per server address and drops cookieless answers from a server that has already sent one until two minutes have passed without a cookie, so blocky's flip-flopping kept resolution failing (c-ares/c-ares#1081, home-assistant/core#145708). Passing the cookie on was wrong regardless of the upstream strategy: blocky can neither validate a Server Cookie a client returns nor reissue one, and a returned cookie is likely to reach an upstream other than the one that issued it, which can't validate it either and may answer BADCOOKIE. Strip the COOKIE option from the response and from the query sent upstream, keeping the OPT record itself so it still carries the DO bit and the buffer size the client advertised. Fixes #2201 Claude-Session: https://claude.ai/code/session_01X2Dg33aFoCW4AvWQasX5rX
With ecs.useAsClient enabled, no mask configured and ecs.forward disabled, the ECS option a client sent is dropped before the query goes upstream. Doing that with util.RemoveEdns0Option also deleted the whole OPT record whenever the subnet was the only option in it, taking the DO bit and the advertised UDP buffer size with it. The ECS resolver sits below the DNSSEC resolver in the chain, so with dnssec.validate enabled it deleted the OPT record that resolver had just set the DO bit on: the upstream then answered without signatures and validation saw an unsigned response. Claude-Session: https://claude.ai/code/session_01X2Dg33aFoCW4AvWQasX5rX
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2252 +/- ##
==========================================
+ Coverage 88.08% 88.14% +0.05%
==========================================
Files 126 126
Lines 9961 9967 +6
==========================================
+ Hits 8774 8785 +11
+ Misses 923 920 -3
+ Partials 264 262 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, consistently applied at the request/response choke points, preserves required EDNS0 OPT semantics, and is backed by targeted new specs covering the reported regressions.
Pull request overview
This PR prevents EDNS0 DNS COOKIE options (RFC 7873) from being forwarded through blocky in either direction, ensuring downstream clients never see cookies and upstreams never receive client cookies. It also preserves the OPT record even when the removed option was the only one present, avoiding loss of EDNS0 metadata (DO bit / UDP buffer size) and fixing a related ECS edge case.
Changes:
- Strip EDNS0 COOKIE from inbound client requests before the resolver chain runs.
- Strip EDNS0 COOKIE from outbound responses as the final normalization step before writing to the wire.
- Introduce
util.RemoveEdns0OptionKeepRecord(shared implementation with existingRemoveEdns0Option) and update ECS removal to keep OPT; add unit/spec coverage.
File summaries
| File | Description |
|---|---|
| util/edns0.go | Adds RemoveEdns0OptionKeepRecord and refactors option removal to support keeping/removing an emptied OPT record. |
| util/edns0_test.go | Adds unit specs validating OPT preservation and correct option removal behavior. |
| server/server.go | Strips client COOKIE from requests early while preserving OPT metadata. |
| server/server_test.go | Adds integration specs ensuring client COOKIE isn’t forwarded and upstream COOKIE isn’t returned. |
| server/client_query.go | Strips upstream COOKIE from responses during normalization while keeping OPT intact. |
| resolver/ecs_resolver.go | Switches ECS option removal to keep the OPT record when it would otherwise become empty. |
| resolver/ecs_resolver_test.go | Adds a regression spec ensuring ECS removal preserves DO bit and UDP size. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Fixes #2201.
The problem
Blocky doesn't implement DNS Cookies (RFC 7873) but forwarded them in both directions: an upstream's Server Cookie reached the client, and a Server Cookie the client returned reached an upstream.
That makes the presence of a cookie in blocky's answers depend on things the client can't see:
parallel_bestpicks two of them at random per query;packForCachestrips the OPT record, so every cache hit is cookieless;So this is not specific to
parallel_bestas the issue title suggests: a single cookie-supporting upstream flip-flops too, as soon as caching or blocking is in play.c-ares tracks cookie support per server address. Once it has seen a cookie from an address it treats a later cookieless answer as a possible spoof and drops it, resetting to "no cookie support" only after two continuous minutes without one (
COOKIE_REGRESSION_TIMEOUT_MS). Blocky's alternating answers keep restarting that timer, so the client keeps discarding valid replies. See c-ares/c-ares#1081 and home-assistant/core#145708.Passing the cookie through was wrong regardless of the upstream strategy. RFC 7873 makes a Server Cookie a commitment by the server that issued it: blocky can neither validate one a client returns nor reissue one of its own. And a returned cookie is likely to reach an upstream other than the one that issued it, which can't validate it either and may answer BADCOOKIE — an rcode blocky passes straight to the client, since only SERVFAIL gets special handling.
The fix
Strip the COOKIE option in both directions, in the two places every query and every response passes through regardless of transport:
server.resolveremoves it from the request before the resolver chain runs;clientQuery.normalizeResponseremoves it from the response as the last step before the wire.The OPT record itself is kept, since on a request it still carries the DO bit and the buffer size the client advertised, and on a response RFC 6891 §6.1.1 requires one for an EDNS0 query (#2240).
util.RemoveEdns0OptionKeepRecordis the existingRemoveEdns0Optionwithout its "drop the record once it is empty" step; both now share one implementation.Blocky consequently never speaks cookies, whatever the upstreams do, and a client settles on "no cookie support" and stays there.
Implementing cookies properly — blocky as a cookie server downstream (RFC 9018 server cookies) and as a cookie client upstream, with its own client cookie per upstream and BADCOOKIE retries — would additionally protect the blocky→upstream hop, which is the one that actually crosses an untrusted network. That's a feature rather than a fix, so it is not part of this PR.
Second commit
The same footgun had a second victim:
ECSResolverdropped the whole OPT record when the ECS option it removes was the only one in it. Because the ECS resolver sits below the DNSSEC resolver in the chain, withdnssec.validateenabled it deleted the OPT record that resolver had just set the DO bit on, and validation then saw an unsigned upstream answer. Needsecs.useAsClient: true, no mask configured andecs.forward: false, so it's narrow — but it's the same one-line change.Note that
util.RemoveEdns0Option(the variant that drops an emptied OPT record) has no callers left after this. I kept it rather than widen the diff; happy to delete it if you'd prefer.Tests
New specs, each watched failing before the fix:
RemoveEdns0OptionKeepRecordunit specs.Full suite (everything but
e2e, which needs Docker) andmake lintare green.https://claude.ai/code/session_01X2Dg33aFoCW4AvWQasX5rX