Skip to content

Enforce RFC 9520 failure cache TTL bounds - #66

Open
zbalkan wants to merge 1 commit into
TechnitiumSoftware:masterfrom
zbalkan:fix/rfc9520-failure-cache-ttl
Open

zbalkan wants to merge 1 commit into
TechnitiumSoftware:masterfrom
zbalkan:fix/rfc9520-failure-cache-ttl

Conversation

@zbalkan

@zbalkan zbalkan commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary

DnsCache caches failed/SERVFAIL lookups (BadCache/FailureCache records) so a resolver doesn't hammer an unreachable name server on every query. Per RFC 9520 ("Negative Caching of DNS Resolution Failures"), that failure TTL must stay bounded — long enough to suppress query storms, short enough that a real recovery is noticed quickly.

The bound wasn't actually enforced. SetExpiry() for failure records used the generic _minimumRecordTtl/_maximumRecordTtl fields (defaults 10s–3600s, and instance-configurable), not a bound specific to failure caching. A caller that raised the generic minimum TTL — a legitimate, independent knob — would silently drag failure-cache entries along with it, past the RFC's 5-minute ceiling. FailureRecordTtl's public setter also took any uint as-is, with no bound at all.

What changed

  • Added FAILURE_RECORD_TTL_MIN (1s) and FAILURE_RECORD_TTL_MAX (300s) constants in DnsCache.
  • BadCache/FailureCache records now call SetExpiry(FAILURE_RECORD_TTL_MIN, FAILURE_RECORD_TTL_MAX, …) instead of the generic min/max, so failure-cache TTL is bounded independently of MinimumRecordTtl/MaximumRecordTtl.
  • The FailureRecordTtl setter now clamps via Math.Clamp(value, FAILURE_RECORD_TTL_MIN, FAILURE_RECORD_TTL_MAX) instead of accepting the raw value.
  • The constructor now assigns through the FailureRecordTtl property instead of the backing field directly, so the clamp applies consistently regardless of whether the value is set at construction or later via the property.

No public API signatures changed — this is a behavioral fix, not an interface change.

Why this approach

Bounding failure-cache TTL independently (rather than, say, just validating input at each call site) keeps the invariant in one place: any path that sets FailureRecordTtl, or that constructs a failure/bad-cache record, goes through the same clamp. That removes the coupling to the generic TTL bounds, which was the actual bug, and makes it impossible to construct a DnsCache instance whose failure TTL disagrees with RFC 9520 regardless of how it's configured.

Testing

  • dotnet build TechnitiumLibrary.Net/TechnitiumLibrary.Net.csproj -c Release — succeeds, 0 warnings, 0 errors.
  • dotnet build TechnitiumLibrary.sln -c Release — succeeds for every project except the pre-existing, unrelated TechnitiumLibrary.Net.Firewall (Windows-only COM interop, fails on Linux regardless of this change).
  • Manually traced every FailureRecordTtl write path in the downstream consumer, TechnitiumSoftware/DnsServer (binary config load, legacy web service, settings API) — all go through the public property, so none bypasses the new clamp.
  • Built DnsServerCore.csproj from TechnitiumSoftware/DnsServer against this branch's compiled assemblies — succeeds with 0 warnings, 0 errors, confirming no breakage in the primary consumer.
  • No automated tests exist for DnsCache in this repo today. Given this is a boundary-value fix, a couple of unit tests (constructing with out-of-range failureRecordTtl and asserting the clamp; caching a failure response and asserting the resulting record's TTL is within [1, 300]) would be a good addition — happy to add them if wanted, opened as a follow-up otherwise.

Caveat

TechnitiumSoftware/DnsServer's settings API and admin UI (cacheFailureRecordTtl) perform no independent range validation before calling FailureRecordTtl's setter. After this change, a value submitted outside [1, 300] is silently clamped rather than rejected — the admin gets a different value than what they entered, with no error surfaced (though the settings-read endpoint does report back the clamped value, so the UI isn't left showing a stale number). This is a downstream UX gap, not a defect introduced here, and not a reason to hold this PR; it's tracked as a follow-up for TechnitiumSoftware/DnsServer to add explicit input validation.

Signed-off-by: Zafer Balkan <zafer@zaferbalkan.com>
@ShreyasZare

Copy link
Copy Markdown
Member

Thanks for the PR. Will check it soon in detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants