Skip to content

Conversation

@gthess
Copy link
Member

@gthess gthess commented Apr 22, 2024

An expired delegation could cause delegation invalidation for its descendants.
When a parent replies with a grandchild delegation, an expired child delegation may never be updated and cause all sub queries to end up in the parent.
With this PR when a gandchild delegation response is returned, child delegations up to the parent are removed from the cache to try and avoid this.

…legations

  up to parent to not cause delegation invalidation because of an
  expired child delegation that would never be updated. Most likely to
  happen without qname-minimisation. Reported by Roland van Rijswijk-Deij.
@gthess gthess requested a review from wcawijngaards April 22, 2024 14:01
@gthess gthess self-assigned this Apr 22, 2024
Copy link
Member

@wcawijngaards wcawijngaards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good. The code removes the expired NS entries for multilabel delegations, from the intermediate labels, so that the ghost domain fixup does not trip on them as expired, but never removed, entries. This keeps the expired NS records in a way to detect that parental delegations need to be rechecked.

@gthess
Copy link
Member Author

gthess commented Apr 23, 2024

Thanks!

gthess added a commit that referenced this pull request Apr 23, 2024
@gthess gthess merged commit 3ec74d1 into master Apr 23, 2024
@gthess gthess deleted the bugfix/grandchild-delegation branch April 23, 2024 12:25
jedisct1 added a commit to jedisct1/unbound that referenced this pull request May 7, 2024
* nlnet/master: (45 commits)
  - Fix for NLnetLabs#1062: declaration before statement, avoid print of null,   and redundant check for array size. And changelog note for merge of NLnetLabs#1062.
  Fix potential overflow bug while parsing port in function cfg_mark_ports
  - Set version number to 1.20.0 for release.
  - Fix for the DNSBomb vulnerability CVE-2024-33655. Thanks to Xiang Li   from the Network and Information Security Lab of Tsinghua University   for reporting it.
  - Fix doxygen comment for errinf_to_str_bogus.
  - Cleanup unnecessary strdup calls for EDE strings.
  - Man page entry for unbound-checkconf -q.
  - Fix NLnetLabs#876: [FR] can unbound-checkconf be silenced when configuration   is valid?
  - Add unit tests for cachedb and subnet cache expired data.
  - Fix cachedb with serve-expired-client-timeout disabled. The edns   subnet module deletes global cache and cachedb cache when it   stores a result, and serve-expired is enabled, so that the global   reply, that is older than the ecs reply, does not return after   the ecs reply expires.
  - Fix doc unit test for out of directory build.
  - Fix to disable fragmentation on systems with IP_DONTFRAG,   with a nonzero value for the socket option argument.
  Changelog note for NLnetLabs#1041 and NLnetLabs#1038. - Merge NLnetLabs#1041: Stub and Forward unshare. This has one structure   for them and fixes NLnetLabs#1038: fatal error: Could not initialize   thread / error: reading root hints.
  Update locking management for iter_fwd and iter_hints methods. (NLnetLabs#1054)
  - Fix configure flto check error, by finding grep for it.
  - Fix ci workflow for macos for moved install locations.
  - Merge NLnetLabs#1053: Remove child delegations from cache when grandchild   delegations are returned from parent.
  - When a granchild delegation is returned, remove any cached child delegations   up to parent to not cause delegation invalidation because of an   expired child delegation that would never be updated. Most likely to   happen without qname-minimisation. Reported by Roland van Rijswijk-Deij.
  - Fix edns subnet to sort rrset references when storing messages   in the cache. This fixes a race condition in the rrset locks.
  - Add checklock feature verbose_locking to trace locks and unlocks.
  ...
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