Skip to content

lmdb NSEC3 record handling hygiene#15767

Merged
miodvallat merged 10 commits into
PowerDNS:masterfrom
miodvallat:nsecticide
Jul 3, 2025
Merged

lmdb NSEC3 record handling hygiene#15767
miodvallat merged 10 commits into
PowerDNS:masterfrom
miodvallat:nsecticide

Conversation

@miodvallat

Copy link
Copy Markdown
Contributor

Short description

This (hopefully) completes the recent NSEC3 record handling work in LMDB, to guarantee that we never leave dangling records when performing deletions or updates:

  • when updating an NSEC3 record, remove the previous back record (since it would become dangling).
  • when removing an NSEC3 record, always remove the other record from the pair.

Note the second item from the list above manifests itself in multiple ways in replaceRRSet:

  • When removing all records regardless of their type (using QType::ANY), we need to check for an NSEC3 record and, if found, remove the matching record of the chain.
  • When removing a particular record type, and not adding any record after that, we need to check whether we end up with a single NSEC3 record to that name, in which case it should be removed (with the matching record from the pair).

This should fix #11612 and give #11611 a chance to get merged soon.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@miodvallat miodvallat added the auth label Jul 2, 2025
@coveralls

coveralls commented Jul 2, 2025

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 16052007491

Details

  • 132 of 149 (88.59%) changed or added relevant lines in 3 files are covered.
  • 21 unchanged lines in 7 files lost coverage.
  • Overall coverage increased (+2.6%) to 65.606%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pdns/rfc2136handler.cc 52 57 91.23%
modules/lmdbbackend/lmdbbackend.cc 75 87 86.21%
Files with Coverage Reduction New Missed Lines %
pdns/rcpgenerator.cc 2 90.13%
pdns/recursordist/sortlist.cc 2 72.94%
pdns/rfc2136handler.cc 2 72.38%
modules/gpgsqlbackend/spgsql.cc 3 68.42%
pdns/recursordist/syncres.cc 3 80.34%
pdns/remote_logger.cc 3 56.7%
modules/godbcbackend/sodbc.cc 6 76.03%
Totals Coverage Status
Change from base Build 16051915340: 2.6%
Covered Lines: 126980
Relevant Lines: 164867

💛 - Coveralls

@miodvallat miodvallat force-pushed the nsecticide branch 2 times, most recently from 3fd9090 to 5ff5c63 Compare July 3, 2025 12:20
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
...now that writeNSEC3RecordPair() can handle updates correctly.

Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
…in replaceRRSet().

Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
The second record from the pair would end up overwriting the first one,
which could confuse the logic assuming pairs are always well-formed.

Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
…g NSEC3.

Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
@miodvallat miodvallat merged commit 738bd41 into PowerDNS:master Jul 3, 2025
89 checks passed
@miodvallat miodvallat deleted the nsecticide branch July 3, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NSEC chain bug with LMDB backend

3 participants